Skip to content

Commit

Permalink
Add new ‘enumWithDescriptions’ Custom Property to allow adding Enum K…
Browse files Browse the repository at this point in the history 
…eys with Description (#17777)

* Add new ‘metaEnum’ Custom Property to allow adding Enum Keys with Description

* replace JsonNodeFactory method with JsonUtils

* rename property from metaEnum to enumWithDescriptions, and other method optimizations

* ui: add support for creating enumWithDescription property

* minor locale changes

* ui: add edit support for created enumWithDescription property

* Refactor enum description field layout in AddCustomProperty and EditCustomPropertyModal

* add support for adding values to enumWithDescription custom property type

* Refactor custom property input IDs in AddCustomProperty and EditCustomPropertyModal components

* Refactor custom property table rendering logic and UI components

* Refactor custom property table rendering logic and UI components

* Refactor custom property table rendering logic and UI components

* add basic card layout

* Refactor CustomPropertyTable component to improve UI and functionality

* update playwright test part 1

* Refactor PropertyValue component to conditionally render right panel styles

* fix: entity reference property update

* Refactor CustomPropertyTable component to conditionally render right panel styles

* fix: flaky test

* Refactor CustomPropertyTable test to use updated test IDs and remove unnecessary code

* fix flaky test

* improve the playwright test

* add more test

---------

Co-authored-by: Sachin Chaurasiya <sachinchaurasiyachotey87@gmail.com>
Co-authored-by: Sriharsha Chintalapani <harshach@users.noreply.github.com>
  • Loading branch information
@sonika-shah @Sachin-chaurasiya @harshach
3 people authored Sep 28, 2024
1 parent f9e70f8 commit 1b029d2
Show file tree
Hide file tree
Showing 35 changed files with 1,528 additions and 227 deletions.
11 changes: 9 additions & 2 deletions openmetadata-service/src/main/java/org/openmetadata/service/TypeRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ public static String getCustomPropertyType(String entityType, String propertyNam
}
}
}
return null;
throw EntityNotFoundException.byMessage(
CatalogExceptionMessage.entityNotFound(Entity.TYPE, String.valueOf(type)));
}

public static String getCustomPropertyConfig(String entityType, String propertyName) {
Expand All @@ -122,7 +123,13 @@ public static String getCustomPropertyConfig(String entityType, String propertyN
if (property.getName().equals(propertyName)
&& property.getCustomPropertyConfig() != null
&& property.getCustomPropertyConfig().getConfig() != null) {
return property.getCustomPropertyConfig().getConfig().toString();
Object config = property.getCustomPropertyConfig().getConfig();
if (config instanceof String || config instanceof Integer) {
return config.toString(); // for simple type config return as string
} else {
return JsonUtils.pojoToJson(
config); // for complex object in config return as JSON string
}
}
}
}
Expand Down
126 changes: 102 additions & 24 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import static org.openmetadata.service.util.EntityUtil.tagLabelMatch;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.CacheBuilder;
Expand Down Expand Up @@ -100,6 +101,7 @@
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import javax.json.JsonPatch;
import javax.validation.constraints.NotNull;
import javax.ws.rs.core.Response.Status;
Expand Down Expand Up @@ -143,6 +145,7 @@
import org.openmetadata.schema.type.api.BulkOperationResult;
import org.openmetadata.schema.type.api.BulkResponse;
import org.openmetadata.schema.type.csv.CsvImportResult;
import org.openmetadata.schema.type.customproperties.EnumWithDescriptionsConfig;
import org.openmetadata.schema.utils.EntityInterfaceUtil;
import org.openmetadata.service.Entity;
import org.openmetadata.service.OpenMetadataApplicationConfig;
Expand Down Expand Up @@ -1426,41 +1429,116 @@ private void validateExtension(T entity) {
}
String customPropertyType = TypeRegistry.getCustomPropertyType(entityType, fieldName);
String propertyConfig = TypeRegistry.getCustomPropertyConfig(entityType, fieldName);
DateTimeFormatter formatter;
try {
if ("date-cp".equals(customPropertyType)) {
DateTimeFormatter inputFormatter =
DateTimeFormatter.ofPattern(Objects.requireNonNull(propertyConfig), Locale.ENGLISH);

// Parse the input string into a TemporalAccessor
TemporalAccessor date = inputFormatter.parse(fieldValue.textValue());

// Create a formatter for the desired output format
DateTimeFormatter outputFormatter =
DateTimeFormatter.ofPattern(propertyConfig, Locale.ENGLISH);
((ObjectNode) jsonNode).put(fieldName, outputFormatter.format(date));
} else if ("dateTime-cp".equals(customPropertyType)) {
formatter = DateTimeFormatter.ofPattern(Objects.requireNonNull(propertyConfig));
LocalDateTime dateTime = LocalDateTime.parse(fieldValue.textValue(), formatter);
((ObjectNode) jsonNode).put(fieldName, dateTime.format(formatter));
} else if ("time-cp".equals(customPropertyType)) {
formatter = DateTimeFormatter.ofPattern(Objects.requireNonNull(propertyConfig));
LocalTime time = LocalTime.parse(fieldValue.textValue(), formatter);
((ObjectNode) jsonNode).put(fieldName, time.format(formatter));
}
try {
validateAndUpdateExtensionBasedOnPropertyType(
entity,
(ObjectNode) jsonNode,
fieldName,
fieldValue,
customPropertyType,
propertyConfig);
} catch (DateTimeParseException e) {
throw new IllegalArgumentException(
CatalogExceptionMessage.dateTimeValidationError(
fieldName, TypeRegistry.getCustomPropertyConfig(entityType, fieldName)));
CatalogExceptionMessage.dateTimeValidationError(fieldName, propertyConfig));
}
Set<ValidationMessage> validationMessages = jsonSchema.validate(fieldValue);

