Fixes #12447 Use JSON schema constrain validation in CSV imports (#12448)

This commit is contained in:
Suresh Srinivas 2023-07-15 15:02:17 -07:00 committed by GitHub
parent 00934279c6
commit 0d119de96e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 99 additions and 15 deletions

View File

@ -53,6 +53,7 @@ import org.openmetadata.service.jdbi3.EntityRepository;
import org.openmetadata.service.util.EntityUtil; import org.openmetadata.service.util.EntityUtil;
import org.openmetadata.service.util.JsonUtils; import org.openmetadata.service.util.JsonUtils;
import org.openmetadata.service.util.RestUtil.PutResponse; import org.openmetadata.service.util.RestUtil.PutResponse;
import org.openmetadata.service.util.ValidatorUtil;
/** /**
* EntityCsv provides export and import capabilities for an entity. Each entity must implement the abstract methods to * EntityCsv provides export and import capabilities for an entity. Each entity must implement the abstract methods to
@ -345,6 +346,12 @@ public abstract class EntityCsv<T extends EntityInterface> {
if (Boolean.FALSE.equals(importResult.getDryRun())) { if (Boolean.FALSE.equals(importResult.getDryRun())) {
try { try {
repository.prepareInternal(entity); repository.prepareInternal(entity);
String violations = ValidatorUtil.validate(entity);
if (violations != null) {
// JSON schema based validation failed for the entity
importFailure(resultsPrinter, violations, csvRecord);
return;
}
PutResponse<T> response = repository.createOrUpdate(null, entity); PutResponse<T> response = repository.createOrUpdate(null, entity);
responseStatus = response.getStatus(); responseStatus = response.getStatus();
} catch (Exception ex) { } catch (Exception ex) {

View File

@ -1017,7 +1017,7 @@ public class UserResource extends EntityResource<User, UserRepository> {
try { try {
decodedBytes = Base64.getDecoder().decode(loginRequest.getPassword()); decodedBytes = Base64.getDecoder().decode(loginRequest.getPassword());
} catch (Exception ex) { } catch (Exception ex) {
throw new IllegalArgumentException("Password need to be encoded in Base-64."); throw new IllegalArgumentException("Password needs to be encoded in Base-64.");
} }
loginRequest.withPassword(new String(decodedBytes)); loginRequest.withPassword(new String(decodedBytes));
return Response.status(Response.Status.OK).entity(authHandler.loginUser(loginRequest)).build(); return Response.status(Response.Status.OK).entity(authHandler.loginUser(loginRequest)).build();

View File

@ -66,8 +66,6 @@ public final class EntityUtil {
// //
// Comparators used for sorting list based on the given type // Comparators used for sorting list based on the given type
// //
// Note ordering is same as server side ordering by ID as string to ensure PATCH operations work
public static final Comparator<EntityReference> compareEntityReference = public static final Comparator<EntityReference> compareEntityReference =
Comparator.comparing(EntityReference::getName); Comparator.comparing(EntityReference::getName);
public static final Comparator<EntityVersionPair> compareVersion = public static final Comparator<EntityVersionPair> compareVersion =

View File

@ -0,0 +1,31 @@
package org.openmetadata.service.util;
import io.dropwizard.jersey.validation.DropwizardConfiguredValidator;
import java.util.Arrays;
import java.util.Set;
import javax.validation.ConstraintViolation;
import javax.validation.Validation;
import javax.validation.ValidatorFactory;
public class ValidatorUtil {
public static final javax.validation.Validator VALIDATOR;
static {
ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
VALIDATOR = new DropwizardConfiguredValidator(factory.getValidator());
}
private ValidatorUtil() {
// Private constructor for a utility class
}
public static <T> String validate(T entity) {
Set<ConstraintViolation<T>> violations = VALIDATOR.validate(entity);
String ret =
violations.isEmpty()
? null
: Arrays.toString(
violations.stream().map(v -> String.format("%s %s", v.getPropertyPath(), v.getMessage())).toArray());
return ret;
}
}

View File

@ -335,13 +335,21 @@ public class GlossaryResourceTest extends EntityResourceTest<Glossary, CreateGlo
String glossaryName = "invalidCsv"; String glossaryName = "invalidCsv";
createEntity(createRequest(glossaryName), ADMIN_AUTH_HEADERS); createEntity(createRequest(glossaryName), ADMIN_AUTH_HEADERS);
// Create glossaryTerm with invalid parent // Create glossaryTerm with invalid name (due to ::)
String resultsHeader = recordToString(EntityCsv.getResultHeaders(GlossaryCsv.HEADERS)); String resultsHeader = recordToString(EntityCsv.getResultHeaders(GlossaryCsv.HEADERS));
String record = "invalidParent,g1,dsp1,dsc1,h1;h2;h3,,term1;http://term1,Tier.Tier1,,,"; String record = ",g::1,dsp1,dsc1,,,,,,,";
String csv = createCsv(GlossaryCsv.HEADERS, listOf(record), null); String csv = createCsv(GlossaryCsv.HEADERS, listOf(record), null);
CsvImportResult result = importCsv(glossaryName, csv, false); CsvImportResult result = importCsv(glossaryName, csv, false);
assertSummary(result, CsvImportResult.Status.FAILURE, 2, 1, 1); assertSummary(result, CsvImportResult.Status.FAILURE, 2, 1, 1);
String[] expectedRows = {resultsHeader, getFailedRecord(record, entityNotFound(0, "invalidParent"))}; String[] expectedRows = {resultsHeader, getFailedRecord(record, "[name must match \"\"^(?U)[\\w'\\- .&()]+$\"\"]")};
assertRows(result, expectedRows);
// Create glossaryTerm with invalid parent
record = "invalidParent,g1,dsp1,dsc1,h1;h2;h3,,term1;http://term1,Tier.Tier1,,,";
csv = createCsv(GlossaryCsv.HEADERS, listOf(record), null);
result = importCsv(glossaryName, csv, false);
assertSummary(result, CsvImportResult.Status.FAILURE, 2, 1, 1);
expectedRows = new String[] {resultsHeader, getFailedRecord(record, entityNotFound(0, "invalidParent"))};
assertRows(result, expectedRows); assertRows(result, expectedRows);
// Create glossaryTerm with invalid tags field // Create glossaryTerm with invalid tags field

View File

@ -963,13 +963,22 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
// Headers - name,displayName,description,email,timezone,isAdmin,teams,roles // Headers - name,displayName,description,email,timezone,isAdmin,teams,roles
Team team = TEAM_TEST.createEntity(TEAM_TEST.createRequest("team-invalidCsv"), ADMIN_AUTH_HEADERS); Team team = TEAM_TEST.createEntity(TEAM_TEST.createRequest("team-invalidCsv"), ADMIN_AUTH_HEADERS);
// Invalid team // Invalid user name with "::"
String resultsHeader = recordToString(EntityCsv.getResultHeaders(UserCsv.HEADERS)); String resultsHeader = recordToString(EntityCsv.getResultHeaders(UserCsv.HEADERS));
String record = "user,,,user@domain.com,,,invalidTeam,"; String record = "invalid::User,,,user@domain.com,,,team-invalidCsv,";
String csv = createCsv(UserCsv.HEADERS, listOf(record), null); String csv = createCsv(UserCsv.HEADERS, listOf(record), null);
CsvImportResult result = importCsv(team.getName(), csv, false); CsvImportResult result = importCsv(team.getName(), csv, false);
assertSummary(result, CsvImportResult.Status.FAILURE, 2, 1, 1); assertSummary(result, CsvImportResult.Status.FAILURE, 2, 1, 1);
String[] expectedRows = {resultsHeader, getFailedRecord(record, EntityCsv.entityNotFound(6, "invalidTeam"))}; String[] expectedRows = {resultsHeader, getFailedRecord(record, "[name must match \"\"^(?U)[\\w\\-.]+$\"\"]")};
assertRows(result, expectedRows);
// Invalid team
resultsHeader = recordToString(EntityCsv.getResultHeaders(UserCsv.HEADERS));
record = "user,,,user@domain.com,,,invalidTeam,";
csv = createCsv(UserCsv.HEADERS, listOf(record), null);
result = importCsv(team.getName(), csv, false);
assertSummary(result, CsvImportResult.Status.FAILURE, 2, 1, 1);
expectedRows = new String[] {resultsHeader, getFailedRecord(record, EntityCsv.entityNotFound(6, "invalidTeam"))};
assertRows(result, expectedRows); assertRows(result, expectedRows);
// Invalid roles // Invalid roles

View File

@ -0,0 +1,31 @@
package org.openmetadata.service.util;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import java.util.UUID;
import org.junit.jupiter.api.Test;
import org.openmetadata.schema.entity.data.Glossary;
class ValidatorUtilTest {
@Test
void testValidator() {
// Required parameters name, description, and id missing
Glossary glossary = new Glossary().withName("name").withDescription("description");
assertEquals("[id must not be null]", ValidatorUtil.validate(glossary));
glossary.withId(UUID.randomUUID()).withName(null);
assertEquals("[name must not be null]", ValidatorUtil.validate(glossary));
glossary.withName("name").withDescription(null);
assertEquals("[description must not be null]", ValidatorUtil.validate(glossary));
// Invalid name
glossary.withName("invalid::Name").withDescription("description");
assertEquals("[name must match \"^(?U)[\\w'\\- .&()]+$\"]", ValidatorUtil.validate(glossary));
// No error
glossary.withName("validName").withId(UUID.randomUUID()).withDescription("description");
assertNull(ValidatorUtil.validate(glossary));
}
}

View File

@ -42,6 +42,6 @@
"default": null "default": null
} }
}, },
"required": ["id", "name", "description", "domain","href"], "required": ["id", "name", "description", "domain"],
"additionalProperties": false "additionalProperties": false
} }

View File

@ -45,6 +45,6 @@
"default": null "default": null
} }
}, },
"required": ["id", "name", "description", "domainType","href"], "required": ["id", "name", "description", "domainType"],
"additionalProperties": false "additionalProperties": false
} }

View File

@ -61,6 +61,6 @@
"$ref": "../../type/entityHistory.json#/definitions/changeDescription" "$ref": "../../type/entityHistory.json#/definitions/changeDescription"
} }
}, },
"required": ["id", "name", "description", "domain", "href"], "required": ["id", "name", "description", "domain"],
"additionalProperties": false "additionalProperties": false
} }

View File

@ -80,6 +80,6 @@
"$ref": "../../type/entityHistory.json#/definitions/changeDescription" "$ref": "../../type/entityHistory.json#/definitions/changeDescription"
} }
}, },
"required": ["id", "name", "description", "domainType, ","href"], "required": ["id", "name", "description", "domainType"],
"additionalProperties": false "additionalProperties": false
} }

View File

@ -129,6 +129,6 @@
"$ref": "../../type/entityReference.json" "$ref": "../../type/entityReference.json"
} }
}, },
"required": ["id", "name", "href"], "required": ["id", "name"],
"additionalProperties": false "additionalProperties": false
} }

View File

@ -139,5 +139,5 @@
} }
}, },
"additionalProperties": false, "additionalProperties": false,
"required": ["id", "name", "email", "href"] "required": ["id", "name", "email"]
} }