sonic-net
diff --git a/‎generic_config_updater/patch_sorter.py‎
Lines changed: 74 additions & 40 deletions b/‎generic_config_updater/patch_sorter.py‎
Lines changed: 74 additions & 40 deletions
@@ -4,7 +4,7 @@
 import sonic_yang
 from collections import deque, OrderedDict
 from enum import Enum
-from typing import IO, Optional, Tuple
+from typing import Any, IO, List, Optional, Tuple
 from .gu_common import OperationWrapper, OperationType, GenericConfigUpdaterError, \
                        JsonChange, PathAddressing, genericUpdaterLogging
 
@@ -1580,55 +1580,89 @@ def generate(self, diff):
         target_config = diff.target_config # Final config after applying whole patch
         reload_config = True
 
-        processed_tables = set()
         for path in self.create_only_filter.get_paths(current_config):
             tokens = self.path_addressing.get_path_tokens(path)
-            table_to_check, create_only_field = tokens[0], tokens[-1]
-
-            if table_to_check in processed_tables:
-                continue
-            else:
-                processed_tables.add(table_to_check)
-
-            if table_to_check not in current_config:
-                continue
+            current_field = self.__fetch_path(current_config, tokens)
+            target_field = self.__fetch_path(target_config, tokens)
 
-            current_members = current_config[table_to_check]
-            if not current_members:
+            # If field is deleted or created we don't care, we only care when it was
+            # already set and is changing value.
+            if current_field is None or target_field is None or current_field == target_field:
                 continue
 
-            if table_to_check not in target_config:
-                continue
+            # Create only filters may reference an exact leaf, but it's really the parent that is the
+            # object we're after.
+            tokens.pop()
 
-            target_members = target_config[table_to_check]
-            if not target_members:
-                continue
+            # First see if there are any dependents for the exact path
+            for move in self.__remove_dependents(diff, tokens, reload_config=reload_config,
+                                                 remove_parent=False):
+                yield move
 
-            for member_name in current_members:
-                if member_name not in target_members:
-                    continue
+            # No need to reload config after first call
+            reload_config = False
 
-                current_field = self._get_create_only_field(
-                    current_config, table_to_check, member_name, create_only_field)
-                target_field = self._get_create_only_field(
-                    target_config, table_to_check, member_name, create_only_field)
+            yield self.__remove_nonempty(diff, tokens)
 
-                if current_field == target_field:
-                    continue
+            # If that didn't work, likely the parents of the dependent path needs to be removed.
+            for move in self.__remove_dependents(diff, tokens, reload_config=reload_config,
+                                                 remove_parent=True):
+                yield move
 
-                member_path = f"/{table_to_check}/{member_name}"
+            # Remove self again after removing the parents of dependents
+            # NOTE: When we use the DFS sorter this is irrelevant.  Right now we only use DFS so we don't need it
+            #       as it will be called again after it removed any parent dependents.  Commenting out for now.
+            #
+            # yield self.__remove_nonempty(diff, tokens)
 
-                for ref_path in self.path_addressing.find_ref_paths(member_path, current_config,
-                                                                    reload_config=reload_config):
-                    yield JsonMoveGroup(self.__class__.__name__, JsonMove(diff, OperationType.REMOVE,
-                                        self.path_addressing.get_path_tokens(ref_path)))
+    def __fetch_path(self, config, tokens: List[str]) -> Any:
+        for token in tokens:
+            config = config.get(token)
+            if config is None:
+                return None
+        return config
+
+    def __get_path_count(self, config, tokens: List[str]) -> int:
+        config = self.__fetch_path(config, tokens)
+        if config is None:
+            return 0
+        return len(config)
+
+    def __remove_nonempty(self, diff: Diff, tokens: List[str]):
+        remove_tokens = list(tokens)
+        # Only trim while there is at least one table token and one deeper level.
+        # This prevents generating an empty path ("") and avoids an infinite loop
+        # when the top-level config has a single table.
+        while len(remove_tokens) > 1 and \
+                self.__get_path_count(diff.current_config, remove_tokens[:-1]) == 1:
+            remove_tokens = remove_tokens[:-1]
+        return JsonMoveGroup(self.__class__.__name__, JsonMove(diff, OperationType.REMOVE, remove_tokens))
+
+    def __remove_dependents(
+        self,
+        diff: Diff,
+        tokens: List[str],
+        reload_config: bool,
+        remove_parent: bool,
+        recursion_depth: int = 0,
+    ):
+        if recursion_depth >= 10:
+            return
 
-                # No need to reload config after first call
-                reload_config = False
+        config = diff.current_config
+        path = self.path_addressing.create_path(tokens)
+        ref_paths = self.path_addressing.find_ref_paths(path, config, reload_config)
+        for ref in ref_paths:
+            ref_tokens = self.path_addressing.get_path_tokens(ref)
+            if remove_parent:
+                ref_tokens.pop()
+
+            # Recurse since there could be a dependency chain
+            for move in self.__remove_dependents(diff, ref_tokens, reload_config=False,
+                                                 remove_parent=remove_parent, recursion_depth=recursion_depth+1):
+                yield move
 
-    def _get_create_only_field(self, config, table_to_check,
-                               member_name, create_only_field):
-        return config[table_to_check][member_name].get(create_only_field, None)
+            yield self.__remove_nonempty(diff, ref_tokens)
 
 
 class LowLevelMoveGenerator:
@@ -2222,10 +2256,10 @@ def __init__(self, operation_wrapper, config_wrapper, path_addressing):
         self.path_addressing = path_addressing
 
     def create(self, algorithm=Algorithm.DFS, path_trace: bool = False):
-        move_generators = [RemoveCreateOnlyDependencyMoveGenerator(self.path_addressing),
-                           LowLevelMoveGenerator(self.path_addressing)]
+        move_generators = [LowLevelMoveGenerator(self.path_addressing)]
         # TODO: Enable TableLevelMoveGenerator once it is confirmed whole table can be updated at the same time
-        move_non_extendable_generators = [BulkKeyLevelMoveGenerator(self.path_addressing),
+        move_non_extendable_generators = [RemoveCreateOnlyDependencyMoveGenerator(self.path_addressing),
+                                          BulkKeyLevelMoveGenerator(self.path_addressing),
                                           KeyLevelMoveGenerator(self.path_addressing),
                                           BulkKeyGroupLowLevelMoveGenerator(self.path_addressing),
                                           BulkLowLevelMoveGenerator(self.path_addressing)]