yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-12-26 15:10:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixes #7396 Custom properties description are not updated (#7571)

Browse Source
This commit is contained in:
Suresh Srinivas 2022-09-19 22:47:49 -07:00 committed by GitHub
parent bfe5710dc7
commit 1128d4847d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 148 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java
View File

@ -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);
}

2
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java
View File

@ -1334,7 +1334,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
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);
}
}

36
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java
View File

@ -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<Type> {
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<CustomProperty> 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<Type> {
List<CustomProperty> origFields = listOrEmpty(original.getCustomProperties());
List<CustomProperty> added = new ArrayList<>();
List<CustomProperty> 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<Type> {
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());
}
}
}

4
openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java
View File

@ -185,7 +185,7 @@ public abstract class EntityResource<T extends EntityInterface, K extends Entity
boolean allowBots)
throws IOException {
OperationContext operationContext = new OperationContext(entityType, MetadataOperation.DELETE);
authorizer.authorize(securityContext, operationContext, getResourceContextById(id), true);
authorizer.authorize(securityContext, operationContext, getResourceContextById(id), allowBots);
DeleteResponse<T> response = dao.delete(securityContext.getUserPrincipal().getName(), id, recursive, hardDelete);
addHref(uriInfo, response.getEntity());
return response.toResponse();
@ -200,7 +200,7 @@ public abstract class EntityResource<T extends EntityInterface, K extends Entity
boolean allowBots)
throws IOException {
OperationContext operationContext = new OperationContext(entityType, MetadataOperation.DELETE);
authorizer.authorize(securityContext, operationContext, getResourceContextByName(name), true);
authorizer.authorize(securityContext, operationContext, getResourceContextByName(name), allowBots);
DeleteResponse<T> response =
dao.deleteByName(securityContext.getUserPrincipal().getName(), name, recursive, hardDelete);
addHref(uriInfo, response.getEntity());

7
openmetadata-service/src/main/java/org/openmetadata/service/resources/types/TypeResource.java
View File

@ -373,15 +373,16 @@ public class TypeResource extends EntityResource<Type, TypeRepository> {
@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,

11
openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java
View File

@ -141,7 +141,9 @@ public final class EntityUtil {
(ref1, ref2) -> ref1.getName().equals(ref2.getName()) && ref1.getEndpoint().equals(ref2.getEndpoint());
public static final BiPredicate<CustomProperty, CustomProperty> 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<Rule, Rule> 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;
}

7
openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java
View File

@ -316,6 +316,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
new TestSuiteResourceTest().setupTestSuites(test);
new TestDefinitionResourceTest().setupTestDefinitions(test);
new TestCaseResourceTest().setupTestCase(test);
new TypeResourceTest().setupTypes();
runWebhookTests = new Random().nextBoolean();
if (runWebhookTests) {
@ -1261,7 +1262,6 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
// PUT valid custom field intA to the entity type
TypeResourceTest typeResourceTest = new TypeResourceTest();
INT_TYPE = typeResourceTest.getEntityByName("integer", "", ADMIN_AUTH_HEADERS);
Type entityType = typeResourceTest.getEntityByName(this.entityType, "customProperties", ADMIN_AUTH_HEADERS);
CustomProperty fieldA =
new CustomProperty().withName("intA").withDescription("intA").withPropertyType(INT_TYPE.getEntityReference());
@ -1269,7 +1269,6 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
final UUID id = entityType.getId();
// PATCH valid custom field stringB
STRING_TYPE = typeResourceTest.getEntityByName("string", "", ADMIN_AUTH_HEADERS);
CustomProperty fieldB =
new CustomProperty()
.withName("stringB")
@ -1677,7 +1676,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
return updated;
}
private void validateEntityHistory(
protected void validateEntityHistory(
UUID id, UpdateType updateType, ChangeDescription expectedChangeDescription, Map<String, String> authHeaders)
throws IOException {
// GET ../entity/{id}/versions to list the all the versions of an entity
@ -1696,7 +1695,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
}
}
private void validateLatestVersion(
protected void validateLatestVersion(
EntityInterface entityInterface,
UpdateType updateType,
ChangeDescription expectedChangeDescription,

84
openmetadata-service/src/test/java/org/openmetadata/service/resources/metadata/TypeResourceTest.java
View File

@ -16,12 +16,18 @@ package org.openmetadata.service.resources.metadata;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import static org.openmetadata.service.util.EntityUtil.fieldAdded;
import static org.openmetadata.service.util.EntityUtil.fieldDeleted;
import static org.openmetadata.service.util.EntityUtil.fieldUpdated;
import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS;
import static org.openmetadata.service.util.TestUtils.UpdateType.MINOR_UPDATE;
import static org.openmetadata.service.util.TestUtils.assertCustomProperties;
import static org.openmetadata.service.util.TestUtils.assertResponse;
import static org.openmetadata.service.util.TestUtils.assertResponseContains;
import java.io.IOException;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.UUID;
@ -38,6 +44,7 @@ import org.openmetadata.schema.api.CreateType;
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.ChangeDescription;
import org.openmetadata.schema.type.EntityReference;
import org.openmetadata.service.Entity;
import org.openmetadata.service.resources.EntityResourceTest;
@ -46,6 +53,7 @@ import org.openmetadata.service.resources.types.TypeResource.TypeList;
import org.openmetadata.service.util.EntityUtil;
import org.openmetadata.service.util.JsonUtils;
import org.openmetadata.service.util.TestUtils;
import org.openmetadata.service.util.TestUtils.UpdateType;
@Slf4j
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
@ -61,7 +69,11 @@ public class TypeResourceTest extends EntityResourceTest<Type, CreateType> {
@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<Type, CreateType> {
}
@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<Type, CreateType> {
.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<Type, CreateType> {
return updated;
}
public Type addCustomPropertyAndCheck(
UUID entityTypeId,
CustomProperty customProperty,
Map<String, String> 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<String, String> authHeaders)
throws HttpResponseException {
@ -176,10 +241,7 @@ public class TypeResourceTest extends EntityResourceTest<Type, CreateType> {
if (fieldName.equals("customProperties")) {
List<CustomProperty> expectedProperties = (List<CustomProperty>) expected;
List<CustomProperty> 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);
}

18
openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java
View File

@ -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<CustomProperty> expected, List<CustomProperty> 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