Set<ValidationMessage> validationMessages = jsonSchema.validate(entry.getValue());
if (!validationMessages.isEmpty()) {
throw new IllegalArgumentException(
CatalogExceptionMessage.jsonValidationError(fieldName, validationMessages.toString()));
}
}
}

private void validateAndUpdateExtensionBasedOnPropertyType(
T entity,
ObjectNode jsonNode,
String fieldName,
JsonNode fieldValue,
String customPropertyType,
String propertyConfig) {

switch (customPropertyType) {
case "date-cp", "dateTime-cp", "time-cp" -> {
String formattedValue =
getFormattedDateTimeField(
fieldValue.textValue(), customPropertyType, propertyConfig, fieldName);
jsonNode.put(fieldName, formattedValue);
}
case "enumWithDescriptions" -> handleEnumWithDescriptions(
fieldName, fieldValue, propertyConfig, jsonNode, entity);
default -> {}
}
}

private String getFormattedDateTimeField(
String fieldValue, String customPropertyType, String propertyConfig, String fieldName) {
DateTimeFormatter formatter;

try {
return switch (customPropertyType) {
case "date-cp" -> {
DateTimeFormatter inputFormatter =
DateTimeFormatter.ofPattern(propertyConfig, Locale.ENGLISH);
TemporalAccessor date = inputFormatter.parse(fieldValue);
DateTimeFormatter outputFormatter =
DateTimeFormatter.ofPattern(propertyConfig, Locale.ENGLISH);
yield outputFormatter.format(date);
}
case "dateTime-cp" -> {
formatter = DateTimeFormatter.ofPattern(propertyConfig);
LocalDateTime dateTime = LocalDateTime.parse(fieldValue, formatter);
yield dateTime.format(formatter);
}
case "time-cp" -> {
formatter = DateTimeFormatter.ofPattern(propertyConfig);
LocalTime time = LocalTime.parse(fieldValue, formatter);
yield time.format(formatter);
}
default -> throw new IllegalArgumentException(
"Unsupported customPropertyType: " + customPropertyType);
};
} catch (DateTimeParseException e) {
throw new IllegalArgumentException(
CatalogExceptionMessage.dateTimeValidationError(fieldName, propertyConfig));
}
}

private void handleEnumWithDescriptions(
String fieldName, JsonNode fieldValue, String propertyConfig, ObjectNode jsonNode, T entity) {
JsonNode propertyConfigNode = JsonUtils.readTree(propertyConfig);
EnumWithDescriptionsConfig config =
JsonUtils.treeToValue(propertyConfigNode, EnumWithDescriptionsConfig.class);

if (!config.getMultiSelect() && fieldValue.size() > 1) {
throw new IllegalArgumentException(
"Only one key is allowed for non-multiSelect enumWithDescriptions");
}
// Replace each enumWithDescriptions key in the fieldValue with the corresponding object from
// the propertyConfig
Map<String, JsonNode> keyToObjectMap =
StreamSupport.stream(propertyConfigNode.get("values").spliterator(), false)
.collect(Collectors.toMap(node -> node.get("key").asText(), node -> node));

if (fieldValue.isArray()) {
ArrayNode newArray = JsonUtils.getObjectNode().arrayNode();
fieldValue.forEach(
valueNode -> {
String key = valueNode.isTextual() ? valueNode.asText() : valueNode.get("key").asText();
JsonNode valueObject = keyToObjectMap.get(key);

if (valueObject == null) {
throw new IllegalArgumentException("Key not found in propertyConfig: " + key);
}
newArray.add(valueNode.isTextual() ? valueObject : valueNode);
});

jsonNode.replace(fieldName, newArray);
entity.setExtension(JsonUtils.treeToValue(jsonNode, Object.class));
}
}

