Add extra validation for checking topic description is between 1 and …

…100 characters long (#2689)

Signed-off-by: Aindriu Lavelle <[email protected]>

core/src/main/java/io/aiven/klaw/error/KlawErrorMessages.java

@@ -414,6 +414,9 @@ public class KlawErrorMessages {
 
   public static final String TOPICS_VLD_ERR_126 = "Failure. Data integrity violation: ";
 
+  public static final String TOPICS_VLD_ERR_127 =
+      "Description must be a minimum of 1 character and a maximum of 100, this can be less if multibyte encoding is being used.";
+
   // Topic overview service
   public static final String TOPIC_OVW_ERR_101 = "Topic does not exist in any environment.";
 

core/src/main/java/io/aiven/klaw/model/requests/BaseRequestModel.java

@@ -13,9 +13,11 @@
 public class BaseRequestModel implements Serializable {
 
   // CREATE / DELETE / ..
-  @NotNull private RequestOperationType requestOperationType;
+  @NotNull(message = "Request operation type must not be null")
+  private RequestOperationType requestOperationType;
 
-  @NotNull private String environment;
+  @NotNull(message = "The environment must not be null")
+  private String environment;
 
   private String appname;
 

core/src/main/java/io/aiven/klaw/model/requests/KafkaConnectorRequestModel.java

@@ -2,6 +2,7 @@
 
 import jakarta.validation.constraints.NotNull;
 import jakarta.validation.constraints.Pattern;
+import jakarta.validation.constraints.Size;
 import java.io.Serializable;
 import lombok.Getter;
 import lombok.Setter;
@@ -12,14 +13,20 @@
 @ToString
 public class KafkaConnectorRequestModel extends BaseRequestModel implements Serializable {
 
-  @NotNull
+  @NotNull(message = "Connector name must not be null")
   @Pattern(message = "Invalid connector name", regexp = "^[a-zA-Z0-9._-]{3,}$")
   private String connectorName;
 
-  @NotNull private String connectorConfig;
+  @NotNull(message = "Connector configuration must not be null")
+  private String connectorConfig;
 
-  @NotNull
+  @NotNull(message = "Connector description must not be null")
   @Pattern(message = "Invalid description", regexp = "^[a-zA-Z 0-9_.,-]{3,}$")
+  @Size(
+      min = 1,
+      max = 100,
+      message =
+          "Description must be a minimum of 1 character and a maximum of 100, this can be less if multibyte encoding is being used.")
   private String description;
 
   private Integer teamId;

core/src/main/java/io/aiven/klaw/model/requests/TopicRequestModel.java

@@ -14,20 +14,20 @@
 @Setter
 @ToString
 public class TopicRequestModel extends BaseRequestModel implements Serializable {
-
-  @NotNull
+  // Validation is overridden by TopicRequestValidatorImpl on TopicCreateRequests.
+  @NotNull(message = "Topic name must not be null")
   @Pattern(message = "Invalid topic name", regexp = "^[a-zA-Z0-9._-]{3,}$")
   private String topicname;
 
-  @NotNull
-  @Min(value = 1, message = "TopicPartitions must be greater than zero")
+  @NotNull(message = "Topic partitions must not be null")
+  @Min(value = 1, message = "Topic Partitions must be greater than zero")
   private Integer topicpartitions;
 
-  @NotNull
+  @NotNull(message = "Topic replication must not be null")
   @Min(value = 1, message = "Replication factor must be greater than zero")
   private String replicationfactor;
 
-  @NotNull private String description;
+  private String description;
 
   private List<TopicConfigEntry> advancedTopicConfigEntries;
 

core/src/main/java/io/aiven/klaw/validation/TopicRequestValidatorImpl.java

@@ -85,6 +85,15 @@ public boolean isValid(
       return false;
     }
 
+    if (RequestOperationType.CREATE.equals(topicRequestModel.getRequestOperationType())
+        && (topicRequestModel.getDescription() == null
+            || topicRequestModel.getDescription().length() < 1
+            || topicRequestModel.getDescription().length() > 100)) {
+      updateConstraint(constraintValidatorContext, TOPICS_VLD_ERR_127);
+
+      return false;
+    }
+
     // verify tenant config exists
     int tenantId = commonUtilsService.getTenantId(userName);
     String syncCluster;

core/src/test/java/io/aiven/klaw/model/ValidationTest.java

@@ -6,17 +6,40 @@
 import io.aiven.klaw.dao.SchemaRequest;
 import io.aiven.klaw.dao.TopicRequest;
 import io.aiven.klaw.model.enums.AclType;
+import io.aiven.klaw.model.enums.RequestOperationType;
+import io.aiven.klaw.model.requests.KafkaConnectorRequestModel;
+import io.aiven.klaw.model.requests.TopicRequestModel;
+import jakarta.validation.ConstraintViolation;
+import jakarta.validation.Validation;
+import jakarta.validation.Validator;
+import jakarta.validation.ValidatorFactory;
 import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestMethodOrder;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 
 @ExtendWith(SpringExtension.class)
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 public class ValidationTest {
 
+  private static Validator validator;
+
+  @BeforeAll
+  public static void setUp() {
+    ValidatorFactory validatorFactory = Validation.buildDefaultValidatorFactory();
+    validator = validatorFactory.getValidator();
+  }
+
   @Test
   public void testNewTopicRequest() {
     TopicRequest topicRequest = new TopicRequest();
@@ -65,4 +88,120 @@ public void testNewSchemaRequest() {
 
     assertThat(schemaRequest).isNotNull();
   }
+
+  @ParameterizedTest
+  @CsvSource({
+    "null topic name,,1,1,mydesc,must not be null",
+    "Invalid topic name,!!!!,1,1,mydesc,Invalid topic name",
+    "partitions must be greater then 0,topic-1,0,1,mydesc,must be greater than zero",
+    "replication must not be null,topic-1,1,,mydesc,must not be null"
+  })
+  public void validateTopicRequestModel(
+      String testName,
+      String topicName,
+      String partitions,
+      String replication,
+      String description,
+      String errMsgContains) {
+    TopicRequestModel model = new TopicRequestModel();
+    model.setTopicname(topicName);
+    model.setTopicpartitions(Integer.parseInt(partitions));
+    model.setReplicationfactor(replication);
+    model.setDescription(description);
+    model.setEnvironment("Dev");
+    model.setRequestOperationType(RequestOperationType.CREATE);
+
+    Set<ConstraintViolation<TopicRequestModel>> violations = validator.validate(model);
+    assertThat(violations).hasSize(1);
+    ConstraintViolation<TopicRequestModel> violation = violations.iterator().next();
+    assertThat(violation.getMessage()).contains(errMsgContains);
+  }
+
+  @ParameterizedTest
+  @MethodSource("kafkaConnectorGenerateTestData")
+  public void validateConnectorRequestModel(
+      String testName,
+      String connectorName,
+      String connectorConfig,
+      String env,
+      String description,
+      String requestOperationType,
+      List<String> errorMessages) {
+    KafkaConnectorRequestModel model = new KafkaConnectorRequestModel();
+    model.setConnectorName(connectorName);
+    model.setConnectorConfig(connectorConfig);
+    model.setDescription(description);
+    model.setEnvironment(env);
+    model.setRequestOperationType(RequestOperationType.of(requestOperationType));
+
+    Set<ConstraintViolation<KafkaConnectorRequestModel>> violations = validator.validate(model);
+    assertThat(violations).hasSize(errorMessages.size());
+    for (ConstraintViolation<KafkaConnectorRequestModel> violation : violations) {
+
+      assertThat(errorMessages).contains(violation.getMessage());
+    }
+  }
+
+  private static Stream<Arguments> kafkaConnectorGenerateTestData() {
+    return Stream.of(
+        Arguments.of(
+            "null Connector name",
+            null,
+            "{}",
+            "DEV",
+            "mydesc",
+            "Create",
+            List.of("Connector name must not be null")),
+        Arguments.of(
+            "Invalid Connector name",
+            "!!!!",
+            "{}",
+            "DEV",
+            "mydesc",
+            "Create",
+            List.of("Invalid connector name")),
+        Arguments.of(
+            "connector config must not be null",
+            "connector-1",
+            null,
+            "DEV",
+            "mydesc",
+            "Create",
+            List.of("Connector configuration must not be null")),
+        Arguments.of(
+            "environment must not be null",
+            "connector-1",
+            "{}",
+            null,
+            "mydesc",
+            "Create",
+            List.of("The environment must not be null")),
+        Arguments.of(
+            "Description must be less then 100",
+            "connector-1",
+            "{}",
+            "DEV",
+            "ddesddedededededededededededededededededededededdededdesddededededededededededededededededededededes1",
+            "Create",
+            List.of(
+                "Description must be a minimum of 1 character and a maximum of 100, this can be less if multibyte encoding is being used.")),
+        Arguments.of(
+            "Description must be at least 1 char",
+            "connector-1",
+            "{}",
+            "DEV",
+            "",
+            "Create",
+            List.of(
+                "Invalid description",
+                "Description must be a minimum of 1 character and a maximum of 100, this can be less if multibyte encoding is being used.")),
+        Arguments.of(
+            "Request Operation must not be null",
+            "connector-1",
+            "{}",
+            "DEV",
+            "a simple description",
+            "",
+            List.of("Request operation type must not be null")));
+  }
 }