Skip to content

Commit

Permalink
GEN -19588 Sort Enum type Custom Property Values (#19637)
Browse files Browse the repository at this point in the history 
* GEN -19588 Sort Enum type Custom Property Values

* fix py-tests

* use streams for sorting

(cherry picked from commit c0eb7d0)
  • Loading branch information
@sonika-shah
sonika-shah authored and OpenMetadata Release Bot committed Feb 11, 2025
1 parent 6a57f87 commit ebab122
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 31 deletions.
2 changes: 1 addition & 1 deletion ingestion/tests/integration/ometa/test_ometa_custom_properties_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
"description": "Rating of a table",
"propertyType": {"name": "enum"},
"customPropertyConfig": {
"config": {"values": ["Good", "Average", "Bad"], "multiSelect": False},
"config": {"values": ["Average", "Bad", "Good"], "multiSelect": False},
},
},
{
Expand Down
39 changes: 15 additions & 24 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 @@ -111,34 +111,25 @@ public static String getPropertyName(String propertyFQN) {
}

public static String getCustomPropertyType(String entityType, String propertyName) {
Type type = TypeRegistry.TYPES.get(entityType);
if (type != null && type.getCustomProperties() != null) {
for (CustomProperty property : type.getCustomProperties()) {
if (property.getName().equals(propertyName)) {
return property.getPropertyType().getName();
}
}
String fqn = getCustomPropertyFQN(entityType, propertyName);
CustomProperty property = CUSTOM_PROPERTIES.get(fqn);
if (property == null) {
throw EntityNotFoundException.byMessage(
CatalogExceptionMessage.entityNotFound(propertyName, entityType));
}
throw EntityNotFoundException.byMessage(
CatalogExceptionMessage.entityNotFound(Entity.TYPE, String.valueOf(type)));
return property.getPropertyType().getName();
}

public static String getCustomPropertyConfig(String entityType, String propertyName) {
Type type = TypeRegistry.TYPES.get(entityType);
if (type != null && type.getCustomProperties() != null) {
for (CustomProperty property : type.getCustomProperties()) {
if (property.getName().equals(propertyName)
&& property.getCustomPropertyConfig() != null
&& property.getCustomPropertyConfig().getConfig() != null) {
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
}
}
}
String fqn = getCustomPropertyFQN(entityType, propertyName);
CustomProperty property = CUSTOM_PROPERTIES.get(fqn);
if (property != null
&& property.getCustomPropertyConfig() != null
&& property.getCustomPropertyConfig().getConfig() != null) {
Object config = property.getCustomPropertyConfig().getConfig();
return (config instanceof String || config instanceof Integer)
? config.toString() // for simple type config return as string
: JsonUtils.pojoToJson(config); // for complex object in config return as JSON string
}
return null;
}
Expand Down
27 changes: 24 additions & 3 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 @@ -107,6 +107,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.ConstraintViolationException;
import javax.validation.constraints.NotNull;
Expand Down Expand Up @@ -1557,7 +1558,16 @@ private void validateAndUpdateExtensionBasedOnPropertyType(
jsonNode.put(fieldName, formattedValue);
}
case "table-cp" -> validateTableType(fieldValue, propertyConfig, fieldName);
case "enum" -> validateEnumKeys(fieldName, fieldValue, propertyConfig);
case "enum" -> {
validateEnumKeys(fieldName, fieldValue, propertyConfig);
List<String> enumValues =
StreamSupport.stream(fieldValue.spliterator(), false)
.map(JsonNode::asText)
.sorted()
.collect(Collectors.toList());
jsonNode.set(fieldName, JsonUtils.valueToTree(enumValues));
entity.setExtension(jsonNode);
}
default -> {}
}
}
Expand Down Expand Up @@ -1704,8 +1714,19 @@ public final Object getExtension(T entity) {
}
ObjectNode objectNode = JsonUtils.getObjectNode();
for (ExtensionRecord extensionRecord : records) {
String fieldName = TypeRegistry.getPropertyName(extensionRecord.extensionName());
objectNode.set(fieldName, JsonUtils.readTree(extensionRecord.extensionJson()));
String fieldName = extensionRecord.extensionName().substring(fieldFQNPrefix.length() + 1);
JsonNode fieldValue = JsonUtils.readTree(extensionRecord.extensionJson());
String customPropertyType = TypeRegistry.getCustomPropertyType(entityType, fieldName);
if ("enum".equals(customPropertyType) && fieldValue.isArray() && fieldValue.size() > 1) {
List<String> sortedEnumValues =
StreamSupport.stream(fieldValue.spliterator(), false)
.map(JsonNode::asText)
.sorted()
.collect(Collectors.toList());
fieldValue = JsonUtils.valueToTree(sortedEnumValues);
}

objectNode.set(fieldName, fieldValue);
}
return objectNode;
}
Expand Down
20 changes: 20 additions & 0 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 @@ -27,6 +27,7 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -174,6 +175,11 @@ private List<CustomProperty> getCustomProperties(Type type) {
for (Triple<String, String, String> result : results) {
CustomProperty property = JsonUtils.readValue(result.getRight(), CustomProperty.class);
property.setPropertyType(this.getReferenceByName(result.getMiddle(), NON_DELETED));

if ("enum".equals(property.getPropertyType().getName())) {
sortEnumKeys(property);
}

customProperties.add(property);
}
customProperties.sort(EntityUtil.compareCustomProperty);
Expand Down Expand Up @@ -270,6 +276,19 @@ private void validateTableTypeConfig(CustomPropertyConfig config) {
}
}

