Merge pull request #235 from ral-facilities/bugfix/nested-include-fil…

…ter-inputs-#234 Fix complex include filters
ral-facilities · May 24, 2021 · d72e473 · d72e473
2 parents 205f155 + 2961b18
commit d72e473
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 2 deletions.
diff --git a/.flake8 b/.flake8
@@ -11,4 +11,5 @@ per-file-ignores =
   util/icat_db_generator.py: S311
   datagateway_api/wsgi.py:E402,F401
   datagateway_api/common/database/models.py: N815,A003
+  datagateway_api/common/icat/filters.py: C901
 enable-extensions=G
diff --git a/datagateway_api/common/icat/filters.py b/datagateway_api/common/icat/filters.py
@@ -241,8 +241,31 @@ def _extract_filter_fields(self, field):
                     self._extract_filter_fields(".".join((key, value)))
                 elif isinstance(value, list):
                     for element in value:
-                        # Will end up as: key.element1, key.element2, key.element3 etc.
-                        self._extract_filter_fields(".".join((key, element)))
+                        if isinstance(element, str):
+                            # Will end up as: key.element1, key.element2 etc.
+                            self._extract_filter_fields(".".join((key, element)))
+                        elif isinstance(element, list):
+                            for sub_element in element:
+                                self._extract_filter_fields(
+                                    ".".join((key, sub_element)),
+                                )
+                        elif isinstance(element, dict):
+                            for (
+                                inner_element_key,
+                                inner_element_value,
+                            ) in element.items():
+                                if not isinstance(inner_element_key, str):
+                                    raise FilterError(
+                                        "Include Filter: Dictionary key should only be"
+                                        " a string, not any other type",
+                                    )
+                                self._extract_filter_fields(
+                                    {
+                                        ".".join(
+                                            (key, inner_element_key),
+                                        ): inner_element_value,
+                                    },
+                                )
                 elif isinstance(value, dict):
                     for inner_key, inner_value in value.items():
                         if not isinstance(inner_key, str):

diff --git a/test/icat/filters/test_include_filter.py b/test/icat/filters/test_include_filter.py
@@ -39,6 +39,19 @@ class TestICATIncludeFilter:
                 {"investigationUsers", "datasets", "facility"},
                 id="nested list",
             ),
+            pytest.param(
+                {"investigationUsers": [{"user": "userGroups"}, "investigation", []]},
+                {
+                    "investigationUsers.user.userGroups",
+                    "investigationUsers.investigation",
+                },
+                id="complex use case used similarly in DataGateway",
+            ),
+            pytest.param(
+                {"investigationUsers": [["investigation.datasets"]]},
+                {"investigationUsers.investigation.datasets"},
+                id="dictionary with nested list value",
+            ),
         ],
     )
     def test_valid_input(self, icat_query, filter_input, expected_output):
@@ -67,6 +80,10 @@ def test_invalid_field(self, icat_query):
                 {"datasets": {"datafiles", "sample"}},
                 id="invalid inner dictionary value",
             ),
+            pytest.param(
+                {"investigationUsers": [{2: "userGroups"}, "investigation"]},
+                id="Invalid inner key with nested dict and list",
+            ),
         ],
     )
     def test_invalid_extract_field(self, filter_input):