[GCU] Copying config_db before callding sonic_yang.loadData (sonic-ne…

…t#1983)

#### What I did
Adding unit-test for cases where `PatchSorter.sort` is called to update config that has tables without YANG. This would sometimes throw the error
```
KeyError: 'NameOfTableWithoutYang'
```
This error happened because `find_ref_paths` would remove tables without yang as a side-effect.

Earlier this was fixed by cropping tables without yang models from current and target configs. That fix had 2 issues:
- It is not addressing the real problem which is cropping the config as a side effect to `find_ref_paths`
- If the move generated by the inner logic of `PatchSorter` was to replace whole config, it would result in replacing whole config with the config with cropped non-yang tables

#### How I did it
- Fixing `find_ref_paths` to not crop the given config as a side-effect
  - Added a unit-test for that
- Revert the change to crop current config and target config in `PatchSorter.sort`
  - Added a unit-test to verify replace whole config move does not result unintentionally in removing tables without yang
- Added unit-test to verify calling `PatchSorter.sort` on a config with non-yang tables does not result in the exception: 
  ```
  KeyError: 'NameOfTableWithoutYang'
  ```

#### How to verify it
unit-tests

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)

generic_config_updater/gu_common.py

@@ -403,7 +403,9 @@ def find_ref_paths(self, path, config):
     def _find_leafref_paths(self, path, config):
         sy = self.config_wrapper.create_sonic_yang_with_loaded_models()
 
-        sy.loadData(config)
+        tmp_config = copy.deepcopy(config)
+
+        sy.loadData(tmp_config)
 
         xpath = self.convert_path_to_xpath(path, config, sy)
 

generic_config_updater/patch_sorter.py

@@ -36,6 +36,13 @@ def apply_move(self, move):
     def has_no_diff(self):
         return self.current_config == self.target_config
 
+    def __str__(self):
+        return f"""current_config: {self.current_config}
+target_config: {self.target_config}"""
+
+    def __repr__(self):
+        return str(self)
+
 class JsonMove:
     """
     A class similar to JsonPatch operation, but it allows the path to refer to non-existing middle elements.
@@ -1333,10 +1340,7 @@ def sort(self, patch, algorithm=Algorithm.DFS, preloaded_current_config=None):
         current_config = preloaded_current_config if preloaded_current_config else self.config_wrapper.get_config_db_as_json()
         target_config = self.patch_wrapper.simulate_patch(patch, current_config)
 
-        cropped_current_config = self.config_wrapper.crop_tables_without_yang(current_config)
-        cropped_target_config = self.config_wrapper.crop_tables_without_yang(target_config)
-
-        diff = Diff(cropped_current_config, cropped_target_config)
+        diff = Diff(current_config, target_config)
 
         sort_algorithm = self.sort_algorithm_factory.create(algorithm)
         moves = sort_algorithm.sort(diff)

tests/generic_config_updater/files/patch_sorter_test_failure.json

@@ -67,5 +67,43 @@
         "expected_error_substrings": [
             "There is no possible sorting"
         ]
+    },
+    "EMPTYING_A_CONFIGDB_TABLE_AND_CONFIG_HAS_NON_YANG_TABLE__FAILURE": {
+        "desc": [
+            "Emptying a configdb table fails because empty tables are not allowed in configdb.",
+            "User should remove whole table instead e.g. remove /ACL_TABLE in this case.",
+            "Also there is a table without YANG in the config, which the sorting logic will loop over.",
+            "The sorting logic should fail with GenericConfigUpdaterError: 'There is no possible sorting' and not KeyError: 'TABLE_WITHOUT_YANG'"
+        ],
+        "current_config": {
+            "TABLE_WITHOUT_YANG": {
+                "key1": "value1",
+                "key2": "value2"
+            },
+            "ACL_TABLE": {
+                "EVERFLOW": {
+                    "policy_desc": "EVERFLOW",
+                    "ports": [
+                        "Ethernet0"
+                    ],
+                    "stage": "ingress",
+                    "type": "MIRROR"
+                }
+            },
+            "PORT": {
+                "Ethernet0": {
+                    "alias": "Eth1",
+                    "lanes": "65, 66, 67, 68",
+                    "description": "Ethernet0 100G link",
+                    "speed": "100000"
+                }
+            }
+        },
+        "patch": [
+            {"op": "remove", "path": "/ACL_TABLE/EVERFLOW"}
+        ],
+        "expected_error_substrings": [
+            "There is no possible sorting"
+        ]
     }
 }

tests/generic_config_updater/gu_common_test.py

@@ -1,3 +1,4 @@
+import copy
 import json
 import jsonpatch
 import sonic_yang
@@ -674,6 +675,18 @@ def test_find_ref_paths__whole_config_path__returns_all_refs(self):
         # Assert
         self.assertEqual(expected, actual)
 
+    def test_find_ref_paths__does_not_remove_tables_without_yang(self):
+        # Arrange
+        config = Files.CONFIG_DB_AS_JSON # This has a table without yang named 'TABLE_WITHOUT_YANG'
+        any_path = ""
+        expected_config = copy.deepcopy(config)
+
+        # Act
+        self.path_addressing.find_ref_paths(any_path, config)
+
+        # Assert
+        self.assertEqual(expected_config, config)
+
     def test_convert_path_to_xpath(self):
         def check(path, xpath, config=None):
             if not config:

tests/generic_config_updater/patch_sorter_test.py

@@ -1829,7 +1829,23 @@ def run_single_failure_case(self, data):
             if notfound_substrings:
                 self.fail(f"Did not find the substrings {notfound_substrings} in the error: '{error}'")
 
-    def create_patch_sorter(self, config=None):
+    def test_sort__does_not_remove_tables_without_yang_unintentionally_if_generated_change_replaces_whole_config(self):
+        # Arrange
+        current_config = Files.CONFIG_DB_AS_JSON # has a table without yang named 'TABLE_WITHOUT_YANG'
+        any_patch = Files.SINGLE_OPERATION_CONFIG_DB_PATCH
+        target_config = any_patch.apply(current_config)
+        sort_algorithm = Mock()
+        sort_algorithm.sort = lambda diff: [ps.JsonMove(diff, OperationType.REPLACE, [], [])]
+        patch_sorter = self.create_patch_sorter(current_config, sort_algorithm)
+        expected = [JsonChange(jsonpatch.JsonPatch([OperationWrapper().create(OperationType.REPLACE, "", target_config)]))]
+
+        # Act
+        actual = patch_sorter.sort(any_patch)
+
+        # Assert
+        self.assertEqual(expected, actual)
+
+    def create_patch_sorter(self, config=None, sort_algorithm=None):
         if config is None:
             config=Files.CROPPED_CONFIG_DB_AS_JSON
         config_wrapper = self.config_wrapper
@@ -1838,6 +1854,8 @@ def create_patch_sorter(self, config=None):
         operation_wrapper = OperationWrapper()
         path_addressing= ps.PathAddressing(config_wrapper)
         sort_algorithm_factory = ps.SortAlgorithmFactory(operation_wrapper, config_wrapper, path_addressing)
+        if sort_algorithm:
+            sort_algorithm_factory.create = MagicMock(return_value=sort_algorithm)
 
         return ps.PatchSorter(config_wrapper, patch_wrapper, sort_algorithm_factory)