diff --git a/ingestion/src/metadata/ingestion/models/custom_properties.py b/ingestion/src/metadata/ingestion/models/custom_properties.py index 6a62c1dbc05..c287f64f122 100644 --- a/ingestion/src/metadata/ingestion/models/custom_properties.py +++ b/ingestion/src/metadata/ingestion/models/custom_properties.py @@ -41,7 +41,6 @@ class CustomPropertyDataTypes(Enum): class OMetaCustomProperties(BaseModel): entity_type: Type[T] - custom_property_type: Optional[CustomPropertyDataTypes] createCustomPropertyRequest: CreateCustomPropertyRequest diff --git a/ingestion/src/metadata/ingestion/ometa/mixins/custom_property_mixin.py b/ingestion/src/metadata/ingestion/ometa/mixins/custom_property_mixin.py index a60aea19246..5b0fe01115c 100644 --- a/ingestion/src/metadata/ingestion/ometa/mixins/custom_property_mixin.py +++ b/ingestion/src/metadata/ingestion/ometa/mixins/custom_property_mixin.py @@ -15,7 +15,8 @@ To be used by OpenMetadata class """ from typing import Dict -from metadata.generated.schema.api.data.createCustomProperty import PropertyType +from metadata.generated.schema.type.customProperty import PropertyType +from metadata.generated.schema.type.entityReference import EntityReference from metadata.ingestion.models.custom_properties import ( CustomPropertyDataTypes, CustomPropertyType, @@ -54,16 +55,6 @@ class OMetaCustomPropertyMixin: f"/metadata/types/name/{entity_type}?category=field" ) - # Get the data type of the custom property - if not ometa_custom_property.createCustomPropertyRequest.propertyType: - custom_property_type = self.get_custom_property_type( - data_type=ometa_custom_property.custom_property_type - ) - property_type = PropertyType(id=custom_property_type.id, type="type") - ometa_custom_property.createCustomPropertyRequest.propertyType = ( - property_type - ) - resp = self.client.put( f"/metadata/types/{entity_schema.get('id')}", data=ometa_custom_property.createCustomPropertyRequest.json(), @@ -78,3 +69,12 @@ class OMetaCustomPropertyMixin: """ resp = self.client.get(f"/metadata/types/name/{data_type.value}?category=field") return CustomPropertyType(**resp) + + def get_property_type_ref(self, data_type: CustomPropertyDataTypes) -> PropertyType: + """ + Get the PropertyType for custom properties + """ + custom_property_type = self.get_custom_property_type(data_type=data_type) + return PropertyType( + __root__=EntityReference(id=custom_property_type.id, type="type") + ) diff --git a/ingestion/tests/integration/ometa/test_ometa_custom_properties_api.py b/ingestion/tests/integration/ometa/test_ometa_custom_properties_api.py index afe57999c70..8834acb121d 100644 --- a/ingestion/tests/integration/ometa/test_ometa_custom_properties_api.py +++ b/ingestion/tests/integration/ometa/test_ometa_custom_properties_api.py @@ -144,9 +144,12 @@ class OMetaCustomAttributeTest(TestCase): # Create the table size property ometa_custom_property_request = OMetaCustomProperties( entity_type=Table, - custom_property_type=CustomPropertyDataTypes.STRING, createCustomPropertyRequest=CreateCustomPropertyRequest( - name="TableSize", description="Size of the Table" + name="TableSize", + description="Size of the Table", + propertyType=self.metadata.get_property_type_ref( + CustomPropertyDataTypes.STRING + ), ), ) self.metadata.create_or_update_custom_property( @@ -156,9 +159,12 @@ class OMetaCustomAttributeTest(TestCase): # Create the DataQuality property for a table ometa_custom_property_request = OMetaCustomProperties( entity_type=Table, - custom_property_type=CustomPropertyDataTypes.MARKDOWN, createCustomPropertyRequest=CreateCustomPropertyRequest( - name="DataQuality", description="Quality Details of a Table" + name="DataQuality", + description="Quality Details of a Table", + propertyType=self.metadata.get_property_type_ref( + CustomPropertyDataTypes.MARKDOWN + ), ), ) self.metadata.create_or_update_custom_property( @@ -168,9 +174,12 @@ class OMetaCustomAttributeTest(TestCase): # Create the SchemaCost property for database schema ometa_custom_property_request = OMetaCustomProperties( entity_type=DatabaseSchema, - custom_property_type=CustomPropertyDataTypes.INTEGER, createCustomPropertyRequest=CreateCustomPropertyRequest( - name="SchemaAge", description="Age in years of a Schema" + name="SchemaAge", + description="Age in years of a Schema", + propertyType=self.metadata.get_property_type_ref( + CustomPropertyDataTypes.INTEGER + ), ), ) self.metadata.create_or_update_custom_property( 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 68ad87df8f2..bce5765a243 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 @@ -23,6 +23,7 @@ import static org.openmetadata.service.util.EntityUtil.customFieldMatch; import static org.openmetadata.service.util.EntityUtil.getCustomField; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.UUID; import javax.ws.rs.core.UriInfo; @@ -32,9 +33,11 @@ import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.schema.entity.Type; import org.openmetadata.schema.entity.type.Category; import org.openmetadata.schema.entity.type.CustomProperty; +import org.openmetadata.schema.type.CustomPropertyConfig; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.Include; import org.openmetadata.schema.type.Relationship; +import org.openmetadata.schema.type.customproperties.EnumConfig; import org.openmetadata.service.Entity; import org.openmetadata.service.TypeRegistry; import org.openmetadata.service.resources.types.TypeResource; @@ -117,6 +120,7 @@ public class TypeRepository extends EntityRepository { property.setPropertyType( Entity.getEntityReferenceById( Entity.TYPE, property.getPropertyType().getId(), NON_DELETED)); + validateProperty(property); if (type.getCategory().equals(Category.Field)) { throw new IllegalArgumentException( "Only entity types can be extended and field types can't be extended"); @@ -161,6 +165,30 @@ public class TypeRepository extends EntityRepository { return customProperties; } + private void validateProperty(CustomProperty customProperty) { + switch (customProperty.getPropertyType().getName()) { + case "enum" -> { + CustomPropertyConfig config = customProperty.getCustomPropertyConfig(); + if (config != null) { + EnumConfig enumConfig = JsonUtils.convertValue(config.getConfig(), EnumConfig.class); + if (enumConfig == null + || (enumConfig.getValues() != null && enumConfig.getValues().isEmpty())) { + throw new IllegalArgumentException( + "Enum Custom Property Type must have EnumConfig populated with values."); + } else if (enumConfig.getValues() != null + && enumConfig.getValues().stream().distinct().count() + != enumConfig.getValues().size()) { + throw new IllegalArgumentException( + "Enum Custom Property values cannot have duplicates."); + } + } else { + throw new IllegalArgumentException("Enum Custom Property Type must have EnumConfig."); + } + } + case "int", "string" -> {} + } + } + /** Handles entity updated from PUT and POST operation. */ public class TypeUpdater extends EntityUpdater { public TypeUpdater(Type original, Type updated, Operation operation) { @@ -199,6 +227,7 @@ public class TypeRepository extends EntityRepository { continue; } updateCustomPropertyDescription(updated, storedProperty, updateProperty); + updateCustomPropertyConfig(updated, storedProperty, updateProperty); } } @@ -270,5 +299,55 @@ public class TypeRepository extends EntityRepository { customPropertyJson); } } + + private void updateCustomPropertyConfig( + Type entity, CustomProperty origProperty, CustomProperty updatedProperty) { + String fieldName = getCustomField(origProperty, "customPropertyConfig"); + if (previous == null || !previous.getVersion().equals(updated.getVersion())) { + validatePropertyConfigUpdate(entity, origProperty, updatedProperty); + } + if (recordChange( + fieldName, + origProperty.getCustomPropertyConfig(), + updatedProperty.getCustomPropertyConfig())) { + String customPropertyFQN = + getCustomPropertyFQN(entity.getName(), updatedProperty.getName()); + EntityReference propertyType = + updatedProperty.getPropertyType(); // Don't store entity reference + String customPropertyJson = JsonUtils.pojoToJson(updatedProperty.withPropertyType(null)); + updatedProperty.withPropertyType(propertyType); // Restore entity reference + daoCollection + .fieldRelationshipDAO() + .upsert( + customPropertyFQN, + updatedProperty.getPropertyType().getName(), + customPropertyFQN, + updatedProperty.getPropertyType().getName(), + Entity.TYPE, + Entity.TYPE, + Relationship.HAS.ordinal(), + "customProperty", + customPropertyJson); + } + } + + private void validatePropertyConfigUpdate( + Type entity, CustomProperty origProperty, CustomProperty updatedProperty) { + if (origProperty.getPropertyType().getName().equals("enum")) { + EnumConfig origConfig = + JsonUtils.convertValue( + origProperty.getCustomPropertyConfig().getConfig(), EnumConfig.class); + EnumConfig updatedConfig = + JsonUtils.convertValue( + updatedProperty.getCustomPropertyConfig().getConfig(), EnumConfig.class); + HashSet updatedValues = new HashSet<>(updatedConfig.getValues()); + if (updatedValues.size() != updatedConfig.getValues().size()) { + throw new IllegalArgumentException("Enum Custom Property values cannot have duplicates."); + } else if (!updatedValues.containsAll(origConfig.getValues())) { + throw new IllegalArgumentException( + "Existing Enum Custom Property values cannot be removed."); + } + } + } } } 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 eeae3923a52..a3c60ae2164 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 @@ -100,6 +100,7 @@ import org.apache.http.client.HttpResponseException; import org.apache.http.util.EntityUtils; import org.awaitility.Awaitility; import org.json.JSONArray; +import org.json.JSONException; import org.json.JSONObject; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -364,6 +365,8 @@ public abstract class EntityResourceTest { public void setupTypes() throws HttpResponseException { INT_TYPE = getEntityByName("integer", "", ADMIN_AUTH_HEADERS); STRING_TYPE = getEntityByName("string", "", ADMIN_AUTH_HEADERS); + ENUM_TYPE = getEntityByName("enum", "", ADMIN_AUTH_HEADERS); } @Override @@ -86,8 +89,8 @@ public class TypeResourceTest extends EntityResourceTest { @Test void put_patch_customProperty_200() throws IOException { - Type tableEntity = getEntityByName("table", "customProperties", ADMIN_AUTH_HEADERS); - assertTrue(listOrEmpty(tableEntity.getCustomProperties()).isEmpty()); + Type topicEntity = getEntityByName("topic", "customProperties", ADMIN_AUTH_HEADERS); + assertTrue(listOrEmpty(topicEntity.getCustomProperties()).isEmpty()); // Add a custom property with name intA with type integer with PUT CustomProperty fieldA = @@ -95,34 +98,170 @@ public class TypeResourceTest extends EntityResourceTest { .withName("intA") .withDescription("intA") .withPropertyType(INT_TYPE.getEntityReference()); - ChangeDescription change = getChangeDescription(tableEntity, MINOR_UPDATE); + ChangeDescription change = getChangeDescription(topicEntity, MINOR_UPDATE); fieldAdded(change, "customProperties", new ArrayList<>(List.of(fieldA))); - tableEntity = + topicEntity = addCustomPropertyAndCheck( - tableEntity.getId(), fieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); - assertCustomProperties(new ArrayList<>(List.of(fieldA)), tableEntity.getCustomProperties()); + topicEntity.getId(), fieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + assertCustomProperties(new ArrayList<>(List.of(fieldA)), topicEntity.getCustomProperties()); // Changing custom property description with PUT fieldA.withDescription("updated"); - change = getChangeDescription(tableEntity, MINOR_UPDATE); + change = getChangeDescription(topicEntity, MINOR_UPDATE); fieldUpdated(change, EntityUtil.getCustomField(fieldA, "description"), "intA", "updated"); - tableEntity = + topicEntity = addCustomPropertyAndCheck( - tableEntity.getId(), fieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); - assertCustomProperties(new ArrayList<>(List.of(fieldA)), tableEntity.getCustomProperties()); + topicEntity.getId(), fieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + assertCustomProperties(new ArrayList<>(List.of(fieldA)), topicEntity.getCustomProperties()); // Changing custom property description with PATCH // Changes from this PATCH is consolidated with the previous changes fieldA.withDescription("updated2"); - String json = JsonUtils.pojoToJson(tableEntity); - tableEntity.setCustomProperties(List.of(fieldA)); - change = getChangeDescription(tableEntity, CHANGE_CONSOLIDATED); + String json = JsonUtils.pojoToJson(topicEntity); + topicEntity.setCustomProperties(List.of(fieldA)); + change = getChangeDescription(topicEntity, CHANGE_CONSOLIDATED); fieldUpdated(change, EntityUtil.getCustomField(fieldA, "description"), "intA", "updated2"); - tableEntity = - patchEntityAndCheck(tableEntity, json, ADMIN_AUTH_HEADERS, CHANGE_CONSOLIDATED, change); + topicEntity = + patchEntityAndCheck(topicEntity, json, ADMIN_AUTH_HEADERS, CHANGE_CONSOLIDATED, change); // Add a second property with name intB with type integer // Note that since this is PUT operation, the previous changes are not consolidated + EntityReference typeRef = + new EntityReference() + .withType(INT_TYPE.getEntityReference().getType()) + .withId(INT_TYPE.getEntityReference().getId()); + CustomProperty fieldB = + new CustomProperty().withName("intB").withDescription("intB").withPropertyType(typeRef); + change = getChangeDescription(topicEntity, MINOR_UPDATE); + fieldAdded(change, "customProperties", new ArrayList<>(List.of(fieldB))); + topicEntity = + addCustomPropertyAndCheck( + topicEntity.getId(), fieldB, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + fieldB.setPropertyType(INT_TYPE.getEntityReference()); + assertEquals(2, topicEntity.getCustomProperties().size()); + assertCustomProperties( + new ArrayList<>(List.of(fieldA, fieldB)), topicEntity.getCustomProperties()); + } + + @Test + void put_patch_customProperty_enum_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 with PUT + CustomProperty enumFieldA = + new CustomProperty() + .withName("enumTest") + .withDescription("enumTest") + .withPropertyType(ENUM_TYPE.getEntityReference()); + ChangeDescription change = getChangeDescription(tableEntity, MINOR_UPDATE); + fieldAdded(change, "customProperties", new ArrayList<>(List.of(enumFieldA))); + Type finalTableEntity = tableEntity; + ChangeDescription finalChange = change; + assertResponseContains( + () -> + addCustomPropertyAndCheck( + finalTableEntity.getId(), + enumFieldA, + ADMIN_AUTH_HEADERS, + MINOR_UPDATE, + finalChange), + Status.BAD_REQUEST, + "Enum Custom Property Type must have EnumConfig."); + enumFieldA.setCustomPropertyConfig(new CustomPropertyConfig().withConfig(new EnumConfig())); + ChangeDescription change1 = getChangeDescription(tableEntity, MINOR_UPDATE); + Type tableEntity1 = tableEntity; + assertResponseContains( + () -> + addCustomPropertyAndCheck( + tableEntity1.getId(), enumFieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change1), + Status.BAD_REQUEST, + "Enum Custom Property Type must have EnumConfig populated with values."); + + enumFieldA.setCustomPropertyConfig( + new CustomPropertyConfig() + .withConfig(new EnumConfig().withValues(List.of("A", "B", "C", "C")))); + ChangeDescription change7 = getChangeDescription(tableEntity, MINOR_UPDATE); + Type tableEntity2 = tableEntity; + assertResponseContains( + () -> + addCustomPropertyAndCheck( + tableEntity2.getId(), enumFieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change7), + Status.BAD_REQUEST, + "Enum Custom Property values cannot have duplicates."); + + enumFieldA.setCustomPropertyConfig( + new CustomPropertyConfig().withConfig(new EnumConfig().withValues(List.of("A", "B", "C")))); + tableEntity = + addCustomPropertyAndCheck( + tableEntity.getId(), enumFieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + assertCustomProperties(new ArrayList<>(List.of(enumFieldA)), tableEntity.getCustomProperties()); + CustomPropertyConfig prevConfig = enumFieldA.getCustomPropertyConfig(); + // Changing custom property description with PUT + enumFieldA.withDescription("updatedEnumTest"); + ChangeDescription change2 = getChangeDescription(tableEntity, MINOR_UPDATE); + fieldUpdated( + change2, + EntityUtil.getCustomField(enumFieldA, "description"), + "enumTest", + "updatedEnumTest"); + tableEntity = + addCustomPropertyAndCheck( + tableEntity.getId(), enumFieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change2); + assertCustomProperties(new ArrayList<>(List.of(enumFieldA)), tableEntity.getCustomProperties()); + + enumFieldA.setCustomPropertyConfig( + new CustomPropertyConfig().withConfig(new EnumConfig().withValues(List.of("A", "B")))); + ChangeDescription change3 = getChangeDescription(tableEntity, MINOR_UPDATE); + assertResponseContains( + () -> + addCustomPropertyAndCheck( + tableEntity1.getId(), enumFieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change3), + Status.BAD_REQUEST, + "Existing Enum Custom Property values cannot be removed."); + + enumFieldA.setCustomPropertyConfig( + new CustomPropertyConfig() + .withConfig(new EnumConfig().withValues(List.of("A", "B", "C", "C")))); + ChangeDescription change4 = getChangeDescription(tableEntity, MINOR_UPDATE); + assertResponseContains( + () -> + addCustomPropertyAndCheck( + tableEntity1.getId(), enumFieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change4), + Status.BAD_REQUEST, + "Enum Custom Property values cannot have duplicates."); + + ChangeDescription change5 = getChangeDescription(tableEntity, MINOR_UPDATE); + enumFieldA.setCustomPropertyConfig( + new CustomPropertyConfig() + .withConfig(new EnumConfig().withValues(List.of("A", "B", "C", "D")))); + fieldUpdated( + change5, + EntityUtil.getCustomField(enumFieldA, "customPropertyConfig"), + prevConfig, + enumFieldA.getCustomPropertyConfig()); + tableEntity = + addCustomPropertyAndCheck( + tableEntity.getId(), enumFieldA, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change5); + assertCustomProperties(new ArrayList<>(List.of(enumFieldA)), tableEntity.getCustomProperties()); + + // Changing custom property description with PATCH + // Changes from this PATCH is consolidated with the previous changes + enumFieldA.withDescription("updated2"); + String json = JsonUtils.pojoToJson(tableEntity); + tableEntity.setCustomProperties(List.of(enumFieldA)); + change = getChangeDescription(tableEntity, CHANGE_CONSOLIDATED); + fieldUpdated( + change5, + EntityUtil.getCustomField(enumFieldA, "description"), + "updatedEnumTest", + "updated2"); + + tableEntity = + patchEntityAndCheck(tableEntity, json, ADMIN_AUTH_HEADERS, CHANGE_CONSOLIDATED, change5); + + /* // Add a second property with name intB with type integer + // Note that since this is PUT operation, the previous changes are not consolidated EntityReference typeRef = new EntityReference() .withType(INT_TYPE.getEntityReference().getType()) @@ -137,7 +276,7 @@ public class TypeResourceTest extends EntityResourceTest { fieldB.setPropertyType(INT_TYPE.getEntityReference()); assertEquals(2, tableEntity.getCustomProperties().size()); assertCustomProperties( - new ArrayList<>(List.of(fieldA, fieldB)), tableEntity.getCustomProperties()); + new ArrayList<>(List.of(fieldA, fieldB)), tableEntity.getCustomProperties());*/ } @Test @@ -225,7 +364,6 @@ public class TypeResourceTest extends EntityResourceTest { assertEquals(expected.getSchema(), patched.getSchema()); assertEquals(expected.getCategory(), patched.getCategory()); assertEquals(expected.getNameSpace(), patched.getNameSpace()); - assertEquals(expected.getCustomProperties(), patched.getCustomProperties()); } @Override @@ -239,6 +377,9 @@ public class TypeResourceTest extends EntityResourceTest { List actualProperties = JsonUtils.readObjects(actual.toString(), CustomProperty.class); TestUtils.assertCustomProperties(expectedProperties, actualProperties); + } else if (fieldName.contains("customPropertyConfig")) { + String expectedStr = JsonUtils.pojoToJson(expected); + String actualStr = JsonUtils.pojoToJson(actual); } else { assertCommonFieldChange(fieldName, expected, actual); } diff --git a/openmetadata-spec/src/main/resources/json/schema/api/data/createCustomProperty.json b/openmetadata-spec/src/main/resources/json/schema/api/data/createCustomProperty.json index 1bd4ee78f0b..7261753a98c 100644 --- a/openmetadata-spec/src/main/resources/json/schema/api/data/createCustomProperty.json +++ b/openmetadata-spec/src/main/resources/json/schema/api/data/createCustomProperty.json @@ -4,23 +4,6 @@ "title": "CreateCustomPropertyRequest", "description": "Create Custom Property Model entity request", "type": "object", - "definitions": { - "propertyType": { - "description": "Property Type", - "type": "object", - "properties": { - "type": { - "description": "Property type", - "type": "string", - "default": "type" - }, - "id": { - "description": "Unique identifier of this instance.", - "$ref": "../../type/basic.json#/definitions/uuid" - } - } - } - }, "properties": { "name": { "description": "Name that identifies this Custom Property model.", @@ -31,10 +14,14 @@ "$ref": "../../type/basic.json#/definitions/markdown" }, "propertyType": { - "description": "Property Type", - "$ref": "#/definitions/propertyType" + "description": "Property Type.", + "$ref": "../../type/customProperty.json#/definitions/propertyType" + }, + "customPropertyConfig": { + "description": "Config to define constraints around CustomProperty.", + "$ref": "../../type/customProperty.json#/definitions/customPropertyConfig" } }, - "required": ["name"], + "required": ["name", "propertyType"], "additionalProperties": false } diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/type.json b/openmetadata-spec/src/main/resources/json/schema/entity/type.json index f0e85aa8b3d..adb0ce9f460 100644 --- a/openmetadata-spec/src/main/resources/json/schema/entity/type.json +++ b/openmetadata-spec/src/main/resources/json/schema/entity/type.json @@ -25,26 +25,6 @@ "name": "Entity" } ] - }, - "customProperty": { - "description": "Type used for adding custom property to an entity to extend it.", - "type": "object", - "javaType": "org.openmetadata.schema.entity.type.CustomProperty", - "properties": { - "name": { - "description": "Name of the entity property. Note a property name must be unique for an entity. Property name must follow camelCase naming adopted by openMetadata - must start with lower case with no space, underscore, or dots.", - "$ref": "#/definitions/entityName" - }, - "description": { - "$ref": "../type/basic.json#/definitions/markdown" - }, - "propertyType": { - "description": "Reference to a property type. Only property types are allowed and entity types are not allowed as custom properties to extend an existing entity", - "$ref": "../type/entityReference.json" - } - }, - "required": ["name", "description", "propertyType"], - "additionalProperties": false } }, "properties": { @@ -84,7 +64,7 @@ "description": "Custom properties added to extend the entity. Only available for entity type", "type": "array", "items": { - "$ref": "#/definitions/customProperty" + "$ref": "../type/customProperty.json" } }, "version": { diff --git a/openmetadata-spec/src/main/resources/json/schema/type/basic.json b/openmetadata-spec/src/main/resources/json/schema/type/basic.json index 22eddb63751..45a4ee7a894 100644 --- a/openmetadata-spec/src/main/resources/json/schema/type/basic.json +++ b/openmetadata-spec/src/main/resources/json/schema/type/basic.json @@ -85,6 +85,14 @@ "type": "string", "format": "time" }, + "enum": { + "$comment" : "@om-field-type", + "description": "List of values in Enum.", + "type": "array", + "items": { + "type": "string" + } + }, "timezone": { "description": "Timezone of the user in the format `America/Los_Angeles`, `Brazil/East`, etc.", "type": "string", diff --git a/openmetadata-spec/src/main/resources/json/schema/type/customProperties/enumConfig.json b/openmetadata-spec/src/main/resources/json/schema/type/customProperties/enumConfig.json new file mode 100644 index 00000000000..459dbac8aab --- /dev/null +++ b/openmetadata-spec/src/main/resources/json/schema/type/customProperties/enumConfig.json @@ -0,0 +1,21 @@ +{ + "$id": "https://open-metadata.org/schema/type/customPropertyEnumConfig.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "EnumConfig", + "type": "object", + "javaType": "org.openmetadata.schema.type.customproperties.EnumConfig", + "description": "Applies to Enum type, this config is used to define list of enum values", + "properties": { + "multiSelect": { + "type": "boolean", + "default": false + }, + "values": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + } \ No newline at end of file diff --git a/openmetadata-spec/src/main/resources/json/schema/type/customProperty.json b/openmetadata-spec/src/main/resources/json/schema/type/customProperty.json new file mode 100644 index 00000000000..f88f5cd512b --- /dev/null +++ b/openmetadata-spec/src/main/resources/json/schema/type/customProperty.json @@ -0,0 +1,61 @@ +{ + "$id": "https://open-metadata.org/schema/type/customProperty.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "CustomProperty", + "description": "This schema defines the custom property to an entity to extend it.", + "type": "object", + "javaType": "org.openmetadata.schema.entity.type.CustomProperty", + "definitions": { + "format": { + "description": "Applies to date interval, date, time format.", + "type": "string" + }, + "entityTypes": { + "description": "Applies to Entity References. Entity Types can be used to restrict what type of entities can be configured for a entity reference.", + "type": "string" + }, + "customPropertyConfig": { + "type": "object", + "javaType": "org.openmetadata.schema.type.CustomPropertyConfig", + "title": "CustomPropertyConfig", + "description": "Config to define constraints around CustomProperty", + "properties": { + "config": { + "oneOf": [ + { + "$ref": "../type/customProperties/enumConfig.json" + }, + { + "$ref": "#/definitions/format" + }, + { + "$ref": "#/definitions/entityTypes" + } + ] + } + }, + "additionalProperties": false + }, + "propertyType": { + "description": "Reference to a property type. Only property types are allowed and entity types are not allowed as custom properties to extend an existing entity", + "$ref": "../type/entityReference.json" + } + }, + "properties": { + "name": { + "description": "Name of the entity property. Note a property name must be unique for an entity. Property name must follow camelCase naming adopted by openMetadata - must start with lower case with no space, underscore, or dots.", + "$ref": "../type/basic.json#/definitions/entityName" + }, + "description": { + "$ref": "../type/basic.json#/definitions/markdown" + }, + "propertyType": { + "$ref": "#/definitions/propertyType" + }, + "customPropertyConfig": { + "$ref": "#/definitions/customPropertyConfig" + } + }, + "required": ["name", "description", "propertyType"], + "additionalProperties": false +} \ No newline at end of file diff --git a/openmetadata-ui/src/main/resources/ui/cypress/common/common.js b/openmetadata-ui/src/main/resources/ui/cypress/common/common.js index 400490ec6ef..9045d69bb26 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/common/common.js +++ b/openmetadata-ui/src/main/resources/ui/cypress/common/common.js @@ -596,6 +596,18 @@ export const addCustomPropertiesForEntity = ( cy.get('[data-testid="propertyType"]').click(); cy.get(`[title="${customType}"]`).click(); + if (customType === 'Enum') { + value.values.forEach((val) => { + cy.get('#root\\/customPropertyConfig').type(`${val}{enter}`); + }); + + cy.clickOutside(); + + if (value.multiSelect) { + cy.get('#root\\/multiSelect').scrollIntoView().click(); + } + } + cy.get(descriptionBox).clear().type(customPropertyData.description); // Check if the property got added @@ -611,19 +623,31 @@ export const addCustomPropertiesForEntity = ( cy.clickOnLogo(); }; -export const editCreatedProperty = (propertyName) => { +export const editCreatedProperty = (propertyName, type) => { // Fetching for edit button cy.get(`[data-row-key="${propertyName}"]`) .find('[data-testid="edit-button"]') .as('editButton'); + if (type === 'Enum') { + cy.get(`[data-row-key="${propertyName}"]`) + .find('[data-testid="enum-config"]') + .should('contain', '["enum1","enum2","enum3"]'); + } + cy.get('@editButton').click(); cy.get(descriptionBox).clear().type('This is new description'); + if (type === 'Enum') { + cy.get('#root\\/customPropertyConfig').type(`updatedValue{enter}`); + + cy.clickOutside(); + } + interceptURL('PATCH', '/api/v1/metadata/types/*', 'checkPatchForDescription'); - cy.get('[data-testid="save"]').click(); + cy.get('button[type="submit"]').scrollIntoView().click(); cy.wait('@checkPatchForDescription', { timeout: 15000 }); @@ -633,6 +657,12 @@ export const editCreatedProperty = (propertyName) => { cy.get(`[data-row-key="${propertyName}"]`) .find('[data-testid="viewer-container"]') .should('contain', 'This is new description'); + + if (type === 'Enum') { + cy.get(`[data-row-key="${propertyName}"]`) + .find('[data-testid="enum-config"]') + .should('contain', '["enum1","enum2","enum3","updatedValue"]'); + } }; export const deleteCreatedProperty = (propertyName) => { diff --git a/openmetadata-ui/src/main/resources/ui/cypress/constants/constants.js b/openmetadata-ui/src/main/resources/ui/cypress/constants/constants.js index f359cc5e8ba..265353dd175 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/constants/constants.js +++ b/openmetadata-ui/src/main/resources/ui/cypress/constants/constants.js @@ -495,6 +495,10 @@ export const ENTITIES = { integerValue: '45', stringValue: 'This is string propery', markdownValue: 'This is markdown value', + enumConfig: { + values: ['enum1', 'enum2', 'enum3'], + multiSelect: false, + }, entityObj: SEARCH_ENTITY_TABLE.table_1, entityApiType: 'tables', }, @@ -504,6 +508,10 @@ export const ENTITIES = { integerValue: '23', stringValue: 'This is string propery', markdownValue: 'This is markdown value', + enumConfig: { + values: ['enum1', 'enum2', 'enum3'], + multiSelect: false, + }, entityObj: SEARCH_ENTITY_TOPIC.topic_1, entityApiType: 'topics', }, @@ -523,6 +531,10 @@ export const ENTITIES = { integerValue: '78', stringValue: 'This is string propery', markdownValue: 'This is markdown value', + enumConfig: { + values: ['enum1', 'enum2', 'enum3'], + multiSelect: true, + }, entityObj: SEARCH_ENTITY_PIPELINE.pipeline_1, entityApiType: 'pipelines', }, diff --git a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Customproperties.spec.js b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Customproperties.spec.js index 033313a76f1..24ed8ef5eca 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Customproperties.spec.js +++ b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Customproperties.spec.js @@ -459,6 +459,66 @@ describe('Custom Properties should work properly', { tags: 'Settings' }, () => { }); }); + describe('Add update and delete Enum custom properties', () => { + Object.values(ENTITIES).forEach((entity) => { + const propertyName = `addcyentity${entity.name}test${uuid()}`; + + it(`Add Enum custom property for ${entity.name} Entities`, () => { + interceptURL( + 'GET', + `/api/v1/metadata/types/name/${entity.name}*`, + 'getEntity' + ); + + // Selecting the entity + cy.settingClick(entity.entityApiType, true); + + verifyResponseStatusCode('@getEntity', 200); + + addCustomPropertiesForEntity( + propertyName, + entity, + 'Enum', + entity.enumConfig, + entity.entityObj + ); + + // Navigating back to custom properties page + cy.settingClick(entity.entityApiType, true); + + verifyResponseStatusCode('@getEntity', 200); + }); + + it(`Edit created property for ${entity.name} entity`, () => { + interceptURL( + 'GET', + `/api/v1/metadata/types/name/${entity.name}*`, + 'getEntity' + ); + + // Selecting the entity + cy.settingClick(entity.entityApiType, true); + + verifyResponseStatusCode('@getEntity', 200); + editCreatedProperty(propertyName, 'Enum'); + }); + + it(`Delete created property for ${entity.name} entity`, () => { + interceptURL( + 'GET', + `/api/v1/metadata/types/name/${entity.name}*`, + 'getEntity' + ); + + // Selecting the entity + cy.settingClick(entity.entityApiType, true); + + verifyResponseStatusCode('@getEntity', 200); + deleteCreatedProperty(propertyName); + }); + }); + }); + describe('Custom properties for glossary and glossary terms', () => { const propertyName = `addcyentity${glossaryTerm.name}test${uuid()}`; const properties = Object.values(CustomPropertyType).join(', '); diff --git a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/OpenMetadata/CustomProperty.md b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/OpenMetadata/CustomProperty.md index 1e232908f64..5a04ea91ef6 100644 --- a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/OpenMetadata/CustomProperty.md +++ b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/OpenMetadata/CustomProperty.md @@ -18,4 +18,16 @@ $$section ### Description $(id="description") Describe your custom property to provide more information to your team. +$$ + +$$section +### Enum Values $(id="customPropertyConfig") + +Add the list of values for enum property. +$$ + +$$section +### Multi Select $(id="multiSelect") + +Enable multi select of values for enum property. $$ \ No newline at end of file diff --git a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/block-editor.less b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/block-editor.less index ca6628b9b44..786a1c44dd1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/block-editor.less +++ b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/block-editor.less @@ -323,11 +323,11 @@ .om-list-decimal { list-style-type: decimal !important; - padding-left: 16px; + padding-left: 24px; } .om-list-disc { list-style-type: disc !important; - padding-left: 16px; + padding-left: 24px; } .om-leading-normal { line-height: 1.5; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Settings/CustomProperty/AddCustomProperty/AddCustomProperty.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Settings/CustomProperty/AddCustomProperty/AddCustomProperty.tsx index ef7f9f7bba6..2e95bf1c5bc 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Settings/CustomProperty/AddCustomProperty/AddCustomProperty.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Settings/CustomProperty/AddCustomProperty/AddCustomProperty.tsx @@ -14,7 +14,7 @@ import { Button, Col, Form, Row } from 'antd'; import { AxiosError } from 'axios'; import { t } from 'i18next'; -import { isUndefined, map, startCase } from 'lodash'; +import { isUndefined, map, omit, startCase } from 'lodash'; import React, { FocusEvent, useCallback, @@ -32,14 +32,12 @@ import { } from '../../../../constants/service-guide.constant'; import { EntityType } from '../../../../enums/entity.enum'; import { ServiceCategory } from '../../../../enums/service.enum'; -import { - Category, - CustomProperty, - Type, -} from '../../../../generated/entity/type'; +import { Category, Type } from '../../../../generated/entity/type'; +import { CustomProperty } from '../../../../generated/type/customProperty'; import { FieldProp, FieldTypes, + FormItemLayout, } from '../../../../interface/FormUtils.interface'; import { addPropertyToEntity, @@ -55,6 +53,7 @@ import ServiceDocPanel from '../../../common/ServiceDocPanel/ServiceDocPanel'; import TitleBreadcrumb from '../../../common/TitleBreadcrumb/TitleBreadcrumb.component'; const AddCustomProperty = () => { + const [form] = Form.useForm(); const { entityType } = useParams<{ entityType: EntityType }>(); const history = useHistory(); @@ -64,6 +63,8 @@ const AddCustomProperty = () => { const [activeField, setActiveField] = useState(''); const [isCreating, setIsCreating] = useState(false); + const watchedPropertyType = Form.useWatch('propertyType', form); + const slashedBreadcrumb = useMemo( () => [ { @@ -99,6 +100,10 @@ const AddCustomProperty = () => { })); }, [propertyTypes]); + const isEnumType = + propertyTypeOptions.find((option) => option.value === watchedPropertyType) + ?.key === 'enum'; + const fetchPropertyType = async () => { try { const response = await getTypeListByCategory(Category.Field); @@ -130,7 +135,15 @@ const AddCustomProperty = () => { * In CustomProperty the propertyType is type of entity reference, however from the form we * get propertyType as string */ - data: Exclude & { propertyType: string } + /** + * In CustomProperty the customPropertyConfig is type of CustomPropertyConfig, however from the + * form we get customPropertyConfig as string[] + */ + data: Exclude & { + propertyType: string; + customPropertyConfig: string[]; + multiSelect?: boolean; + } ) => { if (isUndefined(typeDetail)) { return; @@ -139,11 +152,22 @@ const AddCustomProperty = () => { try { setIsCreating(true); await addPropertyToEntity(typeDetail?.id ?? '', { - ...data, + ...omit(data, 'multiSelect'), propertyType: { id: data.propertyType, type: 'type', }, + // Only add customPropertyConfig if it is an enum type + ...(isEnumType + ? { + customPropertyConfig: { + config: { + multiSelect: Boolean(data?.multiSelect), + values: data.customPropertyConfig, + }, + }, + } + : {}), }); history.goBack(); } catch (error) { @@ -194,29 +218,73 @@ const AddCustomProperty = () => { })}`, }, }, - { - name: 'description', - required: true, - label: t('label.description'), - id: 'root/description', - type: FieldTypes.DESCRIPTION, - props: { - 'data-testid': 'description', - initialValue: '', - }, - }, ]; + const descriptionField: FieldProp = { + name: 'description', + required: true, + label: t('label.description'), + id: 'root/description', + type: FieldTypes.DESCRIPTION, + props: { + 'data-testid': 'description', + initialValue: '', + }, + }; + + const customPropertyConfigTypeValueField: FieldProp = { + name: 'customPropertyConfig', + required: false, + label: t('label.enum-value-plural'), + id: 'root/customPropertyConfig', + type: FieldTypes.SELECT, + props: { + 'data-testid': 'customPropertyConfig', + mode: 'tags', + placeholder: t('label.enum-value-plural'), + }, + rules: [ + { + required: true, + message: t('label.field-required', { + field: t('label.enum-value-plural'), + }), + }, + ], + }; + + const multiSelectField: FieldProp = { + name: 'multiSelect', + label: t('label.multi-select'), + type: FieldTypes.SWITCH, + required: false, + props: { + 'data-testid': 'multiSelect', + }, + id: 'root/multiSelect', + formItemLayout: FormItemLayout.HORIZONTAL, + }; + const firstPanelChildren = (
{generateFormFields(formFields)} + {isEnumType && ( + <> + {generateFormFields([ + customPropertyConfigTypeValueField, + multiSelectField, + ])} + + )} + {generateFormFields([descriptionField])}