From 89bd68cdf6905201d33b4ad7aa466d0ec62f6f9b Mon Sep 17 00:00:00 2001
From: Suresh Srinivas <srini30005@gmail.com>
Date: Fri, 20 Jan 2023 12:20:25 -0800
Subject: [PATCH] Fixes #9829 Add owner field to GlossaryTerm (#9833)

---
 .../service/jdbi3/GlossaryTermRepository.java | 34 ++++++++++++----
 .../glossary/GlossaryTermResource.java        |  3 +-
 .../glossary/GlossaryTermResourceTest.java    | 39 +++++++++++--------
 .../schema/api/data/createGlossaryTerm.json   |  4 ++
 .../json/schema/entity/data/glossaryTerm.json |  4 ++
 5 files changed, 59 insertions(+), 25 deletions(-)

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 67ff073180c..3286cf960e3 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
@@ -51,8 +51,8 @@ import org.openmetadata.service.util.FullyQualifiedName;
 
 @Slf4j
 public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
-  private static final String UPDATE_FIELDS = "tags,references,relatedTerms,reviewers,synonyms";
-  private static final String PATCH_FIELDS = "tags,references,relatedTerms,reviewers,synonyms";
+  private static final String UPDATE_FIELDS = "tags,references,relatedTerms,reviewers,owner,synonyms";
+  private static final String PATCH_FIELDS = "tags,references,relatedTerms,reviewers,owner,synonyms";
 
   public GlossaryTermRepository(CollectionDAO dao) {
     super(
@@ -71,6 +71,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
     entity.setChildren(fields.contains("children") ? getChildren(entity) : null);
     entity.setRelatedTerms(fields.contains("relatedTerms") ? getRelatedTerms(entity) : null);
     entity.setReviewers(fields.contains("reviewers") ? getReviewers(entity) : null);
+    entity.setOwner(fields.contains("owner") ? getOwner(entity) : null);
     return entity.withUsageCount(fields.contains("usageCount") ? getUsageCount(entity) : null);
   }
 
@@ -101,16 +102,29 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
   @Override
   public void prepare(GlossaryTerm entity) throws IOException {
     // Validate parent term
-    EntityReference parentTerm = Entity.getEntityReference(entity.getParent());
-    entity.setParent(parentTerm);
+    GlossaryTerm parentTerm =
+        entity.getParent() != null ? get(null, entity.getParent().getId(), getFields("owner")) : null;
+    List<EntityReference> inheritedReviewers = null;
+    EntityReference inheritedOwner = null;
+    if (parentTerm != null) {
+      entity.setParent(parentTerm.getEntityReference());
+      inheritedReviewers = parentTerm.getReviewers(); // Inherit reviewers from the parent term
+      inheritedOwner = parentTerm.getOwner(); // Inherit ownership from the parent term
+    }
+
     // Validate glossary
-    Glossary glossary = Entity.getEntity(entity.getGlossary(), "reviewers", Include.NON_DELETED);
+    Glossary glossary = Entity.getEntity(entity.getGlossary(), "owner,reviewers", Include.NON_DELETED);
     entity.setGlossary(glossary.getEntityReference());
 
+    // If parent term does not have reviewers or owner then inherit from the glossary
+    inheritedReviewers = inheritedReviewers != null ? inheritedReviewers : glossary.getReviewers();
+    inheritedOwner = inheritedOwner != null ? inheritedOwner : glossary.getOwner();
+
     validateHierarchy(entity);
 
-    // If reviewers is not set in the glossary term, then carry it from the glossary
-    entity.setReviewers(entity.getReviewers() == null ? glossary.getReviewers() : entity.getReviewers());
+    // If reviewers and owner are not set for the glossary term, then carry it from the glossary
+    entity.setReviewers(entity.getReviewers() != null ? entity.getReviewers() : inheritedReviewers);
+    entity.setOwner(entity.getOwner() != null ? entity.getOwner() : inheritedOwner);
 
     // Validate related terms
     EntityUtil.populateEntityReferences(entity.getRelatedTerms());
@@ -127,6 +141,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
     EntityReference parentTerm = entity.getParent();
     List<EntityReference> relatedTerms = entity.getRelatedTerms();
     List<EntityReference> reviewers = entity.getReviewers();
+    EntityReference owner = entity.getOwner();
 
     // Don't store owner, dashboard, href and tags as JSON. Build it on the fly based on relationships
     entity
@@ -134,6 +149,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
         .withParent(null)
         .withRelatedTerms(relatedTerms)
         .withReviewers(null)
+        .withOwner(null)
         .withHref(null)
         .withTags(null);
 
@@ -145,6 +161,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
         .withParent(parentTerm)
         .withRelatedTerms(relatedTerms)
         .withReviewers(reviewers)
+        .withOwner(owner)
         .withTags(tags);
   }
 
@@ -152,6 +169,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
   public void storeRelationships(GlossaryTerm entity) {
     addGlossaryRelationship(entity);
     addParentRelationship(entity);
+    storeOwner(entity, entity.getOwner());
     for (EntityReference relTerm : listOrEmpty(entity.getRelatedTerms())) {
       // Make this bidirectional relationship
       addRelationship(entity.getId(), relTerm.getId(), GLOSSARY_TERM, GLOSSARY_TERM, Relationship.RELATED_TO, true);
@@ -326,7 +344,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
           throw new IllegalArgumentException(
               CatalogExceptionMessage.systemEntityRenameNotAllowed(original.getName(), entityType));
         }
-        // Category name changed - update tag names starting from category and all the children tags
+        // Glossary term name changed - update the FQNs of the children terms to reflect this
         LOG.info("Glossary term name changed from {} to {}", original.getName(), updated.getName());
         daoCollection.glossaryTermDAO().updateFqn(original.getFullyQualifiedName(), updated.getFullyQualifiedName());
         daoCollection.tagUsageDAO().rename(original.getFullyQualifiedName(), updated.getFullyQualifiedName());
diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/glossary/GlossaryTermResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/glossary/GlossaryTermResource.java
index bf1a23f7dde..829205320c3 100644
--- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/glossary/GlossaryTermResource.java
+++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/glossary/GlossaryTermResource.java
@@ -82,6 +82,7 @@ public class GlossaryTermResource extends EntityResource<GlossaryTerm, GlossaryT
     Entity.withHref(uriInfo, term.getChildren());
     Entity.withHref(uriInfo, term.getRelatedTerms());
     Entity.withHref(uriInfo, term.getReviewers());
+    Entity.withHref(uriInfo, term.getOwner());
     return term;
   }
 
@@ -102,7 +103,7 @@ public class GlossaryTermResource extends EntityResource<GlossaryTerm, GlossaryT
     }
   }
 
