From b3472b03ea85b11eca7ea38def5bf1cddf4ee34e Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Wed, 16 Nov 2022 17:57:54 -0800 Subject: [PATCH] Fixes #2254 Tag usage count is incorrect when a tag name is a prefix of another tag name (#8826) --- .../service/jdbi3/CollectionDAO.java | 7 ++-- .../openmetadata/service/util/EntityUtil.java | 21 ++++++++++-- .../glossary/GlossaryResourceTest.java | 15 ++------ .../glossary/GlossaryTermResourceTest.java | 34 +++++++++++++++++-- .../resources/tags/TagResourceTest.java | 2 +- 5 files changed, 58 insertions(+), 21 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java index 9ae3a5b11c3..d9247d384ce 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java @@ -1884,8 +1884,11 @@ public interface CollectionDAO { @SqlQuery("SELECT source, tagFQN, labelType, state FROM tag_usage WHERE targetFQN = :targetFQN ORDER BY tagFQN") List getTagsInternal(@Bind("targetFQN") String targetFQN); - @SqlQuery("SELECT COUNT(*) FROM tag_usage WHERE tagFQN LIKE CONCAT(:fqnPrefix, '%') AND source = :source") - int getTagCount(@Bind("source") int source, @Bind("fqnPrefix") String fqnPrefix); + @SqlQuery( + "SELECT COUNT(*) FROM tag_usage " + + "WHERE (tagFQN LIKE CONCAT(:tagFqn, '.%') OR tagFQN = :tagFqn) " + + "AND source = :source") + int getTagCount(@Bind("source") int source, @Bind("tagFqn") String tagFqn); @SqlUpdate("DELETE FROM tag_usage where targetFQN = :targetFQN") void deleteTagsByTarget(@Bind("targetFQN") String targetFQN); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java index ff79107edf8..5d59dbbfa4c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java @@ -92,7 +92,6 @@ public final class EntityUtil { public static final Comparator compareCustomProperty = Comparator.comparing(CustomProperty::getName); public static final Comparator compareFilters = Comparator.comparing(Filters::getEventType); public static final Comparator compareEventFilters = Comparator.comparing(EventFilter::getEntityType); - public static final Comparator compareOperation = Comparator.comparing(MetadataOperation::value); // // Matchers used for matching two items in a list @@ -425,14 +424,30 @@ public final class EntityUtil { .withDeleted(from.getDeleted()); } - public static TagLabel getTagLabel(GlossaryTerm term) { + public static List toTagLabels(GlossaryTerm... terms) { + List list = new ArrayList<>(); + for (GlossaryTerm term : terms) { + list.add(toTagLabel(term)); + } + return list; + } + + public static List toTagLabels(Tag... tags) { + List list = new ArrayList<>(); + for (Tag tag : tags) { + list.add(toTagLabel(tag)); + } + return list; + } + + public static TagLabel toTagLabel(GlossaryTerm term) { return new TagLabel() .withTagFQN(term.getFullyQualifiedName()) .withDescription(term.getDescription()) .withSource(TagSource.GLOSSARY); } - public static TagLabel getTagLabel(Tag tag) { + public static TagLabel toTagLabel(Tag tag) { return new TagLabel() .withTagFQN(tag.getFullyQualifiedName()) .withDescription(tag.getDescription()) diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java index ee4d53d5f4a..9fbd0e6ff57 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java @@ -25,6 +25,7 @@ import static org.openmetadata.service.util.EntityUtil.fieldDeleted; import static org.openmetadata.service.util.EntityUtil.fieldUpdated; import static org.openmetadata.service.util.EntityUtil.getEntityReference; import static org.openmetadata.service.util.EntityUtil.getFqn; +import static org.openmetadata.service.util.EntityUtil.toTagLabels; 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.assertListNull; @@ -33,7 +34,6 @@ import static org.openmetadata.service.util.TestUtils.validateTagLabel; import java.io.IOException; import java.net.URISyntaxException; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -58,7 +58,6 @@ import org.openmetadata.schema.type.ColumnDataType; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.ProviderType; import org.openmetadata.schema.type.TagLabel; -import org.openmetadata.schema.type.TagLabel.TagSource; import org.openmetadata.service.Entity; import org.openmetadata.service.exception.CatalogExceptionMessage; import org.openmetadata.service.resources.EntityResourceTest; @@ -102,7 +101,7 @@ public class GlossaryResourceTest extends EntityResourceTest toTagLabels(GlossaryTerm... terms) { - List list = new ArrayList<>(); - for (GlossaryTerm term : terms) { - list.add(new TagLabel().withTagFQN(term.getFullyQualifiedName()).withSource(TagSource.GLOSSARY)); - } - return list; - } - public void renameGlossaryAndCheck(Glossary glossary, String newName) throws IOException { String oldName = glossary.getName(); String json = JsonUtils.pojoToJson(glossary); 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 4512ef511da..e0123c0aa17 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 @@ -34,6 +34,7 @@ import static org.openmetadata.service.util.EntityUtil.fieldDeleted; import static org.openmetadata.service.util.EntityUtil.fieldUpdated; import static org.openmetadata.service.util.EntityUtil.getEntityReference; import static org.openmetadata.service.util.EntityUtil.getId; +import static org.openmetadata.service.util.EntityUtil.toTagLabels; 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.UpdateType.NO_CHANGE; @@ -180,6 +181,33 @@ public class GlossaryTermResourceTest extends EntityResourceTest aa -> aaa; + GlossaryTerm a = createTerm(glossary, null, "a", null); + GlossaryTerm aa = createTerm(glossary, null, "aa", null); + GlossaryTerm aaa = createTerm(glossary, null, "aaa", null); + + // Apply each of the tag to a table + TableResourceTest tableResourceTest = new TableResourceTest(); + CreateTable createTable = tableResourceTest.createRequest(getEntityName(test)).withTags(toTagLabels(a, aa, aaa)); + tableResourceTest.createEntity(createTable, ADMIN_AUTH_HEADERS); + + // Ensure prefix based tagLabel doesn't double count due too common prefix + for (GlossaryTerm term : List.of(a, aa, aaa)) { + term = getEntity(term.getId(), "usageCount", ADMIN_AUTH_HEADERS); + assertEquals(1, term.getUsageCount()); + } + } + @Test void patch_addDeleteReviewers(TestInfo test) throws IOException { CreateGlossaryTerm create = createRequest(getEntityName(test), "", "", null).withReviewers(null).withSynonyms(null); @@ -267,18 +295,18 @@ public class GlossaryTermResourceTest extends EntityResourceTest