Fixes #2254 Tag usage count is incorrect when a tag name is a prefix of another tag name (#8826)

This commit is contained in:
Suresh Srinivas 2022-11-16 17:57:54 -08:00 committed by GitHub
parent 422eccf6e7
commit b3472b03ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 58 additions and 21 deletions

View File

@ -1884,8 +1884,11 @@ public interface CollectionDAO {
@SqlQuery("SELECT source, tagFQN, labelType, state FROM tag_usage WHERE targetFQN = :targetFQN ORDER BY tagFQN")
List<TagLabel> 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);

View File

@ -92,7 +92,6 @@ public final class EntityUtil {
public static final Comparator<CustomProperty> compareCustomProperty = Comparator.comparing(CustomProperty::getName);
public static final Comparator<Filters> compareFilters = Comparator.comparing(Filters::getEventType);
public static final Comparator<EventFilter> compareEventFilters = Comparator.comparing(EventFilter::getEntityType);
public static final Comparator<MetadataOperation> 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<TagLabel> toTagLabels(GlossaryTerm... terms) {
List<TagLabel> list = new ArrayList<>();
for (GlossaryTerm term : terms) {
list.add(toTagLabel(term));
}
return list;
}
public static List<TagLabel> toTagLabels(Tag... tags) {
List<TagLabel> 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())

View File

@ -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<Glossary, CreateGlo
.withReviewers(GLOSSARY1.getReviewers());
GLOSSARY1_TERM1 = glossaryTermResourceTest.createAndCheckEntity(createGlossaryTerm, ADMIN_AUTH_HEADERS);
GLOSSARY1_TERM1_REF = GLOSSARY1_TERM1.getEntityReference();
GLOSSARY1_TERM1_LABEL = EntityUtil.getTagLabel(GLOSSARY1_TERM1);
GLOSSARY1_TERM1_LABEL = EntityUtil.toTagLabel(GLOSSARY1_TERM1);
validateTagLabel(GLOSSARY1_TERM1_LABEL);
createGlossaryTerm =
@ -113,7 +112,7 @@ public class GlossaryResourceTest extends EntityResourceTest<Glossary, CreateGlo
.withReviewers(GLOSSARY1.getReviewers());
GLOSSARY2_TERM1 = glossaryTermResourceTest.createAndCheckEntity(createGlossaryTerm, ADMIN_AUTH_HEADERS);
GLOSSARY2_TERM1_REF = GLOSSARY2_TERM1.getEntityReference();
GLOSSARY2_TERM1_LABEL = EntityUtil.getTagLabel(GLOSSARY2_TERM1);
GLOSSARY2_TERM1_LABEL = EntityUtil.toTagLabel(GLOSSARY2_TERM1);
validateTagLabel(GLOSSARY2_TERM1_LABEL);
}
@ -375,14 +374,6 @@ public class GlossaryResourceTest extends EntityResourceTest<Glossary, CreateGlo
return resource.createEntity(create, ADMIN_AUTH_HEADERS);
}
public static List<TagLabel> toTagLabels(GlossaryTerm... terms) {
List<TagLabel> 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);

View File

@ -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<GlossaryTerm, C
assertEquals(create.getReviewers(), t12.getReviewers()); // Reviewers are inherited
}
@Test
void test_commonPrefixTagLabelCount(TestInfo test) throws IOException {
//
// Create glossary terms that start with common prefix and make sure usage count is correct
//
GlossaryResourceTest glossaryTest = new GlossaryResourceTest();
CreateGlossary create =
glossaryTest.createRequest(getEntityName(test)).withReviewers(List.of(USER1_REF)).withDescription("d");
Glossary glossary = glossaryTest.createEntity(create, ADMIN_AUTH_HEADERS);
// Create nested terms a -> 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<GlossaryTerm, C
CreateGlossaryTerm create = createRequest("t1", "", "", null).withGlossary(g1Ref);
GlossaryTerm t1 = createEntity(create, ADMIN_AUTH_HEADERS);
EntityReference tRef1 = t1.getEntityReference();
TagLabel t1Label = EntityUtil.getTagLabel(t1);
TagLabel t1Label = EntityUtil.toTagLabel(t1);
// Create glossary term t11 under t1
create = createRequest("t11with'quote", "", "", null).withReviewers(null).withGlossary(g1Ref).withParent(tRef1);
GlossaryTerm t11 = createEntity(create, ADMIN_AUTH_HEADERS);
EntityReference tRef11 = t11.getEntityReference();
TagLabel t11Label = EntityUtil.getTagLabel(t11);
TagLabel t11Label = EntityUtil.toTagLabel(t11);
// Create glossary term t111 under t11
create = createRequest("t111", "", "", null).withReviewers(null).withGlossary(g1Ref).withParent(tRef11);
GlossaryTerm t111 = createEntity(create, ADMIN_AUTH_HEADERS);
TagLabel t111Label = EntityUtil.getTagLabel(t111);
TagLabel t111Label = EntityUtil.toTagLabel(t111);
// Assign glossary terms to a table
// t1 assigned to table. t11 assigned column1 and t111 assigned to column2

View File

@ -100,7 +100,7 @@ public class TagResourceTest extends OpenMetadataApplicationTest {
}
private TagLabel getTagLabel(String tagName) throws HttpResponseException {
return EntityUtil.getTagLabel(TagResourceTest.getTag(tagName, ADMIN_AUTH_HEADERS));
return EntityUtil.toTagLabel(TagResourceTest.getTag(tagName, ADMIN_AUTH_HEADERS));
}
@Test