Fix #18332: Do not consolidate changes for Parent Entities (#18333)

* Fix #18332: Do not consolidate changes for Parent Entities * Fix #18332: Do not consolidate changes for Parent Entities * added no data placeholder when no tags there * fix failing playwright test due to consolidation changes and added the missing services * Fix tests * Fix parent entity change notifications * Fix tests * Fix parent entity change notifications * Fix tests * Fix tests --------- Co-authored-by: Ashish Gupta <ashish@getcollate.io>
open-metadata · Oct 31, 2024 · 680ace9 · 680ace9
1 parent 2539ffa
commit 680ace9
Show file tree

Hide file tree

Showing 41 changed files with 330 additions and 162 deletions.
diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java b/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java
@@ -243,6 +243,7 @@ public final class Entity {
   public static final String DOCUMENT = "document";
   // ServiceType - Service Entity name map
   static final Map<ServiceType, String> SERVICE_TYPE_ENTITY_MAP = new EnumMap<>(ServiceType.class);
+  public static final List<String> PARENT_ENTITY_TYPES = new ArrayList<>();
 
   static {
     SERVICE_TYPE_ENTITY_MAP.put(ServiceType.DATABASE, DATABASE_SERVICE);
@@ -254,6 +255,25 @@ public final class Entity {
     SERVICE_TYPE_ENTITY_MAP.put(ServiceType.STORAGE, STORAGE_SERVICE);
     SERVICE_TYPE_ENTITY_MAP.put(ServiceType.SEARCH, SEARCH_SERVICE);
     SERVICE_TYPE_ENTITY_MAP.put(ServiceType.API, API_SERVICE);
+    PARENT_ENTITY_TYPES.addAll(
+        listOf(
+            DATABASE_SERVICE,
+            DASHBOARD_SERVICE,
+            MESSAGING_SERVICE,
+            MLMODEL_SERVICE,
+            PIPELINE_SERVICE,
+            API_SERVICE,
+            API_COLLCECTION,
+            STORAGE_SERVICE,
+            METADATA_SERVICE,
+            SEARCH_SERVICE,
+            DATABASE,
+            DATABASE_SCHEMA,
+            CLASSIFICATION,
+            GLOSSARY,
+            DOMAIN,
+            TEST_SUITE,
+            TEAM));
   }
 
   private Entity() {}

diff --git a/...etadata-service/src/main/java/org/openmetadata/service/jdbi3/APICollectionRepository.java b/...etadata-service/src/main/java/org/openmetadata/service/jdbi3/APICollectionRepository.java
@@ -39,6 +39,7 @@ public APICollectionRepository() {
         "",
         "");
     supportsSearch = true;
+    parent = true;
   }
 
   @Override

diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/APIServiceRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/APIServiceRepository.java
@@ -16,5 +16,6 @@ public APIServiceRepository() {
         "",
         ServiceType.API);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/ClassificationRepository.java b/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/ClassificationRepository.java
@@ -54,6 +54,7 @@ public ClassificationRepository() {
     quoteFqn = true;
     supportsSearch = true;
     renameAllowed = true;
+    parent = true;
   }
 
   @Override

diff --git a/...data-service/src/main/java/org/openmetadata/service/jdbi3/DashboardServiceRepository.java b/...data-service/src/main/java/org/openmetadata/service/jdbi3/DashboardServiceRepository.java
@@ -33,5 +33,6 @@ public DashboardServiceRepository() {
         "",
         ServiceType.DASHBOARD);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DatabaseRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DatabaseRepository.java
@@ -69,6 +69,7 @@ public DatabaseRepository() {
         "",
         "");
     supportsSearch = true;
+    parent = true;
     fieldFetchers.put("name", this::fetchAndSetService);
   }
 

diff --git a/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/DatabaseSchemaRepository.java b/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/DatabaseSchemaRepository.java
@@ -74,6 +74,7 @@ public DatabaseSchemaRepository() {
         "",
         "");
     supportsSearch = true;
+    parent = true;
   }
 
   @Override

diff --git a/...adata-service/src/main/java/org/openmetadata/service/jdbi3/DatabaseServiceRepository.java b/...adata-service/src/main/java/org/openmetadata/service/jdbi3/DatabaseServiceRepository.java
@@ -60,6 +60,7 @@ public DatabaseServiceRepository() {
         "",
         ServiceType.DATABASE);
     supportsSearch = true;