public final void storeExtension(EntityInterface entity) {
JsonNode jsonNode = JsonUtils.valueToTree(entity.getExtension());
Iterator<Entry<String, JsonNode>> customFields = jsonNode.fields();
Expand Down
83 changes: 65 additions & 18 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.List;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import javax.ws.rs.core.UriInfo;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.tuple.Triple;
Expand All @@ -40,6 +41,8 @@
import org.openmetadata.schema.type.Include;
import org.openmetadata.schema.type.Relationship;
import org.openmetadata.schema.type.customproperties.EnumConfig;
import org.openmetadata.schema.type.customproperties.EnumWithDescriptionsConfig;
import org.openmetadata.schema.type.customproperties.Value;
import org.openmetadata.service.Entity;
import org.openmetadata.service.TypeRegistry;
import org.openmetadata.service.resources.types.TypeResource;
Expand Down Expand Up @@ -169,24 +172,9 @@ private List<CustomProperty> getCustomProperties(Type type) {

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 "enum" -> validateEnumConfig(customProperty.getCustomPropertyConfig());
case "enumWithDescriptions" -> validateEnumWithDescriptionsConfig(
customProperty.getCustomPropertyConfig());
case "date-cp" -> validateDateFormat(
customProperty.getCustomPropertyConfig(), getDateTokens(), "Invalid date format");
case "dateTime-cp" -> validateDateFormat(
Expand Down Expand Up @@ -229,6 +217,44 @@ private Set<Character> getTimeTokens() {
return Set.of('H', 'h', 'm', 's', 'a', 'S');
}

private void validateEnumConfig(CustomPropertyConfig config) {
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.");
}
}

private void validateEnumWithDescriptionsConfig(CustomPropertyConfig config) {
if (config != null) {
EnumWithDescriptionsConfig enumWithDescriptionsConfig =
JsonUtils.convertValue(config.getConfig(), EnumWithDescriptionsConfig.class);
if (enumWithDescriptionsConfig == null
|| (enumWithDescriptionsConfig.getValues() != null
&& enumWithDescriptionsConfig.getValues().isEmpty())) {
throw new IllegalArgumentException(
"EnumWithDescriptions Custom Property Type must have customPropertyConfig populated with values.");
}
JsonUtils.validateJsonSchema(config.getConfig(), EnumWithDescriptionsConfig.class);
if (enumWithDescriptionsConfig.getValues().stream().map(Value::getKey).distinct().count()
!= enumWithDescriptionsConfig.getValues().size()) {
throw new IllegalArgumentException(
"EnumWithDescriptions Custom Property key cannot have duplicates.");
}
} else {
throw new IllegalArgumentException(
"EnumWithDescriptions Custom Property Type must have customPropertyConfig.");
}
}

/** Handles entity updated from PUT and POST operation. */
public class TypeUpdater extends EntityUpdater {
public TypeUpdater(Type original, Type updated, Operation operation) {
Expand Down Expand Up @@ -387,6 +413,27 @@ private void validatePropertyConfigUpdate(
throw new IllegalArgumentException(
"Existing Enum Custom Property values cannot be removed.");
}
} else if (origProperty.getPropertyType().getName().equals("enumWithDescriptions")) {
EnumWithDescriptionsConfig origConfig =
JsonUtils.convertValue(
origProperty.getCustomPropertyConfig().getConfig(),
EnumWithDescriptionsConfig.class);
EnumWithDescriptionsConfig updatedConfig =
JsonUtils.convertValue(
updatedProperty.getCustomPropertyConfig().getConfig(),
EnumWithDescriptionsConfig.class);
HashSet<String> updatedValues =
updatedConfig.getValues().stream()
.map(Value::getKey)
.collect(Collectors.toCollection(HashSet::new));
if (updatedValues.size() != updatedConfig.getValues().size()) {
throw new IllegalArgumentException(
"EnumWithDescriptions Custom Property values cannot have duplicates.");
} else if (!updatedValues.containsAll(
origConfig.getValues().stream().map(Value::getKey).collect(Collectors.toSet()))) {
throw new IllegalArgumentException(
"Existing EnumWithDescriptions Custom Property values cannot be removed.");
}
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions ...metadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,8 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr

public static Type ENUM_TYPE;

public static Type ENUM_WITH_DESCRIPTIONS_TYPE;

// Run webhook related tests randomly. This will ensure these tests are not run for every entity
// evey time junit tests are run to save time. But over the course of development of a release,
// when tests are run enough times, the webhook tests are run for all the entities.
Expand Down
Loading

0 comments on commit 1b029d2

Please sign in to comment.