Skip to content

Commit

Permalink
Fixes #2899 - Not getting proper relation between parent and child gl…
Browse files Browse the repository at this point in the history 
…ossary-terms
  • Loading branch information
@sureshms
sureshms committed Feb 23, 2022
1 parent ba97b40 commit 1780909
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 49 deletions.
8 changes: 4 additions & 4 deletions ...log-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/GlossaryTermRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ public GlossaryTerm setFields(GlossaryTerm entity, Fields fields) throws IOExcep

private EntityReference getParent(GlossaryTerm entity) throws IOException {
List<String> ids =
findFrom(entity.getId(), Entity.GLOSSARY_TERM, Relationship.PARENT_OF, Entity.GLOSSARY, entity.getDeleted());
findFrom(
entity.getId(), Entity.GLOSSARY_TERM, Relationship.PARENT_OF, Entity.GLOSSARY_TERM, entity.getDeleted());
return ids.size() == 1 ? Entity.getEntityReference(Entity.GLOSSARY_TERM, UUID.fromString(ids.get(0))) : null;
}

private List<EntityReference> getChildren(GlossaryTerm entity) throws IOException {
List<String> ids =
findBoth(
entity.getId(), Entity.GLOSSARY_TERM, Relationship.PARENT_OF, Entity.GLOSSARY_TERM, entity.getDeleted());
findTo(entity.getId(), Entity.GLOSSARY_TERM, Relationship.PARENT_OF, Entity.GLOSSARY_TERM, entity.getDeleted());
List<EntityReference> children = new ArrayList<>();
for (String id : ids) {
children.add(Entity.getEntityReference(Entity.GLOSSARY_TERM, UUID.fromString(id)));
Expand Down Expand Up @@ -120,7 +120,7 @@ public void prepare(GlossaryTerm entity) throws IOException, ParseException {
entity.setFullyQualifiedName(entity.getGlossary().getName() + "." + entity.getName());
} else {
EntityReference parent = Entity.getEntityReference(entity.getParent());
entity.setFullyQualifiedName(entity.getParent().getName() + "." + entity.getName());
entity.setFullyQualifiedName(parent.getName() + "." + entity.getName());
entity.setParent(parent);
}

Expand Down
1 change: 0 additions & 1 deletion catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ public void storeEntity(Table table, boolean update) throws IOException {
@Override
public void storeRelationships(Table table) {
// Add relationship from database to table
String databaseId = table.getDatabase().getId().toString();
addRelationship(table.getDatabase().getId(), table.getId(), DATABASE, TABLE, Relationship.CONTAINS);

// Add table owner relationship
Expand Down
1 change: 1 addition & 0 deletions ...rvice/src/main/java/org/openmetadata/catalog/resources/glossary/GlossaryTermResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public static GlossaryTerm addHref(UriInfo uriInfo, GlossaryTerm term) {
term.withHref(RestUtil.getHref(uriInfo, COLLECTION_PATH, term.getId()));
Entity.withHref(uriInfo, term.getGlossary());
Entity.withHref(uriInfo, term.getParent());
Entity.withHref(uriInfo, term.getChildren());
Entity.withHref(uriInfo, term.getRelatedTerms());
Entity.withHref(uriInfo, term.getReviewers());
return term;
Expand Down
4 changes: 4 additions & 0 deletions catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.joda.time.Period;
import org.joda.time.format.ISOPeriodFormat;
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.entity.data.GlossaryTerm;
import org.openmetadata.catalog.entity.teams.Team;
import org.openmetadata.catalog.entity.teams.User;
import org.openmetadata.catalog.exception.CatalogExceptionMessage;
Expand Down Expand Up @@ -123,6 +124,9 @@ public final class EntityUtil {
(filter1, filter2) ->
filter1.getEventType().equals(filter2.getEventType()) && filter1.getEntities().equals(filter2.getEntities());

public static final BiPredicate<GlossaryTerm, GlossaryTerm> glossaryTermMatch =
(filter1, filter2) -> filter1.getFullyQualifiedName().equals(filter2.getFullyQualifiedName());

private EntityUtil() {}

/** Validate Ingestion Schedule */
Expand Down
8 changes: 3 additions & 5 deletions catalog-rest-service/src/main/resources/json/schema/entity/data/glossary.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@
"default": false
}
},
"required": [
"id",
"name"
]
}
"required": ["id", "name"],
"additionalProperties": false
}
28 changes: 13 additions & 15 deletions catalog-rest-service/src/main/resources/json/schema/entity/data/glossaryTerm.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
"type": "string",
"format": "uri"
}
}
},
"additionalProperties": false
},
"status" : {
"type" : "string",
"status": {
"type": "string",
"enum": ["Draft", "Approved", "Deprecated"],
"default": "Draft"
}
Expand Down Expand Up @@ -62,12 +63,12 @@
}
},
"glossary": {
"description": "Glosary that this term belongs to.",
"$ref" : "../../type/entityReference.json"
"description": "Glossary that this term belongs to.",
"$ref": "../../type/entityReference.json"
},
"parent": {
"description": "Parent glossary term that this term is child of. When `null` this term is the root term of the glossary.",
"$ref" : "../../type/entityReference.json"
"$ref": "../../type/entityReference.json"
},
"children": {
"description": "Other glossary terms that are children of this glossary term.",
Expand Down Expand Up @@ -117,19 +118,16 @@
"description": "Change that lead to this version of the entity.",
"$ref": "../../type/entityHistory.json#/definitions/changeDescription"
},
"status" : {
"description" : "Status of the glossary term.",
"$ref" : "#/definitions/status"
"status": {
"description": "Status of the glossary term.",
"$ref": "#/definitions/status"
},
"deleted": {
"description": "When `true` indicates the entity has been soft deleted.",
"type": "boolean",
"default": false
}
},
"required": [
"id",
"name",
"glossary"
]
}
"required": ["id", "name", "glossary"],
"additionalProperties": false
}
2 changes: 1 addition & 1 deletion ...log-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ void post_entityWithDots_200(TestInfo test) throws HttpResponseException {
String[] split = entityInterface.getFullyQualifiedName().split("\\.");
String actualName = split[split.length - 1];
assertTrue(actualName.contains("foo_DOT_bar"));
assertTrue(!actualName.contains("foo.bar"));
assertFalse(actualName.contains("foo.bar"));
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
64 changes: 41 additions & 23 deletions ...e/src/test/java/org/openmetadata/catalog/resources/glossary/GlossaryTermResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.glossaryTermMismatch;
import static org.openmetadata.catalog.util.TestUtils.ADMIN_AUTH_HEADERS;
Expand Down Expand Up @@ -114,8 +115,12 @@ void get_listGlossaryTermsWithDifferentFilters() throws HttpResponseException {
Glossary glossary1 = glossaryResourceTest.createEntity(createGlossary, ADMIN_AUTH_HEADERS);

GlossaryTerm term1 = createTerm(glossary1, null, "term1");
createTerm(glossary1, term1, "term11");
createTerm(glossary1, term1, "term12");
GlossaryTerm term11 = createTerm(glossary1, term1, "term11");
GlossaryTerm term12 = createTerm(glossary1, term1, "term12");
term1.setChildren(
List.of(
new GlossaryTermEntityInterface(term11).getEntityReference(),
new GlossaryTermEntityInterface(term12).getEntityReference()));

// Create the following glossary
// glossary2
Expand All @@ -126,43 +131,45 @@ void get_listGlossaryTermsWithDifferentFilters() throws HttpResponseException {
Glossary glossary2 = glossaryResourceTest.createEntity(createGlossary, ADMIN_AUTH_HEADERS);

GlossaryTerm term2 = createTerm(glossary2, null, "term2");
createTerm(glossary2, term2, "term21");
createTerm(glossary2, term2, "term22");
GlossaryTerm term21 = createTerm(glossary2, term2, "term21");
GlossaryTerm term22 = createTerm(glossary2, term2, "term22");
term2.setChildren(
List.of(
new GlossaryTermEntityInterface(term21).getEntityReference(),
new GlossaryTermEntityInterface(term22).getEntityReference()));

// List terms without any filters
ResultList<GlossaryTerm> list = listEntities(null, ADMIN_AUTH_HEADERS);
List<String> expectedTerms =
Arrays.asList(
GLOSSARY_TERM1.getName(),
GLOSSARY_TERM2.getName(),
"term1",
"term11",
"term12",
"term2",
"term21",
"term22");
assertContains(list, expectedTerms);
Map<String, String> queryParams = new HashMap<>();
queryParams.put("fields", "children,relatedTerms,reviewers,tags");
ResultList<GlossaryTerm> list = listEntities(queryParams, ADMIN_AUTH_HEADERS);
List<GlossaryTerm> expectedTerms =
Arrays.asList(GLOSSARY_TERM1, GLOSSARY_TERM2, term1, term11, term12, term2, term21, term22);
assertContains(expectedTerms, list.getData());

// List terms under glossary1
Map<String, String> queryParams = new HashMap<>();
queryParams = new HashMap<>();
queryParams.put("fields", "children,relatedTerms,reviewers,tags");
queryParams.put("glossary", glossary1.getId().toString());
list = listEntities(queryParams, ADMIN_AUTH_HEADERS);
assertContains(list, Arrays.asList("term1", "term11", "term12"));
assertContains(Arrays.asList(term1, term11, term12), list.getData());

// List terms under glossary1 parent term1
queryParams = new HashMap<>();
queryParams.put("fields", "children,relatedTerms,reviewers,tags");
queryParams.put("glossary", glossary1.getId().toString());
queryParams.put("parent", term1.getId().toString());
list = listEntities(queryParams, ADMIN_AUTH_HEADERS);
assertContains(list, Arrays.asList("term11", "term12"));
assertContains(Arrays.asList(term11, term12), list.getData());

// List terms under glossary2
queryParams = new HashMap<>();
queryParams.put("fields", "children,relatedTerms,reviewers,tags");
queryParams.put("glossary", glossary2.getId().toString());
list = listEntities(queryParams, ADMIN_AUTH_HEADERS);
assertContains(list, Arrays.asList("term2", "term21", "term22"));
assertContains(Arrays.asList(term2, term21, term22), list.getData());

// List terms under glossary 2 but give glossary term1 in glossary 1 as parent
queryParams.put("fields", "children,relatedTerms,reviewers,tags");
queryParams.put("parent", term1.getId().toString());
Map<String, String> map = Collections.unmodifiableMap(queryParams);
assertResponse(
Expand All @@ -179,9 +186,20 @@ public GlossaryTerm createTerm(Glossary glossary, GlossaryTerm parent, String te
return createEntity(createGlossaryTerm, ADMIN_AUTH_HEADERS);
}

public void assertContains(ResultList<GlossaryTerm> list, List<String> expectedTerm) {
assertEquals(expectedTerm.size(), list.getData().size());
list.getData().forEach(term -> assertTrue(expectedTerm.contains(term.getName())));
public void assertContains(List<GlossaryTerm> expectedTerms, List<GlossaryTerm> actualTerms)
throws HttpResponseException {
assertEquals(expectedTerms.size(), actualTerms.size());
for (GlossaryTerm expected : expectedTerms) {
GlossaryTerm actual =
actualTerms.stream().filter(a -> EntityUtil.glossaryTermMatch.test(a, expected)).findAny().orElse(null);
assertNotNull(actual, "Expected glossaryTerm " + expected.getFullyQualifiedName() + " not found");
assertEquals(expected.getFullyQualifiedName(), actual.getFullyQualifiedName());
assertEquals(expected.getSynonyms(), actual.getSynonyms());
assertEquals(expected.getParent(), actual.getParent());
assertEntityReferenceList(expected.getChildren(), actual.getChildren());
assertEntityReferenceList(expected.getReviewers(), actual.getReviewers());
TestUtils.validateTags(expected.getTags(), actual.getTags());
}
}

@Override
Expand Down

0 comments on commit 1780909

Please sign in to comment.