refactor: make query params more efficient to create less include fil…

…ter objects for multiple and nested related entities #261
ral-facilities · Jan 10, 2022 · eab7f3d · eab7f3d
1 parent fc4556c
commit eab7f3d
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 22 deletions.
diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py
@@ -15,10 +15,6 @@
 log = logging.getLogger()
 
 
-# TODO - Implement each of these filters for Search API, inheriting from the Python ICAT
-# versions
-
-
 class SearchAPIWhereFilter(PythonICATWhereFilter):
     def __init__(self, field, value, operation, search_api_query=None):
         self.search_api_query = search_api_query

diff --git a/datagateway_api/src/search_api/panosc_mappings.py b/datagateway_api/src/search_api/panosc_mappings.py
@@ -45,7 +45,6 @@ def get_icat_mapping(self, panosc_entity_name, field_name):
             icat_mapping = mappings.mappings[panosc_entity_name][field_name]
             log.debug("ICAT mapping/translation found: %s", icat_mapping)
         except KeyError as e:
-            # TODO - do we want to change the exception?
             raise FilterError(f"Bad PaNOSC to ICAT mapping: {e.args}")
 
         if isinstance(icat_mapping, str):

diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py
@@ -202,18 +202,25 @@ def get_include_filter(include_filter_input, entity_name):
         are part of include filters.
 
         :param include_filter_input: The filter from which to create a list of
-            `SearchAPIIncludeFilter` and any `NestedWhereFilters` and/ or
+            `SearchAPIIncludeFilter` and any `NestedWhereFilters` and/or
             `SearchAPIWhereFilter` objects
-        :type include_filter_input: :class:`dict`
+        :type include_filter_input: :class:`list`
         :return: The list of `SearchAPIIncludeFilter` and any `NestedWhereFilters` and/
             or `SearchAPIWhereFilter` objects created
         :raises FilterError: If scope filter has a limit or skip filter
         """
+
         query_filters = []
+        included_entities = []
+
         for related_model in include_filter_input:
+            log.debug("Related model: %s", related_model)
+            nested_include = False
             included_entity = related_model["relation"]
+            # List of included entities made so they can put into a single include
+            # filter instead of having a new filter object for each related entity
+            included_entities.append(related_model["relation"])
 
-            nested_include = False
             if "scope" in related_model:
                 log.info("Scope found in include JSON object")
                 if "limit" in related_model["scope"]:
@@ -245,17 +252,23 @@ def get_include_filter(include_filter_input, entity_name):
                             scope_query_filter, included_entity,
                         )
                     if isinstance(scope_query_filter, SearchAPIIncludeFilter):
-                        nested_include = True
-                        included_filter = scope_query_filter.included_filters[0]
-
-                        scope_query_filter.included_filters[
-                            0
-                        ] = f"{included_entity}.{included_filter}"
+                        for included_entity_pointer in range(
+                            len(scope_query_filter.included_filters),
+                        ):
+                            nested_include = True
+                            included_filter = scope_query_filter.included_filters[
+                                included_entity_pointer
+                            ]
+                            scope_query_filter.included_filters[
+                                included_entity_pointer
+                            ] = f"{included_entity}.{included_filter}"
 
                 query_filters.extend(scope_query_filters)
 
-            if not nested_include:
-                query_filters.append(SearchAPIIncludeFilter(included_entity))
+        if not nested_include:
+            query_filters.append(
+                SearchAPIIncludeFilter(included_entities, entity_name),
+            )
 
         return query_filters
 

diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py
@@ -1252,8 +1252,8 @@ def test_valid_where_filter_with_nested_or_boolean_operator(
                     },
                 },
                 "Dataset",
-                2,
-                [["files"], ["instrument"]],
+                1,
+                [["files", "instrument"]],
                 id="Multiple related models",
             ),
         ],
@@ -1654,8 +1654,8 @@ def test_valid_include_filter(
                     },
                 },
                 "Dataset",
-                4,
-                [["parameters"], ["documents"]],
+                3,
+                [["parameters", "documents"]],
                 [
                     SearchAPIWhereFilter("parameters.name", "My parameter", "eq"),
                     SearchAPIWhereFilter("documents.title", "Document title", "eq"),
@@ -1763,8 +1763,8 @@ def test_valid_include_filter_with_where_filter_in_scope(
                     },
                 },
                 "Document",
-                2,
-                [["datasets.parameters"], ["datasets.instrument"]],
+                1,
+                [["datasets.parameters", "datasets.instrument"]],
                 id="Multiple related models",
             ),
             pytest.param(