@SuppressWarnings("unchecked")
private void sortEnumKeys(CustomProperty property) {
Object enumConfig = property.getCustomPropertyConfig().getConfig();
if (enumConfig instanceof Map) {
Map<String, Object> configMap = (Map<String, Object>) enumConfig;
if (configMap.get("values") instanceof List) {
List<String> values = (List<String>) configMap.get("values");
List<String> sortedValues = values.stream().sorted().collect(Collectors.toList());
configMap.put("values", sortedValues);
}
}
}

/** 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 @@ -460,6 +479,7 @@ private void postUpdateCustomPropertyConfig(
Type entity, CustomProperty origProperty, CustomProperty updatedProperty) {
String updatedBy = entity.getUpdatedBy();
if (origProperty.getPropertyType().getName().equals("enum")) {
sortEnumKeys(updatedProperty);
EnumConfig origConfig =
JsonUtils.convertValue(
origProperty.getCustomPropertyConfig().getConfig(), EnumConfig.class);
Expand Down
11 changes: 8 additions & 3 deletions ...rvice/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,12 @@ void testGlossaryImportExport() throws IOException {
.withConfig(
Map.of(
"values",
List.of("single1", "single2", "single3", "single4", "\"single5\""),
List.of(
"\"single val with quotes\"",
"single1",
"single2",
"single3",
"single4"),
"multiSelect",
false))),
new CustomProperty()
Expand Down Expand Up @@ -911,7 +916,7 @@ void testGlossaryImportExport() throws IOException {
",g1,dsp1,\"dsc1,1\",h1;h2;h3,g1.g1t1;g2.g2t1,term1;http://term1,PII.None,user:%s,user:%s,%s,\"glossaryTermDateCp:18-09-2024;glossaryTermDateTimeCp:18-09-2024 01:09:34;glossaryTermDurationCp:PT5H30M10S;glossaryTermEmailCp:[email protected];glossaryTermEntRefCp:team:\"\"%s\"\";glossaryTermEntRefListCp:user:\"\"%s\"\"|user:\"\"%s\"\"\"",
reviewerRef.get(0), user1, "Approved", team11, user1, user2),
String.format(
",g2,dsp2,dsc3,h1;h3;h3,g1.g1t1;g2.g2t1,term2;https://term2,PII.NonSensitive,,user:%s,%s,\"glossaryTermEnumCpMulti:val3|val2|val1|val4|val5;glossaryTermEnumCpSingle:single1;glossaryTermIntegerCp:7777;glossaryTermMarkdownCp:# Sample Markdown Text;glossaryTermNumberCp:123456;\"\"glossaryTermQueryCp:select col,row from table where id ='30';\"\";glossaryTermStringCp:sample string content;glossaryTermTimeCp:10:08:45;glossaryTermTimeIntervalCp:1726142300000:17261420000;glossaryTermTimestampCp:1726142400000\"",
",g2,dsp2,dsc3,h1;h3;h3,g1.g1t1;g2.g2t1,term2;https://term2,PII.NonSensitive,,user:%s,%s,\"glossaryTermEnumCpMulti:val1|val2|val3|val4|val5;glossaryTermEnumCpSingle:single1;glossaryTermIntegerCp:7777;glossaryTermMarkdownCp:# Sample Markdown Text;glossaryTermNumberCp:123456;\"\"glossaryTermQueryCp:select col,row from table where id ='30';\"\";glossaryTermStringCp:sample string content;glossaryTermTimeCp:10:08:45;glossaryTermTimeIntervalCp:1726142300000:17261420000;glossaryTermTimestampCp:1726142400000\"",
user1, "Approved"),
String.format(
"importExportTest.g1,g11,dsp2,dsc11,h1;h3;h3,g1.g1t1;g2.g2t1,,,user:%s,team:%s,%s,",
Expand All @@ -924,7 +929,7 @@ void testGlossaryImportExport() throws IOException {
",g1,dsp1,new-dsc1,h1;h2;h3,g1.g1t1;importExportTest.g2;g2.g2t1,term1;http://term1,PII.None,user:%s,user:%s,%s,\"glossaryTermDateCp:18-09-2024;glossaryTermDateTimeCp:18-09-2024 01:09:34;glossaryTermDurationCp:PT5H30M10S;glossaryTermEmailCp:[email protected];glossaryTermEntRefCp:team:\"\"%s\"\";glossaryTermEntRefListCp:user:\"\"%s\"\"|user:\"\"%s\"\"\"",
reviewerRef.get(0), user1, "Approved", team11, user1, user2),
String.format(
",g2,dsp2,new-dsc3,h1;h3;h3,importExportTest.g1;g1.g1t1;g2.g2t1,term2;https://term2,PII.NonSensitive,user:%s,user:%s,%s,\"glossaryTermEnumCpMulti:val3|val2|val1|val4|val5;glossaryTermEnumCpSingle:single1;glossaryTermIntegerCp:7777;glossaryTermMarkdownCp:# Sample Markdown Text;glossaryTermNumberCp:123456;\"\"glossaryTermQueryCp:select col,row from table where id ='30';\"\";glossaryTermStringCp:sample string content;glossaryTermTimeCp:10:08:45;glossaryTermTimeIntervalCp:1726142300000:17261420000;glossaryTermTimestampCp:1726142400000\"",
",g2,dsp2,new-dsc3,h1;h3;h3,importExportTest.g1;g1.g1t1;g2.g2t1,term2;https://term2,PII.NonSensitive,user:%s,user:%s,%s,\"glossaryTermEnumCpMulti:val1|val2|val3|val4|val5;glossaryTermEnumCpSingle:single1;glossaryTermIntegerCp:7777;glossaryTermMarkdownCp:# Sample Markdown Text;glossaryTermNumberCp:123456;\"\"glossaryTermQueryCp:select col,row from table where id ='30';\"\";glossaryTermStringCp:sample string content;glossaryTermTimeCp:10:08:45;glossaryTermTimeIntervalCp:1726142300000:17261420000;glossaryTermTimestampCp:1726142400000\"",
user1, user2, "Approved"),
String.format(
"importExportTest.g1,g11,dsp2,new-dsc11,h1;h3;h3,,,,user:%s,team:%s,%s,\"\"\"glossaryTermTableCol1Cp:row_1_col1_Value,,\"\";\"\"glossaryTermTableCol3Cp:row_1_col1_Value,row_1_col2_Value,row_1_col3_Value|row_2_col1_Value,row_2_col2_Value,row_2_col3_Value\"\"\"",
Expand Down

0 comments on commit ebab122

Please sign in to comment.