Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extra validation for checking topic description is between 1 and 100 characters long #2689

Merged
merged 2 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/src/main/java/io/aiven/klaw/error/KlawErrorMessages.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
139 changes: 139 additions & 0 deletions core/src/test/java/io/aiven/klaw/model/ValidationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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")));
}
}