From ad6c82367530d472bd96c9f51cdd5d4a58de7e42 Mon Sep 17 00:00:00 2001 From: Teddy Date: Wed, 20 Mar 2024 07:11:29 +0100 Subject: [PATCH] [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 --- .../service/jdbi3/EntityRepository.java | 23 +++++++++ .../service/resources/EntityResourceTest.java | 47 +++++++++++++++++++ .../domains/DataProductResourceTest.java | 20 ++++++++ .../resources/domains/DomainResourceTest.java | 39 +++++++++++++++ .../glossary/GlossaryTermResourceTest.java | 19 ++++++++ 5 files changed, 148 insertions(+) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java index 279824467dd..d87457519ce 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java @@ -1907,6 +1907,25 @@ public abstract class EntityRepository { 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 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 */ public String exportToCsv(String name, String user) throws IOException { throw new IllegalArgumentException(csvNotSupported(entityType)); @@ -2263,6 +2282,7 @@ public abstract class EntityRepository { origDomain.getId(), Entity.DOMAIN, original.getId(), entityType, Relationship.HAS); } if (updatedDomain != null) { + validateDomain(updatedDomain); // Add relationship owner --- owns ---> ownedEntity LOG.info( "Adding domain {} for entity {}", @@ -2283,6 +2303,7 @@ public abstract class EntityRepository { } List origDataProducts = listOrEmpty(original.getDataProducts()); List updatedDataProducts = listOrEmpty(updated.getDataProducts()); + validateDataProducts(updatedDataProducts); updateFromRelationships( FIELD_DATA_PRODUCTS, DATA_PRODUCT, @@ -2299,6 +2320,7 @@ public abstract class EntityRepository { } List origExperts = getEntityReferences(original.getExperts()); List updatedExperts = getEntityReferences(updated.getExperts()); + validateUsers(updatedExperts); updateToRelationships( FIELD_EXPERTS, entityType, @@ -2317,6 +2339,7 @@ public abstract class EntityRepository { } List origReviewers = getEntityReferences(original.getReviewers()); List updatedReviewers = getEntityReferences(updated.getReviewers()); + validateUsers(updatedReviewers); updateFromRelationships( "reviewers", Entity.USER, diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java index 80edd6c1384..bb88983337e 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java @@ -103,6 +103,7 @@ import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; @@ -231,6 +232,10 @@ public abstract class EntityResourceTest 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 @Execution(ExecutionMode.CONCURRENT) protected void get_entityListWithPagination_200(TestInfo test) throws IOException { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DataProductResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DataProductResourceTest.java index 1f1e74d54f2..7e69d726ef8 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DataProductResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DataProductResourceTest.java @@ -1,9 +1,11 @@ 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.assertTrue; import static org.openmetadata.common.utils.CommonUtil.listOf; 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.fieldDeleted; import static org.openmetadata.service.util.TestUtils.*; @@ -14,6 +16,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import javax.ws.rs.core.Response.Status; import org.apache.http.client.HttpResponseException; 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.type.Style; import org.openmetadata.schema.type.ChangeDescription; +import org.openmetadata.schema.type.EntityReference; 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.databases.TableResourceTest; import org.openmetadata.service.resources.domains.DataProductResource.DataProductList; @@ -160,6 +166,20 @@ public class DataProductResourceTest extends EntityResourceTest 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( EntityInterface entity, EntityInterface product, boolean inDataProduct) throws HttpResponseException { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DomainResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DomainResourceTest.java index f416213aa73..9f55c8451db 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DomainResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/domains/DomainResourceTest.java @@ -1,7 +1,10 @@ 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.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.util.EntityUtil.fieldAdded; 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.assertListNotNull; import static org.openmetadata.service.util.TestUtils.assertListNull; +import static org.openmetadata.service.util.TestUtils.assertResponse; import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.UUID; import javax.ws.rs.core.Response.Status; import org.apache.http.client.HttpResponseException; 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.type.Style; import org.openmetadata.schema.type.ChangeDescription; +import org.openmetadata.schema.type.EntityReference; 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.domains.DomainResource.DomainList; import org.openmetadata.service.util.JsonUtils; @@ -113,6 +121,37 @@ public class DomainResourceTest extends EntityResourceTest 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 public CreateDomain createRequest(String name) { return new CreateDomain() diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java index 201d7d344e3..d9489c0ec39 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java @@ -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.FORBIDDEN; +import static javax.ws.rs.core.Response.Status.NOT_FOUND; import static org.junit.jupiter.api.Assertions.*; import static org.openmetadata.common.utils.CommonUtil.listOf; import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; @@ -53,6 +54,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.UUID; import javax.ws.rs.core.Response; import org.apache.http.client.HttpResponseException; import org.junit.jupiter.api.MethodOrderer; @@ -644,6 +646,23 @@ public class GlossaryTermResourceTest extends EntityResourceTest 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) throws IOException { return createTerm(glossary, parent, termName, glossary.getReviewers());