Merge branch '1.4.2' into limits

ingestion/tests/integration/ometa/test_ometa_glossary.py

@@ -439,7 +439,7 @@ def test_patch_reviewer(self):
         )
 
         self.assertIsNotNone(res_glossary_term)
-        self.assertEqual(1, len(res_glossary_term.reviewers.__root__))
+        self.assertEqual(2, len(res_glossary_term.reviewers.__root__))
         self.assertEqual(self.user_1.id, res_glossary_term.reviewers.__root__[0].id)
         dest_glossary_term_1 = deepcopy(res_glossary_term)
         dest_glossary_term_1.reviewers.__root__.pop(0)
@@ -449,7 +449,7 @@ def test_patch_reviewer(self):
             destination=dest_glossary_term_1,
         )
         self.assertIsNotNone(res_glossary_term)
-        self.assertEqual(0, len(res_glossary_term.reviewers.__root__))
+        self.assertEqual(2, len(res_glossary_term.reviewers.__root__))
 
     def test_patch_glossary_term_synonyms(self):
         """

openmetadata-service/src/main/java/org/openmetadata/service/OpenMetadataApplication.java

@@ -172,12 +172,12 @@ public void run(OpenMetadataApplicationConfig catalogConfig, Environment environ
     // as first step register all the repositories
     Entity.initializeRepositories(catalogConfig, jdbi);
 
-    // Init Settings Cache after repositories
-    SettingsCache.initialize(catalogConfig);
-
     // Configure the Fernet instance
     Fernet.getInstance().setFernetKey(catalogConfig);
 
+    // Init Settings Cache after repositories
+    SettingsCache.initialize(catalogConfig);
+
     initializeWebsockets(catalogConfig, environment);
 
     // init Secret Manager

openmetadata-service/src/main/java/org/openmetadata/service/events/EventFilter.java

@@ -13,6 +13,8 @@
 
 package org.openmetadata.service.events;
 
+import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -44,22 +46,26 @@ public EventFilter(OpenMetadataApplicationConfig config) {
     registerEventHandlers(config);
   }
 
+  @SuppressWarnings("unchecked")
   private void registerEventHandlers(OpenMetadataApplicationConfig config) {
-    try {
+    if (!nullOrEmpty(config.getEventHandlerConfiguration())) {
       Set<String> eventHandlerClassNames =
           new HashSet<>(config.getEventHandlerConfiguration().getEventHandlerClassNames());
       for (String eventHandlerClassName : eventHandlerClassNames) {
-        @SuppressWarnings("unchecked")
-        EventHandler eventHandler =
-            ((Class<EventHandler>) Class.forName(eventHandlerClassName))
-                .getConstructor()
-                .newInstance();
-        eventHandler.init(config);
-        eventHandlers.add(eventHandler);
-        LOG.info("Added event handler {}", eventHandlerClassName);
+        try {
+          EventHandler eventHandler =
+              ((Class<EventHandler>) Class.forName(eventHandlerClassName))
+                  .getConstructor()
+                  .newInstance();
+          eventHandler.init(config);
+          eventHandlers.add(eventHandler);
+          LOG.info("Added event handler {}", eventHandlerClassName);
+        } catch (Exception e) {
+          LOG.info("Exception ", e);
+        }
       }
-    } catch (Exception e) {
-      LOG.info("Exception ", e);
+    } else {
+      LOG.info("Event handler configuration is empty");
     }
   }
 

openmetadata-service/src/main/java/org/openmetadata/service/exception/CatalogExceptionMessage.java

@@ -182,6 +182,12 @@ public static String notAdmin(String name) {
     return String.format("Principal: CatalogPrincipal{name='%s'} is not admin", name);
   }
 
+  public static String operationNotAllowed(String name, MetadataOperation operation) {
+    return String.format(
+        "Principal: CatalogPrincipal{name='%s'} operations [%s] not allowed",
+        name, operation.value());
+  }
+
   public static String notReviewer(String name) {
     return String.format("User '%s' is not a reviewer", name);
   }
@@ -313,6 +319,10 @@ public static String invalidFieldForTask(String fieldName, TaskType type) {
     return String.format("The field name %s is not supported for %s task.", fieldName, type);
   }
 
+  public static String invalidReviewerType(String type) {
+    return String.format("Reviewers can only be a Team or User. Given Reviewer Type : %s", type);
+  }
+
   public static String invalidEnumValue(Class<? extends Enum<?>> enumClass) {
     String className = enumClass.getSimpleName();
     String classNameWithLowercaseFirstLetter =

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java

@@ -908,6 +908,13 @@ void deleteTo(
         @Bind("relation") int relation,
         @Bind("fromEntity") String fromEntity);
 
+    @SqlUpdate(
+        "DELETE from entity_relationship WHERE toId = :toId AND toEntity = :toEntity AND relation = :relation")
+    void deleteTo(
+        @BindUUID("toId") UUID toId,
+        @Bind("toEntity") String toEntity,
+        @Bind("relation") int relation);
+
     @SqlUpdate(
         "DELETE from entity_relationship WHERE (toId = :id AND toEntity = :entity) OR "
             + "(fromId = :id AND fromEntity = :entity)")

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java

@@ -44,6 +44,7 @@
 import static org.openmetadata.service.Entity.FIELD_STYLE;
 import static org.openmetadata.service.Entity.FIELD_TAGS;
 import static org.openmetadata.service.Entity.FIELD_VOTES;
+import static org.openmetadata.service.Entity.TEAM;
 import static org.openmetadata.service.Entity.USER;
 import static org.openmetadata.service.Entity.getEntityByName;
 import static org.openmetadata.service.Entity.getEntityFields;
@@ -471,6 +472,7 @@ public final void initializeEntity(T entity) {
   public final T copy(T entity, CreateEntity request, String updatedBy) {
     EntityReference owner = validateOwner(request.getOwner());
     EntityReference domain = validateDomain(request.getDomain());
+    validateReviewers(request.getReviewers());
     entity.setId(UUID.randomUUID());
     entity.setName(request.getName());
     entity.setDisplayName(request.getDisplayName());
@@ -483,6 +485,7 @@ public final T copy(T entity, CreateEntity request, String updatedBy) {
     entity.setExtension(request.getExtension());
     entity.setUpdatedBy(updatedBy);
     entity.setUpdatedAt(System.currentTimeMillis());
+    entity.setReviewers(request.getReviewers());
     return entity;
   }
 
@@ -754,6 +757,7 @@ public final void storeRelationshipsInternal(T entity) {
     applyTags(entity);
     storeDomain(entity, entity.getDomain());
     storeDataProducts(entity, entity.getDataProducts());
+    storeReviewers(entity, entity.getReviewers());
     storeRelationships(entity);
   }
 
@@ -854,7 +858,7 @@ public final PatchResponse<T> patch(UriInfo uriInfo, UUID id, String user, JsonP
     EventType change = ENTITY_NO_CHANGE;
     if (entityUpdater.fieldsChanged()) {
       change = EventType.ENTITY_UPDATED;
-      setInheritedFields(original, patchFields); // Restore inherited fields after a change
+      setInheritedFields(updated, patchFields); // Restore inherited fields after a change
     }
     return new PatchResponse<>(Status.OK, withHref(uriInfo, updated), change);
   }
@@ -883,7 +887,7 @@ public final PatchResponse<T> patch(UriInfo uriInfo, String fqn, String user, Js
     EventType change = ENTITY_NO_CHANGE;
     if (entityUpdater.fieldsChanged()) {
       change = EventType.ENTITY_UPDATED;
-      setInheritedFields(original, patchFields); // Restore inherited fields after a change
+      setInheritedFields(updated, patchFields); // Restore inherited fields after a change
     }
     return new PatchResponse<>(Status.OK, withHref(uriInfo, updated), change);
   }
@@ -1672,9 +1676,13 @@ public final void deleteRelationship(
 
   public final void deleteTo(
       UUID toId, String toEntityType, Relationship relationship, String fromEntityType) {
-    daoCollection
-        .relationshipDAO()
-        .deleteTo(toId, toEntityType, relationship.ordinal(), fromEntityType);
+    if (fromEntityType == null) {
+      daoCollection.relationshipDAO().deleteTo(toId, toEntityType, relationship.ordinal());
+    } else {
+      daoCollection
+          .relationshipDAO()
+          .deleteTo(toId, toEntityType, relationship.ordinal(), fromEntityType);
+    }
   }
 
   public final void deleteFrom(
@@ -1699,6 +1707,43 @@ public final void validateUsers(List<EntityReference> entityReferences) {
     }
   }
 
+  private boolean validateIfAllRefsAreEntityType(List<EntityReference> list, String entityType) {
+    return list.stream().allMatch(obj -> obj.getType().equals(entityType));
+  }
+
+  public final void validateReviewers(List<EntityReference> entityReferences) {
+    if (!nullOrEmpty(entityReferences)) {
+      boolean areAllTeam = validateIfAllRefsAreEntityType(entityReferences, TEAM);
+      boolean areAllUsers = validateIfAllRefsAreEntityType(entityReferences, USER);
+      if (areAllTeam) {
+        // If all are team then only one team is allowed
+        if (entityReferences.size() > 1) {
+          throw new IllegalArgumentException("Only one team can be assigned as reviewer.");
+        } else {
+          EntityReference ref =
+              entityReferences.get(0).getId() != null
+                  ? Entity.getEntityReferenceById(TEAM, entityReferences.get(0).getId(), ALL)
+                  : Entity.getEntityReferenceByName(
+                      TEAM, entityReferences.get(0).getFullyQualifiedName(), ALL);
+          EntityUtil.copy(ref, entityReferences.get(0));
+        }
+      } else if (areAllUsers) {
+        for (EntityReference entityReference : entityReferences) {
+          EntityReference ref =
+              entityReference.getId() != null
+                  ? Entity.getEntityReferenceById(USER, entityReference.getId(), ALL)
+                  : Entity.getEntityReferenceByName(
+                      USER, entityReference.getFullyQualifiedName(), ALL);
+          EntityUtil.copy(ref, entityReference);
+        }
+      } else {
+        throw new IllegalArgumentException(
+            "Invalid Reviewer Type. Only one team or multiple users can be assigned as reviewer.");
+      }
+      entityReferences.sort(EntityUtil.compareEntityReference);
+    }
+  }
+
   public final void validateRoles(List<EntityReference> roles) {
     if (roles != null) {
       for (EntityReference entityReference : roles) {
@@ -1751,7 +1796,7 @@ protected List<EntityReference> getChildren(T entity) {
 
   protected List<EntityReference> getReviewers(T entity) {
     return supportsReviewers
-        ? findFrom(entity.getId(), entityType, Relationship.REVIEWS, Entity.USER)
+        ? findFrom(entity.getId(), entityType, Relationship.REVIEWS, null)
         : null;
   }
 
@@ -1785,9 +1830,24 @@ public final void inheritExperts(T entity, Fields fields, EntityInterface parent
   }
 
   public final void inheritReviewers(T entity, Fields fields, EntityInterface parent) {
-    if (fields.contains(FIELD_REVIEWERS) && nullOrEmpty(entity.getReviewers()) && parent != null) {
-      entity.setReviewers(parent.getReviewers());
-      listOrEmpty(entity.getReviewers()).forEach(reviewer -> reviewer.withInherited(true));
+    if (fields.contains(FIELD_REVIEWERS) && parent != null) {
+      List<EntityReference> combinedReviewers = new ArrayList<>(listOrEmpty(entity.getReviewers()));
+      // Fetch Unique Reviewers from parent as inherited
+      List<EntityReference> uniqueEntityReviewers =
+          listOrEmpty(parent.getReviewers()).stream()
+              .filter(
+                  parentReviewer ->
+                      combinedReviewers.stream()
+                          .noneMatch(
+                              entityReviewer ->
+                                  parentReviewer.getId().equals(entityReviewer.getId())
+                                      && parentReviewer.getType().equals(entityReviewer.getType())))
+              .toList();
+      uniqueEntityReviewers.forEach(reviewer -> reviewer.withInherited(true));
+
+      combinedReviewers.addAll(uniqueEntityReviewers);
+      combinedReviewers.sort(EntityUtil.compareEntityReference);
+      entity.setReviewers(combinedReviewers);
     }
   }
 
@@ -1827,6 +1887,17 @@ protected void storeDomain(T entity, EntityReference domain) {
     }
   }
 
+  @Transaction
+  protected void storeReviewers(T entity, List<EntityReference> reviewers) {
+    if (supportsReviewers) {
+      // Add relationship user/team --- reviews ---> entity
+      for (EntityReference reviewer : listOrEmpty(reviewers)) {
+        addRelationship(
+            reviewer.getId(), entity.getId(), reviewer.getType(), entityType, Relationship.REVIEWS);
+      }
+    }
+  }
+
   @Transaction
   protected void storeDataProducts(T entity, List<EntityReference> dataProducts) {
     if (supportsDataProducts && !nullOrEmpty(dataProducts)) {
@@ -2467,10 +2538,12 @@ protected void updateReviewers() {
       }
       List<EntityReference> origReviewers = getEntityReferences(original.getReviewers());
       List<EntityReference> updatedReviewers = getEntityReferences(updated.getReviewers());
-      validateUsers(updatedReviewers);
+      validateReviewers(updatedReviewers);
+      // Either all users or team which is one team at a time, assuming all ref to have same type,
+      // validateReviewer checks it
       updateFromRelationships(
           "reviewers",
-          Entity.USER,
+          null,
           origReviewers,
           updatedReviewers,
           Relationship.REVIEWS,
@@ -2746,7 +2819,7 @@ public final void updateFromRelationships(
 
       // Add relationships from updated
       for (EntityReference ref : updatedFromRefs) {
-        addRelationship(ref.getId(), toId, fromEntityType, toEntityType, relationshipType);
+        addRelationship(ref.getId(), toId, ref.getType(), toEntityType, relationshipType);
       }
       updatedFromRefs.sort(EntityUtil.compareEntityReference);
       originFromRefs.sort(EntityUtil.compareEntityReference);

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java

@@ -16,7 +16,6 @@
 
 package org.openmetadata.service.jdbi3;
 
-import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
 import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
 import static org.openmetadata.csv.CsvUtil.FIELD_SEPARATOR;
 import static org.openmetadata.csv.CsvUtil.addEntityReference;
@@ -99,9 +98,7 @@ public void clearFields(Glossary glossary, Fields fields) {
   }
 
   @Override
-  public void prepare(Glossary glossary, boolean update) {
-    validateUsers(glossary.getReviewers());
-  }
+  public void prepare(Glossary glossary, boolean update) {}
 
   @Override
   public void storeEntity(Glossary glossary, boolean update) {
@@ -114,10 +111,7 @@ public void storeEntity(Glossary glossary, boolean update) {
 
   @Override
   public void storeRelationships(Glossary glossary) {
-    for (EntityReference reviewer : listOrEmpty(glossary.getReviewers())) {
-      addRelationship(
-          reviewer.getId(), glossary.getId(), Entity.USER, Entity.GLOSSARY, Relationship.REVIEWS);
-    }
+    // Nothing to do
   }
 
   private Integer getUsageCount(Glossary glossary) {

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java

@@ -21,6 +21,7 @@
 import static org.openmetadata.schema.type.Include.ALL;
 import static org.openmetadata.service.Entity.GLOSSARY;
 import static org.openmetadata.service.Entity.GLOSSARY_TERM;
+import static org.openmetadata.service.Entity.TEAM;
 import static org.openmetadata.service.exception.CatalogExceptionMessage.invalidGlossaryTermMove;
 import static org.openmetadata.service.exception.CatalogExceptionMessage.notReviewer;
 import static org.openmetadata.service.resources.tags.TagLabelUtil.checkMutuallyExclusive;
@@ -63,6 +64,7 @@
 import org.openmetadata.schema.entity.data.GlossaryTerm;
 import org.openmetadata.schema.entity.data.GlossaryTerm.Status;
 import org.openmetadata.schema.entity.feed.Thread;
+import org.openmetadata.schema.entity.teams.Team;
 import org.openmetadata.schema.type.ApiStatus;
 import org.openmetadata.schema.type.EntityReference;
 import org.openmetadata.schema.type.Include;
@@ -181,9 +183,6 @@ public void prepare(GlossaryTerm entity, boolean update) {
     // Validate related terms
     EntityUtil.populateEntityReferences(entity.getRelatedTerms());
 
-    // Validate reviewers
-    EntityUtil.populateEntityReferences(entity.getReviewers());
-
     if (!update || entity.getStatus() == null) {
       // If parentTerm or glossary has reviewers set, the glossary term can only be created in
       // `Draft` mode
@@ -224,10 +223,6 @@ public void storeRelationships(GlossaryTerm entity) {
           Relationship.RELATED_TO,
           true);
     }
-    for (EntityReference reviewer : listOrEmpty(entity.getReviewers())) {
-      addRelationship(
-          reviewer.getId(), entity.getId(), Entity.USER, GLOSSARY_TERM, Relationship.REVIEWS);
-    }
   }
 
   @Override
@@ -698,8 +693,20 @@ private void checkUpdatedByReviewer(GlossaryTerm term, String updatedBy) {
       boolean isReviewer =
           reviewers.stream()
               .anyMatch(
-                  e ->
-                      e.getName().equals(updatedBy) || e.getFullyQualifiedName().equals(updatedBy));
+                  e -> {
+                    if (e.getType().equals(TEAM)) {
+                      Team team =
+                          Entity.getEntityByName(TEAM, e.getName(), "users", Include.NON_DELETED);
+                      return team.getUsers().stream()
+                          .anyMatch(
+                              u ->
+                                  u.getName().equals(updatedBy)
+                                      || u.getFullyQualifiedName().equals(updatedBy));
+                    } else {
+                      return e.getName().equals(updatedBy)
+                          || e.getFullyQualifiedName().equals(updatedBy);
+                    }
+                  });
       if (!isReviewer) {
         throw new AuthorizationException(notReviewer(updatedBy));
       }