[MINOR] validate domain on PATCH request (#15556)

* fix: add domain validation on PATCH update

* style: apply java linting

* fix: added validation for DP, Experts and Reviewers

* style: ran java linting

* fix: skip domain patch test for data products

* style: ran java linting
This commit is contained in:
Teddy 2024-03-20 07:11:29 +01:00 committed by GitHub
parent e06e5c1bdd
commit ad6c823675
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 148 additions and 0 deletions

View File

@ -1907,6 +1907,25 @@ public abstract class EntityRepository<T extends EntityInterface> {
return Entity.getEntityReferenceByName(Entity.DOMAIN, domainFqn, NON_DELETED); return Entity.getEntityReferenceByName(Entity.DOMAIN, domainFqn, NON_DELETED);
} }
public final void validateDomain(EntityReference domain) {
if (!supportsDomain) {
throw new IllegalArgumentException(CatalogExceptionMessage.invalidField(FIELD_DOMAIN));
}
Entity.getEntityReferenceById(Entity.DOMAIN, domain.getId(), NON_DELETED);
}
public final void validateDataProducts(List<EntityReference> dataProducts) {
if (!supportsDataProducts) {
throw new IllegalArgumentException(CatalogExceptionMessage.invalidField(FIELD_DATA_PRODUCTS));
}
if (!nullOrEmpty(dataProducts)) {
for (EntityReference dataProduct : dataProducts) {
Entity.getEntityReferenceById(Entity.DATA_PRODUCT, dataProduct.getId(), NON_DELETED);
}
}
}
/** Override this method to support downloading CSV functionality */ /** Override this method to support downloading CSV functionality */
public String exportToCsv(String name, String user) throws IOException { public String exportToCsv(String name, String user) throws IOException {
throw new IllegalArgumentException(csvNotSupported(entityType)); throw new IllegalArgumentException(csvNotSupported(entityType));
@ -2263,6 +2282,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
origDomain.getId(), Entity.DOMAIN, original.getId(), entityType, Relationship.HAS); origDomain.getId(), Entity.DOMAIN, original.getId(), entityType, Relationship.HAS);
} }
if (updatedDomain != null) { if (updatedDomain != null) {
validateDomain(updatedDomain);
// Add relationship owner --- owns ---> ownedEntity // Add relationship owner --- owns ---> ownedEntity
LOG.info( LOG.info(
"Adding domain {} for entity {}", "Adding domain {} for entity {}",
@ -2283,6 +2303,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
} }
List<EntityReference> origDataProducts = listOrEmpty(original.getDataProducts()); List<EntityReference> origDataProducts = listOrEmpty(original.getDataProducts());
List<EntityReference> updatedDataProducts = listOrEmpty(updated.getDataProducts()); List<EntityReference> updatedDataProducts = listOrEmpty(updated.getDataProducts());
validateDataProducts(updatedDataProducts);
updateFromRelationships( updateFromRelationships(
FIELD_DATA_PRODUCTS, FIELD_DATA_PRODUCTS,
DATA_PRODUCT, DATA_PRODUCT,
@ -2299,6 +2320,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
} }
List<EntityReference> origExperts = getEntityReferences(original.getExperts()); List<EntityReference> origExperts = getEntityReferences(original.getExperts());
List<EntityReference> updatedExperts = getEntityReferences(updated.getExperts()); List<EntityReference> updatedExperts = getEntityReferences(updated.getExperts());
validateUsers(updatedExperts);
updateToRelationships( updateToRelationships(
FIELD_EXPERTS, FIELD_EXPERTS,
entityType, entityType,
@ -2317,6 +2339,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
} }
List<EntityReference> origReviewers = getEntityReferences(original.getReviewers()); List<EntityReference> origReviewers = getEntityReferences(original.getReviewers());
List<EntityReference> updatedReviewers = getEntityReferences(updated.getReviewers()); List<EntityReference> updatedReviewers = getEntityReferences(updated.getReviewers());
validateUsers(updatedReviewers);
updateFromRelationships( updateFromRelationships(
"reviewers", "reviewers",
Entity.USER, Entity.USER,

View File

@ -103,6 +103,7 @@ import org.json.JSONArray;
import org.json.JSONException; import org.json.JSONException;
import org.json.JSONObject; import org.json.JSONObject;
import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestInfo;
@ -231,6 +232,10 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
protected final boolean supportsCustomExtension; protected final boolean supportsCustomExtension;
protected final boolean supportsLifeCycle; protected final boolean supportsLifeCycle;
protected final boolean supportsDomain;
protected final boolean supportsDataProducts;
protected final boolean supportsExperts;
protected final boolean supportsReviewers;
public static final String DATA_STEWARD_ROLE_NAME = "DataSteward"; public static final String DATA_STEWARD_ROLE_NAME = "DataSteward";
public static final String DATA_CONSUMER_ROLE_NAME = "DataConsumer"; public static final String DATA_CONSUMER_ROLE_NAME = "DataConsumer";
@ -406,6 +411,10 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
this.supportsCustomExtension = allowedFields.contains(FIELD_EXTENSION); this.supportsCustomExtension = allowedFields.contains(FIELD_EXTENSION);
this.supportsLifeCycle = allowedFields.contains(FIELD_LIFE_CYCLE); this.supportsLifeCycle = allowedFields.contains(FIELD_LIFE_CYCLE);
this.systemEntityName = systemEntityName; this.systemEntityName = systemEntityName;
this.supportsDomain = allowedFields.contains(FIELD_DOMAIN);
this.supportsDataProducts = allowedFields.contains(FIELD_DATA_PRODUCTS);
this.supportsExperts = allowedFields.contains(FIELD_EXPERTS);
this.supportsReviewers = allowedFields.contains(FIELD_REVIEWERS);
} }
@BeforeAll @BeforeAll
@ -593,6 +602,44 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
"Invalid field name invalidField"); "Invalid field name invalidField");
} }
@Test
void patchWrongDomainId(TestInfo test) throws IOException {
Assumptions.assumeTrue(supportsDomain);
T entity = createEntity(createRequest(test, 0), ADMIN_AUTH_HEADERS);
// Data Product domain cannot be modified see DataProductRepository.restorePatchAttributes
Assumptions.assumeTrue(!(entity.getEntityReference().getType().equals(DATA_PRODUCT)));
// Add random domain reference
EntityReference domainReference =
new EntityReference().withId(UUID.randomUUID()).withType(Entity.DOMAIN);
String originalJson = JsonUtils.pojoToJson(entity);
ChangeDescription change = getChangeDescription(entity, MINOR_UPDATE);
entity.setDomain(domainReference);
assertResponse(
() -> patchEntityAndCheck(entity, originalJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change),
NOT_FOUND,
String.format("domain instance for %s not found", domainReference.getId()));
}
@Test
@Execution(ExecutionMode.CONCURRENT)
void patchWrongDataProducts(TestInfo test) throws IOException {
Assumptions.assumeTrue(supportsDataProducts);
T entity = createEntity(createRequest(test, 0), ADMIN_AUTH_HEADERS);
// Add random domain reference
EntityReference dataProductReference = new EntityReference().withId(UUID.randomUUID());
String originalJson = JsonUtils.pojoToJson(entity);
ChangeDescription change = getChangeDescription(entity, MINOR_UPDATE);
entity.setDataProducts(List.of(dataProductReference));
assertResponse(
() -> patchEntityAndCheck(entity, originalJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change),
NOT_FOUND,
String.format("dataProduct instance for %s not found", dataProductReference.getId()));
}
@Test @Test
@Execution(ExecutionMode.CONCURRENT) @Execution(ExecutionMode.CONCURRENT)
protected void get_entityListWithPagination_200(TestInfo test) throws IOException { protected void get_entityListWithPagination_200(TestInfo test) throws IOException {

View File

@ -1,9 +1,11 @@
package org.openmetadata.service.resources.domains; package org.openmetadata.service.resources.domains;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.openmetadata.common.utils.CommonUtil.listOf; import static org.openmetadata.common.utils.CommonUtil.listOf;
import static org.openmetadata.service.Entity.FIELD_ASSETS; import static org.openmetadata.service.Entity.FIELD_ASSETS;
import static org.openmetadata.service.Entity.TABLE;
import static org.openmetadata.service.util.EntityUtil.fieldAdded; import static org.openmetadata.service.util.EntityUtil.fieldAdded;
import static org.openmetadata.service.util.EntityUtil.fieldDeleted; import static org.openmetadata.service.util.EntityUtil.fieldDeleted;
import static org.openmetadata.service.util.TestUtils.*; import static org.openmetadata.service.util.TestUtils.*;
@ -14,6 +16,7 @@ import java.io.IOException;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.UUID;
import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.Response.Status;
import org.apache.http.client.HttpResponseException; import org.apache.http.client.HttpResponseException;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -24,7 +27,10 @@ import org.openmetadata.schema.entity.data.Topic;
import org.openmetadata.schema.entity.domains.DataProduct; import org.openmetadata.schema.entity.domains.DataProduct;
import org.openmetadata.schema.entity.type.Style; import org.openmetadata.schema.entity.type.Style;
import org.openmetadata.schema.type.ChangeDescription; import org.openmetadata.schema.type.ChangeDescription;
import org.openmetadata.schema.type.EntityReference;
import org.openmetadata.service.Entity; import org.openmetadata.service.Entity;
import org.openmetadata.service.exception.EntityNotFoundException;
import org.openmetadata.service.jdbi3.TableRepository;
import org.openmetadata.service.resources.EntityResourceTest; import org.openmetadata.service.resources.EntityResourceTest;
import org.openmetadata.service.resources.databases.TableResourceTest; import org.openmetadata.service.resources.databases.TableResourceTest;
import org.openmetadata.service.resources.domains.DataProductResource.DataProductList; import org.openmetadata.service.resources.domains.DataProductResource.DataProductList;
@ -160,6 +166,20 @@ public class DataProductResourceTest extends EntityResourceTest<DataProduct, Cre
assertTrue(list.stream().anyMatch(s -> s.getName().equals(p4.getName()))); assertTrue(list.stream().anyMatch(s -> s.getName().equals(p4.getName())));
} }
@Test
void testValidateDataProducts() {
UUID rdnUUID = UUID.randomUUID();
EntityReference entityReference = new EntityReference().withId(rdnUUID);
TableRepository entityRepository = (TableRepository) Entity.getEntityRepository(TABLE);
assertThatThrownBy(
() -> {
entityRepository.validateDataProducts(List.of(entityReference));
})
.isInstanceOf(EntityNotFoundException.class)
.hasMessage(String.format("dataProduct instance for %s not found", rdnUUID));
}
private void entityInDataProduct( private void entityInDataProduct(
EntityInterface entity, EntityInterface product, boolean inDataProduct) EntityInterface entity, EntityInterface product, boolean inDataProduct)
throws HttpResponseException { throws HttpResponseException {

View File

@ -1,7 +1,10 @@
package org.openmetadata.service.resources.domains; package org.openmetadata.service.resources.domains;
import static javax.ws.rs.core.Response.Status.NOT_FOUND;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.openmetadata.common.utils.CommonUtil.listOf; import static org.openmetadata.common.utils.CommonUtil.listOf;
import static org.openmetadata.service.Entity.TABLE;
import static org.openmetadata.service.security.SecurityUtil.authHeaders; import static org.openmetadata.service.security.SecurityUtil.authHeaders;
import static org.openmetadata.service.util.EntityUtil.fieldAdded; import static org.openmetadata.service.util.EntityUtil.fieldAdded;
import static org.openmetadata.service.util.EntityUtil.fieldDeleted; import static org.openmetadata.service.util.EntityUtil.fieldDeleted;
@ -13,10 +16,12 @@ import static org.openmetadata.service.util.TestUtils.UpdateType.REVERT;
import static org.openmetadata.service.util.TestUtils.assertEntityReferenceNames; import static org.openmetadata.service.util.TestUtils.assertEntityReferenceNames;
import static org.openmetadata.service.util.TestUtils.assertListNotNull; import static org.openmetadata.service.util.TestUtils.assertListNotNull;
import static org.openmetadata.service.util.TestUtils.assertListNull; import static org.openmetadata.service.util.TestUtils.assertListNull;
import static org.openmetadata.service.util.TestUtils.assertResponse;
import java.io.IOException; import java.io.IOException;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.UUID;
import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.Response.Status;
import org.apache.http.client.HttpResponseException; import org.apache.http.client.HttpResponseException;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -26,7 +31,10 @@ import org.openmetadata.schema.api.domains.CreateDomain.DomainType;
import org.openmetadata.schema.entity.domains.Domain; import org.openmetadata.schema.entity.domains.Domain;
import org.openmetadata.schema.entity.type.Style; import org.openmetadata.schema.entity.type.Style;
import org.openmetadata.schema.type.ChangeDescription; import org.openmetadata.schema.type.ChangeDescription;
import org.openmetadata.schema.type.EntityReference;
import org.openmetadata.service.Entity; import org.openmetadata.service.Entity;
import org.openmetadata.service.exception.EntityNotFoundException;
import org.openmetadata.service.jdbi3.TableRepository;
import org.openmetadata.service.resources.EntityResourceTest; import org.openmetadata.service.resources.EntityResourceTest;
import org.openmetadata.service.resources.domains.DomainResource.DomainList; import org.openmetadata.service.resources.domains.DomainResource.DomainList;
import org.openmetadata.service.util.JsonUtils; import org.openmetadata.service.util.JsonUtils;
@ -113,6 +121,37 @@ public class DomainResourceTest extends EntityResourceTest<Domain, CreateDomain>
createEntity(create, authHeaders(DATA_CONSUMER.getName())); createEntity(create, authHeaders(DATA_CONSUMER.getName()));
} }
@Test
void testValidateDomain() {
UUID rdnUUID = UUID.randomUUID();
EntityReference entityReference = new EntityReference().withId(rdnUUID);
TableRepository entityRepository = (TableRepository) Entity.getEntityRepository(TABLE);
assertThatThrownBy(
() -> {
entityRepository.validateDomain(entityReference);
})
.isInstanceOf(EntityNotFoundException.class)
.hasMessage(String.format("domain instance for %s not found", rdnUUID));
}
@Test
void patchWrongExperts(TestInfo test) throws IOException {
Domain entity = createEntity(createRequest(test, 0), ADMIN_AUTH_HEADERS);
// Add random domain reference
EntityReference expertReference =
new EntityReference().withId(UUID.randomUUID()).withType(Entity.USER);
String originalJson = JsonUtils.pojoToJson(entity);
ChangeDescription change = getChangeDescription(entity, MINOR_UPDATE);
entity.setExperts(List.of(expertReference));
assertResponse(
() -> patchEntityAndCheck(entity, originalJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change),
NOT_FOUND,
String.format("user instance for %s not found", expertReference.getId()));
}
@Override @Override
public CreateDomain createRequest(String name) { public CreateDomain createRequest(String name) {
return new CreateDomain() return new CreateDomain()

View File

@ -18,6 +18,7 @@ package org.openmetadata.service.resources.glossary;
import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static javax.ws.rs.core.Response.Status.FORBIDDEN;
import static javax.ws.rs.core.Response.Status.NOT_FOUND;
import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.*;
import static org.openmetadata.common.utils.CommonUtil.listOf; import static org.openmetadata.common.utils.CommonUtil.listOf;
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
@ -53,6 +54,7 @@ import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.UUID;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import org.apache.http.client.HttpResponseException; import org.apache.http.client.HttpResponseException;
import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.MethodOrderer;
@ -644,6 +646,23 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
assertTrue(table.getTags().isEmpty()); // tag t1 is removed assertTrue(table.getTags().isEmpty()); // tag t1 is removed
} }
@Test
void patchWrongReviewers(TestInfo test) throws IOException {
GlossaryTerm entity = createEntity(createRequest(test, 0), ADMIN_AUTH_HEADERS);
// Add random domain reference
EntityReference reviewerReference =
new EntityReference().withId(UUID.randomUUID()).withType(Entity.USER);
String originalJson = JsonUtils.pojoToJson(entity);
ChangeDescription change = getChangeDescription(entity, MINOR_UPDATE);
entity.setReviewers(List.of(reviewerReference));
assertResponse(
() -> patchEntityAndCheck(entity, originalJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change),
NOT_FOUND,
String.format("user instance for %s not found", reviewerReference.getId()));
}
public GlossaryTerm createTerm(Glossary glossary, GlossaryTerm parent, String termName) public GlossaryTerm createTerm(Glossary glossary, GlossaryTerm parent, String termName)
throws IOException { throws IOException {
return createTerm(glossary, parent, termName, glossary.getReviewers()); return createTerm(glossary, parent, termName, glossary.getReviewers());