diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java index e047c4db7b7..48591255538 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java @@ -364,6 +364,9 @@ public interface CollectionDAO { @SqlUpdate("DELETE FROM entity_extension WHERE id = :id AND extension = :extension") void delete(@Bind("id") String id, @Bind("extension") String extension); + @SqlUpdate("DELETE FROM entity_extension WHERE extension = :extension") + void deleteExtension(@Bind("extension") String extension); + @SqlUpdate("DELETE FROM entity_extension WHERE id = :id") void deleteAll(@Bind("id") String id); } 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 3aa7e1af273..6ec59659478 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 @@ -1334,7 +1334,7 @@ public abstract class EntityRepository { for (K U : updatedList) { // If an entry in the updated list is not in original list, then it is added during update K stored = origList.stream().filter(c -> typeMatch.test(c, U)).findAny().orElse(null); - if (stored == null) { // New column added + if (stored == null) { // New entry added addedItems.add(U); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java index b798bb0f92e..6c26cfd1c6f 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java @@ -17,6 +17,9 @@ package org.openmetadata.service.jdbi3; import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; +import static org.openmetadata.service.Entity.FIELD_DESCRIPTION; +import static org.openmetadata.service.util.EntityUtil.customFieldMatch; +import static org.openmetadata.service.util.EntityUtil.getCustomField; import com.fasterxml.jackson.core.JsonProcessingException; import java.io.IOException; @@ -105,7 +108,16 @@ public class TypeRepository extends EntityRepository { setFields(type, putFields); dao.findEntityById(property.getPropertyType().getId()); // Validate customProperty type exists - type.getCustomProperties().add(property); + + // If property already exists, then update it. Else add the new property. + List updatedProperties = new ArrayList<>(List.of(property)); + for (CustomProperty existing : type.getCustomProperties()) { + if (!existing.getName().equals(property.getName())) { + updatedProperties.add(existing); + } + } + + type.setCustomProperties(updatedProperties); type.setUpdatedBy(updatedBy); type.setUpdatedAt(System.currentTimeMillis()); return createOrUpdate(uriInfo, type); @@ -146,7 +158,7 @@ public class TypeRepository extends EntityRepository { List origFields = listOrEmpty(original.getCustomProperties()); List added = new ArrayList<>(); List deleted = new ArrayList<>(); - recordListChange("customProperties", origFields, updatedFields, added, deleted, EntityUtil.customFieldMatch); + recordListChange("customProperties", origFields, updatedFields, added, deleted, customFieldMatch); for (CustomProperty property : added) { String customPropertyFQN = getCustomPropertyFQN(updated.getName(), property.getName()); String customPropertyJson = JsonUtils.pojoToJson(property); @@ -180,7 +192,27 @@ public class TypeRepository extends EntityRepository { Entity.TYPE, Entity.TYPE, Relationship.HAS.ordinal()); + // Delete all the data stored in the entity extension for the custom property + daoCollection.entityExtensionDAO().deleteExtension(customPropertyFQN); } + + // Record changes to updated custome properties (only description can be updated) + for (CustomProperty updated : updatedFields) { + // Find property that matches name and type + CustomProperty stored = + origFields.stream().filter(c -> customFieldMatch.test(c, updated)).findAny().orElse(null); + if (stored == null) { // New property added + continue; + } + + updateCustomPropertyDescription(stored, updated); + } + } + + private void updateCustomPropertyDescription(CustomProperty orig, CustomProperty updated) + throws JsonProcessingException { + String fieldName = getCustomField(orig, FIELD_DESCRIPTION); + recordChange(fieldName, orig.getDescription(), updated.getDescription()); } } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java index e5ed07d5a32..5fa6f7bf411 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java @@ -185,7 +185,7 @@ public abstract class EntityResource response = dao.delete(securityContext.getUserPrincipal().getName(), id, recursive, hardDelete); addHref(uriInfo, response.getEntity()); return response.toResponse(); @@ -200,7 +200,7 @@ public abstract class EntityResource response = dao.deleteByName(securityContext.getUserPrincipal().getName(), name, recursive, hardDelete); addHref(uriInfo, response.getEntity()); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/types/TypeResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/types/TypeResource.java index 33d90dc48fa..e2a70792949 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/types/TypeResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/types/TypeResource.java @@ -373,15 +373,16 @@ public class TypeResource extends EntityResource { @Path("/{id}") @Operation( operationId = "addProperty", - summary = "Add a Property to an entity", + summary = "Add or update a Property to an entity", tags = "metadata", description = - "Add a property to an entity type. Properties can only be added to entity type and not property type.", + "Add or update a property to an entity type. " + + "Properties can only be added to entity type and not property type.", responses = { @ApiResponse(responseCode = "200", description = "OK"), @ApiResponse(responseCode = "404", description = "type for instance {id} is not found") }) - public Response addProperty( + public Response addOrUpdateProperty( @Context UriInfo uriInfo, @Context SecurityContext securityContext, @Parameter(description = "Type Id", schema = @Schema(type = "string")) @PathParam("id") String id, diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java index 594c6af19e6..afd3f36f262 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java @@ -141,7 +141,9 @@ public final class EntityUtil { (ref1, ref2) -> ref1.getName().equals(ref2.getName()) && ref1.getEndpoint().equals(ref2.getEndpoint()); public static final BiPredicate customFieldMatch = - (ref1, ref2) -> ref1.getName().equals(ref2.getName()); + (ref1, ref2) -> + ref1.getName().equals(ref2.getName()) + && entityReferenceMatch.test(ref1.getPropertyType(), ref2.getPropertyType()); public static final BiPredicate ruleMatch = (ref1, ref2) -> ref1.getName().equals(ref2.getName()); @@ -386,6 +388,13 @@ public final class EntityUtil { : FullyQualifiedName.build("rules", rule.getName(), ruleField); } + /** Return customer property field name of format "extension".propertyName */ + public static String getCustomField(CustomProperty property, String propertyFieldName) { + return propertyFieldName == null + ? FullyQualifiedName.build("customProperties", property.getName()) + : FullyQualifiedName.build("customProperties", property.getName(), propertyFieldName); + } + public static Double nextVersion(Double version) { return Math.round((version + 0.1) * 10.0) / 10.0; } 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 6c776036a42..d71654b84d5 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 @@ -316,6 +316,7 @@ public abstract class EntityResourceTest authHeaders) throws IOException { // GET ../entity/{id}/versions to list the all the versions of an entity @@ -1696,7 +1695,7 @@ public abstract class EntityResourceTest { @BeforeAll public void setup(TestInfo test) throws IOException, URISyntaxException { super.setup(test); + } + + public void setupTypes() throws HttpResponseException { INT_TYPE = getEntityByName("integer", "", ADMIN_AUTH_HEADERS); + STRING_TYPE = getEntityByName("string", "", ADMIN_AUTH_HEADERS); } @Override @@ -83,16 +95,54 @@ public class TypeResourceTest extends EntityResourceTest { } @Test - void put_customProperty_200() throws HttpResponseException { + void put_patch_customProperty_200() throws IOException { Type tableEntity = getEntityByName("table", "customProperties", ADMIN_AUTH_HEADERS); assertTrue(listOrEmpty(tableEntity.getCustomProperties()).isEmpty()); // Add a custom property with name intA with type integer CustomProperty fieldA = new CustomProperty().withName("intA").withDescription("intA").withPropertyType(INT_TYPE.getEntityReference()); - tableEntity = addCustomProperty(tableEntity.getId(), fieldA, Status.OK, ADMIN_AUTH_HEADERS); - assertEquals(1, tableEntity.getCustomProperties().size()); - assertEquals(fieldA, tableEntity.getCustomProperties().get(0)); + ChangeDescription change = getChangeDescription(tableEntity.getVersion()); + fieldAdded(change, "customProperties", new ArrayList<>(List.of(fieldA))); + tableEntity = addCustomPropertyAndCheck(tableEntity.getId(), fieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + assertCustomProperties(new ArrayList<>(List.of(fieldA)), tableEntity.getCustomProperties()); + + // Changing custom property description with PUT + fieldA.withDescription("updated"); + change = getChangeDescription(tableEntity.getVersion()); + fieldUpdated(change, EntityUtil.getCustomField(fieldA, "description"), "intA", "updated"); + tableEntity = addCustomPropertyAndCheck(tableEntity.getId(), fieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + assertCustomProperties(new ArrayList<>(List.of(fieldA)), tableEntity.getCustomProperties()); + + // Changing property type with PUT - old property deleted and new customer property of the same name added + fieldA.withPropertyType(STRING_TYPE.getEntityReference()); + change = getChangeDescription(tableEntity.getVersion()); + fieldDeleted(change, "customProperties", tableEntity.getCustomProperties()); + fieldAdded(change, "customProperties", new ArrayList<>(List.of(fieldA))); + tableEntity = addCustomPropertyAndCheck(tableEntity.getId(), fieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + assertCustomProperties(new ArrayList<>(List.of(fieldA)), tableEntity.getCustomProperties()); + + // Changing custom property description with PATCH + fieldA.withDescription("updated2"); + String json = JsonUtils.pojoToJson(tableEntity); + tableEntity.setCustomProperties(List.of(fieldA)); + change = getChangeDescription(tableEntity.getVersion()); + fieldUpdated(change, EntityUtil.getCustomField(fieldA, "description"), "updated", "updated2"); + tableEntity = patchEntityAndCheck(tableEntity, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + + // Changing property type with PATCH - old property deleted and new customer property of the same name added + CustomProperty fieldA1 = + new CustomProperty() + .withDescription(fieldA.getDescription()) + .withPropertyType(INT_TYPE.getEntityReference()) + .withName(fieldA.getName()); + json = JsonUtils.pojoToJson(tableEntity); + tableEntity.setCustomProperties(new ArrayList<>(List.of(fieldA1))); + change = getChangeDescription(tableEntity.getVersion()); + fieldDeleted(change, "customProperties", new ArrayList<>(List.of(fieldA))); + fieldAdded(change, "customProperties", new ArrayList<>(List.of(fieldA1))); + tableEntity = patchEntityAndCheck(tableEntity, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + assertCustomProperties(new ArrayList<>(List.of(fieldA1)), tableEntity.getCustomProperties()); // Add a second property with name intB with type integer EntityReference typeRef = @@ -100,11 +150,12 @@ public class TypeResourceTest extends EntityResourceTest { .withType(INT_TYPE.getEntityReference().getType()) .withId(INT_TYPE.getEntityReference().getId()); CustomProperty fieldB = new CustomProperty().withName("intB").withDescription("intB").withPropertyType(typeRef); - tableEntity = addCustomProperty(tableEntity.getId(), fieldB, Status.OK, ADMIN_AUTH_HEADERS); + change = getChangeDescription(tableEntity.getVersion()); + fieldAdded(change, "customProperties", new ArrayList<>(List.of(fieldB))); + tableEntity = addCustomPropertyAndCheck(tableEntity.getId(), fieldB, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); fieldB.setPropertyType(INT_TYPE.getEntityReference()); assertEquals(2, tableEntity.getCustomProperties().size()); - assertEquals(fieldA, tableEntity.getCustomProperties().get(0)); - assertEquals(fieldB, tableEntity.getCustomProperties().get(1)); + assertCustomProperties(new ArrayList<>(List.of(fieldA1, fieldB)), tableEntity.getCustomProperties()); } @Test @@ -139,6 +190,20 @@ public class TypeResourceTest extends EntityResourceTest { return updated; } + public Type addCustomPropertyAndCheck( + UUID entityTypeId, + CustomProperty customProperty, + Map authHeaders, + UpdateType updateType, + ChangeDescription expectedChange) + throws IOException { + Type returned = addCustomProperty(entityTypeId, customProperty, Status.OK, authHeaders); + validateChangeDescription(returned, updateType, expectedChange); + validateEntityHistory(returned.getId(), updateType, expectedChange, authHeaders); + validateLatestVersion(returned, updateType, expectedChange, authHeaders); + return returned; + } + public Type addCustomProperty( UUID entityTypeId, CustomProperty customProperty, Status status, Map authHeaders) throws HttpResponseException { @@ -176,10 +241,7 @@ public class TypeResourceTest extends EntityResourceTest { if (fieldName.equals("customProperties")) { List expectedProperties = (List) expected; List actualProperties = JsonUtils.readObjects(actual.toString(), CustomProperty.class); - assertEquals(expectedProperties.size(), actualProperties.size()); - expectedProperties.sort(EntityUtil.compareCustomProperty); - actualProperties.sort(EntityUtil.compareCustomProperty); - assertEquals(expectedProperties, actualProperties); + TestUtils.assertCustomProperties(expectedProperties, actualProperties); } else { assertCommonFieldChange(fieldName, expected, actual); } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java b/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java index 405f2392840..d3cca09c23d 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java @@ -50,6 +50,7 @@ import org.openmetadata.schema.api.services.DatabaseConnection; import org.openmetadata.schema.entity.data.GlossaryTerm; import org.openmetadata.schema.entity.tags.Tag; import org.openmetadata.schema.entity.teams.User; +import org.openmetadata.schema.entity.type.CustomProperty; import org.openmetadata.schema.security.credentials.AWSCredentials; import org.openmetadata.schema.services.connections.dashboard.SupersetConnection; import org.openmetadata.schema.services.connections.database.BigQueryConnection; @@ -104,6 +105,23 @@ public final class TestUtils { public static URI PIPELINE_URL; + public static void assertCustomProperties(List expected, List actual) { + if (expected == actual) { // Take care of both being null + return; + } + if (expected != null) { + actual = listOrEmpty(actual); + expected.sort(EntityUtil.compareCustomProperty); + actual.sort(EntityUtil.compareCustomProperty); + assertEquals(expected.size(), actual.size()); + for (int i = 0; i < expected.size(); i++) { + assertEquals(expected.get(i).getName(), actual.get(i).getName()); + assertEquals(expected.get(i).getPropertyType().getId(), actual.get(i).getPropertyType().getId()); + assertEquals(expected.get(i).getPropertyType().getType(), actual.get(i).getPropertyType().getType()); + } + } + } + public enum UpdateType { CREATED, // Not updated instead entity was created NO_CHANGE, // PUT/PATCH made no change