From f7ee1c76a03570a4a9f23b089d85cdf3924987e0 Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Sun, 17 Sep 2023 09:37:29 -0700 Subject: [PATCH] Code cleanup necessary for Glossary workflow (#13219) --- .../service/jdbi3/EntityRepository.java | 20 ++++----- .../service/jdbi3/FeedRepository.java | 44 ++++++++++++------- .../service/jdbi3/GlossaryRepository.java | 4 +- .../service/jdbi3/GlossaryTermRepository.java | 4 +- .../service/resources/EntityResource.java | 1 - .../glossary/GlossaryTermResourceTest.java | 22 +++++----- 6 files changed, 51 insertions(+), 44 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java index 7ffec6d4d25..9747d494e48 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java @@ -198,6 +198,7 @@ public abstract class EntityRepository { protected final boolean supportsVotes; @Getter protected final boolean supportsDomain; protected final boolean supportsDataProducts; + @Getter protected final boolean supportsReviewers; protected boolean quoteFqn = false; // Entity fqns not hierarchical such user, teams, services need to be quoted /** Fields that can be updated during PATCH operation */ @@ -256,6 +257,11 @@ public abstract class EntityRepository { this.patchFields.addField(allowedFields, FIELD_DOMAIN); this.putFields.addField(allowedFields, FIELD_DOMAIN); } + this.supportsReviewers = allowedFields.contains(FIELD_REVIEWERS); + if (supportsReviewers) { + this.patchFields.addField(allowedFields, FIELD_REVIEWERS); + this.putFields.addField(allowedFields, FIELD_REVIEWERS); + } this.supportsDataProducts = allowedFields.contains(FIELD_DATA_PRODUCTS); if (supportsDataProducts) { this.patchFields.addField(allowedFields, FIELD_DATA_PRODUCTS); @@ -626,7 +632,6 @@ public abstract class EntityRepository { public final T create(UriInfo uriInfo, T entity) { entity = withHref(uriInfo, createInternal(entity)); - postCreate(entity); return entity; } @@ -685,16 +690,6 @@ public abstract class EntityRepository { } public final PutResponse createOrUpdate(UriInfo uriInfo, T updated) { - PutResponse response = createOrUpdateInternal(uriInfo, updated); - if (response.getStatus() == Status.CREATED) { - postCreate(response.getEntity()); - } else if (response.getStatus() == Status.OK) { - postUpdate(response.getEntity()); - } - return response; - } - - public final PutResponse createOrUpdateInternal(UriInfo uriInfo, T updated) { T original = JsonUtils.readValue(dao.findJsonByFqn(updated.getFullyQualifiedName(), ALL), entityClass); if (original == null) { // If an original entity does not exist then create it, else update return new PutResponse<>(Status.CREATED, withHref(uriInfo, createNewEntity(updated)), RestUtil.ENTITY_CREATED); @@ -785,7 +780,7 @@ public abstract class EntityRepository { .withCurrentVersion(entity.getVersion()) .withPreviousVersion(change.getPreviousVersion()); entity.setChangeDescription(change); - postUpdate(JsonUtils.deepCopy(entity, entityClass)); + postUpdate(entity); return new PutResponse<>(Status.OK, changeEvent, RestUtil.ENTITY_FIELDS_CHANGED); } @@ -1014,6 +1009,7 @@ public abstract class EntityRepository { storeExtension(entity); storeRelationshipsInternal(entity); setInheritedFields(entity, new Fields(allowedFields)); + postCreate(entity); return entity; } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java index d1eaf23a49d..653856d3e4e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java @@ -21,6 +21,7 @@ import static org.openmetadata.schema.type.Relationship.ADDRESSED_TO; import static org.openmetadata.schema.type.Relationship.CREATED; import static org.openmetadata.schema.type.Relationship.IS_ABOUT; import static org.openmetadata.schema.type.Relationship.REPLIED_TO; +import static org.openmetadata.schema.type.TaskStatus.Open; import static org.openmetadata.service.Entity.USER; import static org.openmetadata.service.exception.CatalogExceptionMessage.ANNOUNCEMENT_INVALID_START_TIME; import static org.openmetadata.service.exception.CatalogExceptionMessage.ANNOUNCEMENT_OVERLAP; @@ -101,7 +102,7 @@ import org.openmetadata.service.util.ResultList; @Slf4j public class FeedRepository { private final CollectionDAO dao; - private static final MessageDecorator feedMessageFormatter = new FeedMessageDecorator(); + private static final MessageDecorator FEED_MESSAGE_FORMATTER = new FeedMessageDecorator(); public FeedRepository(CollectionDAO dao) { this.dao = dao; @@ -307,7 +308,7 @@ public class FeedRepository { closeTask(threadContext, user, new CloseTask()); } - private String getTagFQNs(List tags) { + private static String getTagFQNs(List tags) { return tags.stream().map(TagLabel::getTagFQN).collect(Collectors.joining(", ")); } @@ -315,26 +316,16 @@ public class FeedRepository { // Add a post to the task String message; if (closingComment != null) { - message = String.format("Closed the Task with comment - %s", closingComment); + message = closeTaskMessage(closingComment); } else { // The task was resolved with an update. // Add a default message to the Task thread with updated description/tag TaskDetails task = thread.getTask(); TaskType type = task.getType(); if (EntityUtil.isDescriptionTask(type)) { - message = - String.format( - "Resolved the Task with Description - %s", - feedMessageFormatter.getPlaintextDiff(task.getOldValue(), task.getNewValue())); + message = resolveDescriptionTaskMessage(task); } else if (EntityUtil.isTagTask(type)) { - String oldValue = - task.getOldValue() != null - ? getTagFQNs(JsonUtils.readObjects(task.getOldValue(), TagLabel.class)) - : StringUtils.EMPTY; - String newValue = getTagFQNs(JsonUtils.readObjects(task.getNewValue(), TagLabel.class)); - message = - String.format( - "Resolved the Task with Tag(s) - %s", feedMessageFormatter.getPlaintextDiff(oldValue, newValue)); + message = resolveTagTaskMessage(task); } else { message = "Resolved the Task."; } @@ -352,6 +343,9 @@ public class FeedRepository { private void closeTask(ThreadContext threadContext, String user, CloseTask closeTask) { Thread thread = threadContext.getThread(); TaskDetails task = thread.getTask(); + if (task.getStatus() != Open) { + return; + } TaskWorkflow workflow = threadContext.getTaskWorkflow(); workflow.closeTask(user, closeTask); task.withStatus(TaskStatus.Closed).withClosedBy(user).withClosedAt(System.currentTimeMillis()); @@ -971,6 +965,26 @@ public class FeedRepository { return user != null ? FullyQualifiedName.buildHash(user.getFullyQualifiedName()) : null; } + public static String resolveDescriptionTaskMessage(TaskDetails task) { + return String.format( + "Resolved the Task with Description - %s", + FEED_MESSAGE_FORMATTER.getPlaintextDiff(task.getOldValue(), task.getNewValue())); + } + + public static String resolveTagTaskMessage(TaskDetails task) { + String oldValue = + task.getOldValue() != null + ? getTagFQNs(JsonUtils.readObjects(task.getOldValue(), TagLabel.class)) + : StringUtils.EMPTY; + String newValue = getTagFQNs(JsonUtils.readObjects(task.getNewValue(), TagLabel.class)); + return String.format( + "Resolved the Task with Tag(s) - %s", FEED_MESSAGE_FORMATTER.getPlaintextDiff(oldValue, newValue)); + } + + public static String closeTaskMessage(String closingComment) { + return String.format("Closed the Task with comment - %s", closingComment); + } + public static class FilteredThreads { @Getter private final List threads; @Getter private final int totalCount; diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java index 044bd515443..c52c9970b5e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java @@ -62,8 +62,8 @@ import org.openmetadata.service.util.FullyQualifiedName; @Slf4j public class GlossaryRepository extends EntityRepository { - private static final String UPDATE_FIELDS = "reviewers"; - private static final String PATCH_FIELDS = "reviewers"; + private static final String UPDATE_FIELDS = ""; + private static final String PATCH_FIELDS = ""; public GlossaryRepository(CollectionDAO dao) { super( diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java index 714d5e07d53..95848bb6ceb 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java @@ -56,8 +56,8 @@ import org.openmetadata.service.util.RestUtil; @Slf4j public class GlossaryTermRepository extends EntityRepository { - private static final String UPDATE_FIELDS = "references,relatedTerms,reviewers,synonyms"; - private static final String PATCH_FIELDS = "references,relatedTerms,reviewers,synonyms"; + private static final String UPDATE_FIELDS = "references,relatedTerms,synonyms"; + private static final String PATCH_FIELDS = "references,relatedTerms,synonyms"; public GlossaryTermRepository(CollectionDAO dao) { super( diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java index 28c97aea1f6..74d63221904 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java @@ -245,7 +245,6 @@ public abstract class EntityResource response = repository.patch(uriInfo, id, securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); - repository.postUpdate(response.getEntity()); return response.toResponse(); } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java index 91ee64c349c..f3ea946bef2 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java @@ -86,7 +86,7 @@ import org.openmetadata.service.util.TestUtils.UpdateType; @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class GlossaryTermResourceTest extends EntityResourceTest { - private final GlossaryResourceTest glossaryResourceTest = new GlossaryResourceTest(); + private final GlossaryResourceTest glossaryTest = new GlossaryResourceTest(); public GlossaryTermResourceTest() { super( @@ -189,8 +189,8 @@ public class GlossaryTermResourceTest extends EntityResourceTest glossaryResourceTest.deleteEntity(g1.getId(), ADMIN_AUTH_HEADERS), - BAD_REQUEST, - entityIsNotEmpty(GLOSSARY)); + () -> glossaryTest.deleteEntity(g1.getId(), ADMIN_AUTH_HEADERS), BAD_REQUEST, entityIsNotEmpty(GLOSSARY)); // t1 is not empty and can't be deleted assertResponse(() -> deleteEntity(t1.getId(), ADMIN_AUTH_HEADERS), BAD_REQUEST, entityIsNotEmpty(GLOSSARY_TERM)); @@ -368,8 +366,8 @@ public class GlossaryTermResourceTest extends EntityResourceTest reviewers, EntityReference owner) throws IOException { - return createGlossary(glossaryResourceTest.getEntityName(test), reviewers, owner); + return createGlossary(glossaryTest.getEntityName(test), reviewers, owner); } public Glossary createGlossary(String name, List reviewers, EntityReference owner) throws IOException { - CreateGlossary create = glossaryResourceTest.createRequest(name).withReviewers(getFqns(reviewers)).withOwner(owner); - return glossaryResourceTest.createAndCheckEntity(create, ADMIN_AUTH_HEADERS); + CreateGlossary create = glossaryTest.createRequest(name).withReviewers(getFqns(reviewers)).withOwner(owner); + return glossaryTest.createAndCheckEntity(create, ADMIN_AUTH_HEADERS); } public Glossary getGlossary(String name) throws IOException { - return glossaryResourceTest.getEntityByName(name, glossaryResourceTest.getAllowedFields(), ADMIN_AUTH_HEADERS); + return glossaryTest.getEntityByName(name, glossaryTest.getAllowedFields(), ADMIN_AUTH_HEADERS); } }