diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/TypeRegistry.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/TypeRegistry.java index 40cfb741581..4440ee66ffc 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/TypeRegistry.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/TypeRegistry.java @@ -21,6 +21,8 @@ import java.util.concurrent.ConcurrentHashMap; import lombok.extern.slf4j.Slf4j; import org.openmetadata.catalog.entity.Type; import org.openmetadata.catalog.entity.type.CustomProperty; +import org.openmetadata.catalog.exception.CatalogExceptionMessage; +import org.openmetadata.catalog.exception.EntityNotFoundException; import org.openmetadata.catalog.util.FullyQualifiedName; import org.openmetadata.catalog.util.JsonUtils; @@ -48,7 +50,6 @@ public class TypeRegistry { public void addType(Type type) { TYPES.put(type.getName(), type); - LOG.info("Updated type {}\n", type.getName()); // Store custom properties added to a type for (CustomProperty property : type.getCustomProperties()) { @@ -58,18 +59,17 @@ public class TypeRegistry { public void removeType(String typeName) { TYPES.remove(typeName); - LOG.info("Deleted type {}\n", typeName); + LOG.info("Deleted type {}", typeName); // TODO cleanup custom properties } private void addCustomProperty(String entityType, String propertyName, CustomProperty customProperty) { String customPropertyFQN = getCustomPropertyFQN(entityType, propertyName); CUSTOM_PROPERTIES.put(customPropertyFQN, customProperty); - LOG.info("Adding custom property {}\n", customPropertyFQN); JsonSchema jsonSchema = JsonUtils.getJsonSchema(TYPES.get(customProperty.getPropertyType().getName()).getSchema()); CUSTOM_PROPERTY_SCHEMAS.put(customPropertyFQN, jsonSchema); - LOG.info("Adding json schema for {} {}\n", customProperty, jsonSchema); + LOG.info("Adding custom property {} with JSON schema {}", customPropertyFQN, jsonSchema); } public JsonSchema getSchema(String entityType, String propertyName) { @@ -81,9 +81,8 @@ public class TypeRegistry { // Validate custom properties added to a type for (CustomProperty property : listOrEmpty(type.getCustomProperties())) { if (TYPES.get(property.getPropertyType().getName()) == null) { - throw new IllegalArgumentException( - String.format( - "Type %s not found for custom property %s", property.getPropertyType().getName(), property.getName())); + throw EntityNotFoundException.byMessage( + CatalogExceptionMessage.entityNotFound(Entity.TYPE, property.getPropertyType().getId())); } } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TypeRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TypeRepository.java index 028017c2438..260a2938f06 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TypeRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TypeRepository.java @@ -87,11 +87,6 @@ public class TypeRepository extends EntityRepository { TypeRegistry.instance().removeType(entity.getName()); } - @Override - protected void postUpdate(Type entity) { - TypeRegistry.instance().addType(entity); - } - @Override public EntityUpdater getUpdater(Type original, Type updated, Operation operation) { return new TypeUpdater(original, updated, operation); @@ -102,7 +97,7 @@ public class TypeRepository extends EntityRepository { Type type = dao.findEntityById(UUID.fromString(id), Include.NON_DELETED); property.setPropertyType(dao.findEntityReferenceById(property.getPropertyType().getId(), Include.NON_DELETED)); if (type.getCategory().equals(Category.Field)) { - throw new IllegalArgumentException("Property types can't be extended"); + throw new IllegalArgumentException("Only entity types can be extended and field types can't be extended"); } setFields(type, putFields); @@ -148,7 +143,7 @@ public class TypeRepository extends EntityRepository { List origFields = listOrEmpty(original.getCustomProperties()); List added = new ArrayList<>(); List deleted = new ArrayList<>(); - recordListChange("customProperty", origFields, updatedFields, added, deleted, EntityUtil.customFieldMatch); + recordListChange("customProperties", origFields, updatedFields, added, deleted, EntityUtil.customFieldMatch); for (CustomProperty property : added) { String customPropertyFQN = getCustomPropertyFQN(updated.getName(), property.getName()); String customPropertyJson = JsonUtils.pojoToJson(property); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/types/TypeResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/types/TypeResource.java index 31085b5466a..0e987b70624 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/types/TypeResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/types/TypeResource.java @@ -56,6 +56,7 @@ import org.openmetadata.catalog.CatalogApplicationConfig; import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.api.CreateType; import org.openmetadata.catalog.entity.Type; +import org.openmetadata.catalog.entity.type.Category; import org.openmetadata.catalog.entity.type.CustomProperty; import org.openmetadata.catalog.jdbi3.CollectionDAO; import org.openmetadata.catalog.jdbi3.ListFilter; @@ -66,6 +67,7 @@ import org.openmetadata.catalog.security.Authorizer; import org.openmetadata.catalog.security.SecurityUtil; import org.openmetadata.catalog.type.EntityHistory; import org.openmetadata.catalog.type.Include; +import org.openmetadata.catalog.util.EntityUtil.Fields; import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.RestUtil.PutResponse; import org.openmetadata.catalog.util.ResultList; @@ -92,7 +94,7 @@ public class TypeResource extends EntityResource { @SuppressWarnings("unused") // Method used for reflection public void initialize(CatalogApplicationConfig config) throws IOException { - // Find tag definitions and load tag categories from the json file, if necessary + // Load types defined in OpenMetadata schemas long now = System.currentTimeMillis(); List types = JsonUtils.getTypes(); types.forEach( @@ -100,6 +102,17 @@ public class TypeResource extends EntityResource { type.withId(UUID.randomUUID()).withUpdatedBy("admin").withUpdatedAt(now); LOG.info("Loading type {}", type.getName()); try { + Fields fields = getFields("customProperties"); + try { + Type storedType = dao.getByName(null, type.getName(), fields); + type.setId(storedType.getId()); + // If entity type already exists, then carry forward custom properties + if (storedType.getCategory().equals(Category.Entity)) { + type.setCustomProperties(storedType.getCustomProperties()); + } + } catch (Exception e) { + LOG.debug("Creating entity that does not exist ", e); + } this.dao.createOrUpdate(null, type); } catch (IOException e) { LOG.error("Error loading type {}", type.getName(), e); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java index 4d206a7c57c..75392e44f59 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java @@ -159,7 +159,7 @@ public abstract class EntityResourceTest typeResourceTest.addCustomProperty(entity.getId(), fieldInvalid, OK, ADMIN_AUTH_HEADERS), + () -> typeResourceTest.addCustomProperty(id, fieldInvalid, OK, ADMIN_AUTH_HEADERS), NOT_FOUND, - CatalogExceptionMessage.entityNotFound(Entity.TYPE, INVALID_TYPE.getId())); + CatalogExceptionMessage.entityNotFound(Entity.TYPE, invalidType.getId())); + + // PATCH invalid custom fields to the entity - custom field has invalid type + String json1 = JsonUtils.pojoToJson(entity); + entity.getCustomProperties().add(fieldInvalid); + Type finalEntity = entity; + assertResponse( + () -> typeResourceTest.patchEntity(id, json1, finalEntity, ADMIN_AUTH_HEADERS), + NOT_FOUND, + CatalogExceptionMessage.entityNotFound(Entity.TYPE, invalidType.getId())); // Now create an entity with custom field ObjectMapper mapper = new ObjectMapper(); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/metadata/TypeResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/metadata/TypeResourceTest.java index 5c04e546c79..a912902fdad 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/metadata/TypeResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/metadata/TypeResourceTest.java @@ -22,6 +22,7 @@ import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; import java.io.IOException; import java.net.URISyntaxException; +import java.util.List; import java.util.Map; import java.util.UUID; import javax.ws.rs.client.WebTarget; @@ -42,6 +43,8 @@ import org.openmetadata.catalog.resources.EntityResourceTest; import org.openmetadata.catalog.resources.types.TypeResource; import org.openmetadata.catalog.resources.types.TypeResource.TypeList; import org.openmetadata.catalog.type.EntityReference; +import org.openmetadata.catalog.util.EntityUtil; +import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.TestUtils; @Slf4j @@ -112,7 +115,7 @@ public class TypeResourceTest extends EntityResourceTest { assertResponse( () -> addCustomProperty(INT_TYPE.getId(), field, Status.CREATED, ADMIN_AUTH_HEADERS), Status.BAD_REQUEST, - "Property types can't be extended"); + "Only entity types can be extended and field types can't be extended"); } @Override @@ -125,6 +128,17 @@ public class TypeResourceTest extends EntityResourceTest { return type; } + public Type addAndCheckCustomProperty( + UUID entityTypeId, CustomProperty customProperty, Status status, Map authHeaders) + throws HttpResponseException { + Type updated = addCustomProperty(entityTypeId, customProperty, status, authHeaders); + assertTrue(updated.getCustomProperties().contains(customProperty)); + + Type get = getEntity(entityTypeId, "customProperties", authHeaders); + assertTrue(get.getCustomProperties().contains(customProperty)); + return updated; + } + public Type addCustomProperty( UUID entityTypeId, CustomProperty customProperty, Status status, Map authHeaders) throws HttpResponseException { @@ -159,6 +173,15 @@ public class TypeResourceTest extends EntityResourceTest { if (expected == actual) { return; } - assertCommonFieldChange(fieldName, expected, actual); + 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); + } else { + assertCommonFieldChange(fieldName, expected, actual); + } } }