-  static final String FIELDS = "children,relatedTerms,reviewers,tags,usageCount";
+  static final String FIELDS = "children,relatedTerms,reviewers,owner,tags,usageCount";
 
   @GET
   @Valid
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 6fe695586e0..6138dbcf364 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
@@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.openmetadata.common.utils.CommonUtil.listOf;
 import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
 import static org.openmetadata.schema.type.ColumnDataType.BIGINT;
 import static org.openmetadata.service.Entity.FIELD_TAGS;
@@ -102,7 +103,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
     // - term1
     //   - term11
     //   - term12
-    Glossary glossary1 = createGlossary("glossary1");
+    Glossary glossary1 = createGlossary("glossary1", null, null);
 
     GlossaryTerm term1 = createTerm(glossary1, null, "term1");
     GlossaryTerm term11 = createTerm(glossary1, term1, "term11");
@@ -114,7 +115,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
     // - term2
     //   - term21
     //   - term22
-    Glossary glossary2 = createGlossary("glossary2");
+    Glossary glossary2 = createGlossary("glossary2", null, null);
 
     GlossaryTerm term2 = createTerm(glossary2, null, "term2");
     GlossaryTerm term21 = createTerm(glossary2, term2, "term21");
@@ -162,18 +163,22 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
   }
 
   @Test