+    parent = true;
   }
 
   @Override

diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DomainRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DomainRepository.java
@@ -46,6 +46,7 @@ public DomainRepository() {
         UPDATE_FIELDS,
         UPDATE_FIELDS);
     supportsSearch = true;
+    parent = true;
   }
 
   @Override

diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java
@@ -260,6 +260,7 @@ public record EntityHistoryWithOffset(EntityHistory entityHistory, int nextOffse
   @Getter protected final Fields putFields;
 
   protected boolean supportsSearch = false;
+  @Getter protected boolean parent = false;
   protected final Map<String, BiConsumer<List<T>, Fields>> fieldFetchers = new HashMap<>();
 
   protected EntityRepository(
@@ -3276,7 +3277,9 @@ public static void setSessionTimeout(long timeout) {
 
     private boolean consolidateChanges(T original, T updated, Operation operation) {
       // If user is the same and the new update is with in the user session timeout
-      return original.getVersion() > 0.1 // First update on an entity that
+      return !parent // Parent entity shouldn't consolidate changes, as we need ChangeDescription to
+          // propagate to children
+          && original.getVersion() > 0.1 // First update on an entity that
           && operation == Operation.PATCH
           && !Boolean.TRUE.equals(original.getDeleted()) // Entity is not soft deleted
           && !operation.isDelete() // Operation must be an update
@@ -3285,6 +3288,7 @@ private boolean consolidateChanges(T original, T updated, Operation operation) {
               .equals(updated.getUpdatedBy()) // Must be updated by the same user
           && updated.getUpdatedAt() - original.getUpdatedAt()
               <= sessionTimeoutMillis; // With in session timeout
+      // changes to children
     }
 
     private T getPreviousVersion(T original) {

diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java
@@ -89,6 +89,7 @@ public GlossaryRepository() {
     quoteFqn = true;
     supportsSearch = true;
     renameAllowed = true;
+    parent = true;
   }
 
   @Override

diff --git a/...data-service/src/main/java/org/openmetadata/service/jdbi3/MessagingServiceRepository.java b/...data-service/src/main/java/org/openmetadata/service/jdbi3/MessagingServiceRepository.java
@@ -34,5 +34,6 @@ public MessagingServiceRepository() {
         UPDATE_FIELDS,
         ServiceType.MESSAGING);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/...adata-service/src/main/java/org/openmetadata/service/jdbi3/MetadataServiceRepository.java b/...adata-service/src/main/java/org/openmetadata/service/jdbi3/MetadataServiceRepository.java
@@ -19,5 +19,6 @@ public MetadataServiceRepository() {
         UPDATE_FIELDS,
         ServiceType.METADATA);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/MlModelServiceRepository.java b/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/MlModelServiceRepository.java
@@ -34,5 +34,6 @@ public MlModelServiceRepository() {
         UPDATE_FIELDS,
         ServiceType.ML_MODEL);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/...adata-service/src/main/java/org/openmetadata/service/jdbi3/PipelineServiceRepository.java b/...adata-service/src/main/java/org/openmetadata/service/jdbi3/PipelineServiceRepository.java
@@ -33,5 +33,6 @@ public PipelineServiceRepository() {
         "",
         ServiceType.PIPELINE);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/...etadata-service/src/main/java/org/openmetadata/service/jdbi3/SearchServiceRepository.java b/...etadata-service/src/main/java/org/openmetadata/service/jdbi3/SearchServiceRepository.java
@@ -17,5 +17,6 @@ public SearchServiceRepository() {
         "",
         ServiceType.SEARCH);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/StorageServiceRepository.java b/...tadata-service/src/main/java/org/openmetadata/service/jdbi3/StorageServiceRepository.java
@@ -17,5 +17,6 @@ public StorageServiceRepository() {
         "",
         ServiceType.STORAGE);
     supportsSearch = true;
+    parent = true;
   }
 }
diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TeamRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TeamRepository.java
@@ -102,6 +102,7 @@ public TeamRepository() {
         TEAM_UPDATE_FIELDS);
     this.quoteFqn = true;
     supportsSearch = true;
+    parent = true;
   }
 
   @Override

diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java
@@ -104,6 +104,7 @@ public TestSuiteRepository() {
         UPDATE_FIELDS);
     quoteFqn = false;
     supportsSearch = true;
+    parent = true;
   }
 
   @Override

diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java
@@ -449,7 +449,7 @@ private Pair<String, Map<String, Object>> getInheritedFieldChanges(
     Map<String, Object> fieldData = new HashMap<>();
 
     if (changeDescription != null) {
-      for (FieldChange field : changeDescription.getFieldsAdded()) {
+      for (FieldChange field : changeDescription.getFieldsDeleted()) {
         if (inheritableFields.contains(field.getName())) {
           try {
             if (field.getName().equals(FIELD_OWNERS)) {
@@ -458,24 +458,21 @@ private Pair<String, Map<String, Object>> getInheritedFieldChanges(
               for (EntityReference inheritedOwner : inheritedOwners) {
                 inheritedOwner.setInherited(true);
               }
-              fieldData.put("updatedOwners", inheritedOwners);
-              scriptTxt.append(ADD_OWNERS_SCRIPT);
+              fieldData.put("deletedOwners", inheritedOwners);
+              scriptTxt.append(REMOVE_OWNERS_SCRIPT);
             } else {
               EntityReference entityReference =
-                  JsonUtils.readValue(field.getNewValue().toString(), EntityReference.class);
+                  JsonUtils.readValue(field.getOldValue().toString(), EntityReference.class);
               scriptTxt.append(
                   String.format(
-                      PROPAGATE_ENTITY_REFERENCE_FIELD_SCRIPT,
-                      field.getName(),
-                      field.getName(),
+                      REMOVE_PROPAGATED_ENTITY_REFERENCE_FIELD_SCRIPT,
                       field.getName(),
                       field.getName(),
                       field.getName()));
-              fieldData.put(field.getName(), entityReference);
+              fieldData.put(field.getName(), JsonUtils.getMap(entityReference));
             }
           } catch (UnhandledServerException e) {
-            scriptTxt.append(
-                String.format(PROPAGATE_FIELD_SCRIPT, field.getName(), field.getNewValue()));
+            scriptTxt.append(String.format(REMOVE_PROPAGATED_FIELD_SCRIPT, field.getName()));
           }
         }
       }
@@ -507,7 +504,7 @@ private Pair<String, Map<String, Object>> getInheritedFieldChanges(
           }
         }
       }
-      for (FieldChange field : changeDescription.getFieldsDeleted()) {
+      for (FieldChange field : changeDescription.getFieldsAdded()) {
         if (inheritableFields.contains(field.getName())) {
           try {
             if (field.getName().equals(FIELD_OWNERS)) {
@@ -516,21 +513,24 @@ private Pair<String, Map<String, Object>> getInheritedFieldChanges(
               for (EntityReference inheritedOwner : inheritedOwners) {
                 inheritedOwner.setInherited(true);
               }
-              fieldData.put("deletedOwners", inheritedOwners);
-              scriptTxt.append(REMOVE_OWNERS_SCRIPT);
+              fieldData.put("updatedOwners", inheritedOwners);
+              scriptTxt.append(ADD_OWNERS_SCRIPT);
             } else {
               EntityReference entityReference =
-                  JsonUtils.readValue(field.getOldValue().toString(), EntityReference.class);
+                  JsonUtils.readValue(field.getNewValue().toString(), EntityReference.class);
               scriptTxt.append(
                   String.format(
-                      REMOVE_PROPAGATED_ENTITY_REFERENCE_FIELD_SCRIPT,
+                      PROPAGATE_ENTITY_REFERENCE_FIELD_SCRIPT,
+                      field.getName(),
+                      field.getName(),
                       field.getName(),
                       field.getName(),
                       field.getName()));
-              fieldData.put(field.getName(), JsonUtils.getMap(entityReference));
+              fieldData.put(field.getName(), entityReference);
             }
           } catch (UnhandledServerException e) {
-            scriptTxt.append(String.format(REMOVE_PROPAGATED_FIELD_SCRIPT, field.getName()));
+            scriptTxt.append(
+                String.format(PROPAGATE_FIELD_SCRIPT, field.getName(), field.getNewValue()));
           }
         }
       }