From 93489c1922cb1714f8080247d4080fd5596dbd92 Mon Sep 17 00:00:00 2001
From: Stefaan Lippens <stefaan.lippens@vito.be>
Date: Thu, 24 Nov 2022 21:13:51 +0100
Subject: [PATCH] fixup! issue #76 ignore description when comparing returns
 field

---
 src/openeo_aggregator/metadata/merging.py | 13 +++++---
 src/openeo_aggregator/utils.py            | 16 ++++++----
 tests/metadata/test_merging.py            | 13 ++++----
 tests/test_utils.py                       | 37 +++++++++++++++++++++++
 4 files changed, 61 insertions(+), 18 deletions(-)

diff --git a/src/openeo_aggregator/metadata/merging.py b/src/openeo_aggregator/metadata/merging.py
index c919e6ae..64af6064 100644
--- a/src/openeo_aggregator/metadata/merging.py
+++ b/src/openeo_aggregator/metadata/merging.py
@@ -7,7 +7,7 @@
 
 import difflib
 import flask
-import itertools
+import functools
 import json
 import logging
 from typing import Dict, Optional, Callable, Any, List
@@ -21,7 +21,7 @@
 from openeo_aggregator.metadata.models.extent import Extent
 from openeo_aggregator.metadata.models.stac_summaries import StacSummaries
 from openeo_aggregator.metadata.reporter import LoggerReporter
-from openeo_aggregator.utils import MultiDictGetter, common_prefix, remove_key
+from openeo_aggregator.utils import MultiDictGetter, common_prefix, drop_dict_keys
 from openeo_driver.errors import OpenEOApiException
 
 _log = logging.getLogger(__name__)
@@ -255,6 +255,10 @@ def sort_dicts(x: Any) -> Any:
     )
 
 
+def ignore_description(data: Any) -> Any:
+    return drop_dict_keys(data, keys=["description"])
+
+
 class ProcessMetadataMerger:
     def __init__(self, report: Callable = DEFAULT_REPORTER.report):
         self.report = report
@@ -444,11 +448,10 @@ def _merge_process_returns(
         getter = MultiDictGetter(by_backend.values())
         # TODO: real merge instead of taking first schema as "merged" schema?
         merged = getter.first("returns", {"schema": {}})
-        remove_key(merged, "description")
+
         for backend_id, process_metadata in by_backend.items():
             other_returns = process_metadata.get("returns", {"schema": {}})
-            remove_key(other_returns, "description")
-            if other_returns != merged:
+            if ignore_description(other_returns) != ignore_description(merged):
                 self.report(
                     f"Returns schema is different from merged.",
                     backend_id=backend_id,
diff --git a/src/openeo_aggregator/utils.py b/src/openeo_aggregator/utils.py
index 7a6f60fe..1ba3b2b7 100644
--- a/src/openeo_aggregator/utils.py
+++ b/src/openeo_aggregator/utils.py
@@ -93,12 +93,16 @@ def dict_merge(*args, **kwargs) -> dict:
     return result
 
 
-def remove_key(container, key):
-    if type(container) is dict:
-        if key in container:
-            del container[key]
-        for v in container.values():
-            remove_key(v, key)
+def drop_dict_keys(data: Any, keys: List[Any]) -> Any:
+    """Recursively drop given keys from (nested) dictionaries"""
+    if isinstance(data, dict):
+        return {
+            k: drop_dict_keys(v, keys=keys) for k, v in data.items() if k not in keys
+        }
+    elif isinstance(data, (list, tuple)):
+        return type(data)(drop_dict_keys(v, keys=keys) for v in data)
+    else:
+        return data
 
 
 class EventHandler:
diff --git a/tests/metadata/test_merging.py b/tests/metadata/test_merging.py
index 3804ca12..3326e9fc 100644
--- a/tests/metadata/test_merging.py
+++ b/tests/metadata/test_merging.py
@@ -65,9 +65,7 @@ def test_merge_process_returns(self, merger, reporter):
                     "returns": {
                         "schema": {
                             "type": "array",
-                            "items": {
-                                "description": "All data type are allowed."
-                            }
+                            "items": {"description": "All data types are allowed."},
                         },
                         "description": "some description"
                     }
@@ -91,10 +89,11 @@ def test_merge_process_returns(self, merger, reporter):
             "description": "add",
             "parameters": [],
             'returns': {
-                'schema': {
-                    'type': 'array',
-                    'items': {}
-                }
+                "description": "some description",
+                "schema": {
+                    "type": "array",
+                    "items": {"description": "All data types are allowed."},
+                },
             },
             "federation:backends": ["b1", "b2"],
             'deprecated': False,
diff --git a/tests/test_utils.py b/tests/test_utils.py
index e7e6bdb1..bda5ca21 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -11,6 +11,7 @@
     strip_join,
     timestamp_to_rfc3339,
     common_prefix,
+    drop_dict_keys,
 )
 
 
@@ -259,3 +260,39 @@ def test_common_prefix_multiple():
             range(1, 9),
         )
     ) == [1, 2, 3]
+
+
+def test_drop_dict_keys():
+    assert drop_dict_keys({}, keys=["foo"]) == {}
+    assert drop_dict_keys({"foo": 2, "bar": 3}, keys=["foo"]) == {"bar": 3}
+    assert drop_dict_keys(
+        [{"foo": 2, "bar": 3}, {"baz": 5}, {"meh": 8}], keys=["foo", "baz"]
+    ) == [{"bar": 3}, {}, {"meh": 8}]
+    assert drop_dict_keys(
+        {
+            "foo": {1: 1, 2: 2, 3: 3},
+            "bar": {2: 22, 3: 33, 4: 44},
+            "baz": {3: 333, 2: 222, 5: 555},
+        },
+        keys=[1, 2, 4, "bar"],
+    ) == {
+        "foo": {3: 3},
+        "baz": {3: 333, 5: 555},
+    }
+    assert drop_dict_keys(
+        [
+            [{1: 1}, {2: 2, 3: 3}],
+            ({3: 3}, {4: 4, 5: 5}),
+        ],
+        keys=[3, 5],
+    ) == [
+        [{1: 1}, {2: 2}],
+        ({}, {4: 4}),
+    ]
+
+
+def test_drop_dict_keys_copy():
+    d = {"foo": 1, "bar": 2}
+    res = drop_dict_keys(d, keys=["foo"])
+    assert d == {"foo": 1, "bar": 2}
+    assert res == {"bar": 2}