-  void test_inheritGlossaryReviewer(TestInfo test) throws IOException {
+  void test_inheritGlossaryReviewerAndOwner(TestInfo test) throws IOException {
     //
     // When reviewers are not set for a glossary term, carry it forward from the glossary
     //
-    Glossary glossary = createGlossary(test);
+    Glossary glossary = createGlossary(test, listOf(USER1_REF), USER2_REF);
 
-    // Create terms t1 and a term t12 under t1 in the glossary without reviewers
-    GlossaryTerm t1 = createTerm(glossary, null, "t1", null);
+    // Create terms t1 and a term t12 under t1 in the glossary without reviewers and owner
+    CreateGlossaryTerm create =
+        new CreateGlossaryTerm().withGlossary(glossary.getEntityReference()).withDescription("description");
+    GlossaryTerm t1 = createEntity(create.withName("t1"), ADMIN_AUTH_HEADERS);
     assertEquals(glossary.getReviewers(), t1.getReviewers()); // Reviewers are inherited
+    assertReference(glossary.getOwner(), t1.getOwner()); // Owner is inherited
 
-    GlossaryTerm t12 = createTerm(glossary, t1, "t12", null);
+    GlossaryTerm t12 = createEntity(create.withName("t12").withParent(t1.getEntityReference()), ADMIN_AUTH_HEADERS);
     assertEquals(glossary.getReviewers(), t12.getReviewers()); // Reviewers are inherited
+    assertReference(glossary.getOwner(), t12.getOwner()); // Owner is inherited
   }
 
   @Test
@@ -181,7 +186,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
     //
     // Create glossary terms that start with common prefix and make sure usage count is correct
     //
-    Glossary glossary = createGlossary(test);
+    Glossary glossary = createGlossary(test, null, null);
 
     // Create nested terms a -> aa -> aaa;
     GlossaryTerm a = createTerm(glossary, null, "a", null);
@@ -278,7 +283,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
 
   @Test
   void delete_recursive(TestInfo test) throws IOException {
-    Glossary g1 = createGlossary(test);
+    Glossary g1 = createGlossary(test, null, null);
     EntityReference g1Ref = g1.getEntityReference();
 
     // Create glossary term t1 in glossary g1
@@ -437,14 +442,14 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
         byName
             ? getEntityByName(term.getFullyQualifiedName(), fields, ADMIN_AUTH_HEADERS)
             : getEntity(term.getId(), fields, ADMIN_AUTH_HEADERS);
-    assertListNull(term.getChildren(), term.getRelatedTerms(), term.getReviewers(), term.getTags());
+    assertListNull(term.getChildren(), term.getRelatedTerms(), term.getReviewers(), term.getOwner(), term.getTags());
 
-    fields = "children,relatedTerms,reviewers,tags";
+    fields = "children,relatedTerms,reviewers,owner,tags";
     term =
         byName
             ? getEntityByName(term.getFullyQualifiedName(), fields, ADMIN_AUTH_HEADERS)
             : getEntity(term.getId(), fields, ADMIN_AUTH_HEADERS);
-    assertListNotNull(term.getRelatedTerms(), term.getReviewers(), term.getTags());
+    assertListNotNull(term.getRelatedTerms(), term.getReviewers(), term.getOwner(), term.getTags());
     assertListNotEmpty(term.getRelatedTerms(), term.getReviewers());
     // Checks for other owner, tags, and followers is done in the base class
     return term;
@@ -589,12 +594,14 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
     }
   }
 
-  public Glossary createGlossary(TestInfo test) throws IOException {
-    return createGlossary(glossaryResourceTest.getEntityName(test));
+  public Glossary createGlossary(TestInfo test, List<EntityReference> reviewers, EntityReference owner)
+      throws IOException {
+    return createGlossary(glossaryResourceTest.getEntityName(test), reviewers, owner);
   }
 
-  public Glossary createGlossary(String name) throws IOException {
-    CreateGlossary create = glossaryResourceTest.createRequest(name);
+  public Glossary createGlossary(String name, List<EntityReference> reviewers, EntityReference owner)
+      throws IOException {
+    CreateGlossary create = glossaryResourceTest.createRequest(name).withReviewers(reviewers).withOwner(owner);
     return glossaryResourceTest.createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
   }
 
diff --git a/openmetadata-spec/src/main/resources/json/schema/api/data/createGlossaryTerm.json b/openmetadata-spec/src/main/resources/json/schema/api/data/createGlossaryTerm.json
index 52e106ef886..f74c6f34994 100644
--- a/openmetadata-spec/src/main/resources/json/schema/api/data/createGlossaryTerm.json
+++ b/openmetadata-spec/src/main/resources/json/schema/api/data/createGlossaryTerm.json
@@ -50,6 +50,10 @@
       "description": "User names of the reviewers for this glossary.",
       "$ref": "../../type/entityReference.json#/definitions/entityReferenceList"
     },
+    "owner": {
+      "description": "Owner of this glossary term.",
+      "$ref": "../../type/entityReference.json"
+    },
     "tags": {
       "description": "Tags for this glossary term.",
       "type": "array",
diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/data/glossaryTerm.json b/openmetadata-spec/src/main/resources/json/schema/entity/data/glossaryTerm.json
index d2d5d9c344e..647a583309e 100644
--- a/openmetadata-spec/src/main/resources/json/schema/entity/data/glossaryTerm.json
+++ b/openmetadata-spec/src/main/resources/json/schema/entity/data/glossaryTerm.json
@@ -103,6 +103,10 @@
       "description": "User names of the reviewers for this glossary.",
       "$ref": "../../type/entityReference.json#/definitions/entityReferenceList"
     },
+    "owner": {
+      "description": "Owner of this glossary term.",
+      "$ref": "../../type/entityReference.json"
+    },
     "usageCount": {
       "description": "Count of how many times this and it's children glossary terms are used as labels.",
       "type": "integer"