Merge branch 'bugfix/wrong-calculation-for-ispublic-fields-#329' into…

… test-coverage-search-api

datagateway_api/config.json.example

@@ -12,8 +12,7 @@
     "search_api": {
         "extension": "/search-api",
         "icat_url": "https://localhost:8181",
-        "icat_check_cert": false,
-        "num_of_years_determining_public_data": 3
+        "icat_check_cert": false
     },
     "flask_reloader": false,
     "log_level": "WARN",

datagateway_api/search_api_mapping.json.example

@@ -12,7 +12,7 @@
         "base_icat_entity": "Dataset",
         "pid": ["doi", "id"],
         "title": "name",
-        "isPublic": "createTime",
+        "isPublic": "",
         "creationDate": "createTime",
         "size": "",
         "documents": {"Document": "investigation"},
@@ -25,7 +25,7 @@
     "Document": {
         "base_icat_entity": "Investigation",
         "pid": ["doi", "id"],
-        "isPublic": "releaseDate",
+        "isPublic": "",
         "type": "type.name",
         "title": "name",
         "summary": "summary",

datagateway_api/src/common/config.py

@@ -130,7 +130,6 @@ class SearchAPI(BaseModel):
     extension: StrictStr
     icat_check_cert: StrictBool
     icat_url: StrictStr
-    num_of_years_determining_public_data: StrictInt
 
     _validate_extension = validator("extension", allow_reuse=True)(validate_extension)
 

datagateway_api/src/search_api/models.py

@@ -1,14 +1,11 @@
 from abc import ABC, abstractmethod
-from datetime import datetime, timezone
+from datetime import datetime
 import sys
 from typing import ClassVar, List, Optional, Union
 
-from dateutil.relativedelta import relativedelta
-from pydantic import BaseModel, Field, ValidationError, validator
+from pydantic import BaseModel, Field, root_validator, ValidationError, validator
 from pydantic.error_wrappers import ErrorWrapper
 
-from datagateway_api.src.common.config import Config
-from datagateway_api.src.common.date_handler import DateHandler
 from datagateway_api.src.search_api.panosc_mappings import mappings
 
 
@@ -195,17 +192,12 @@ class Dataset(PaNOSCAttribute):
     def set_pid(cls, value):  # noqa: B902, N805
         return f"pid:{value}" if isinstance(value, int) else value
 
-    @validator("is_public", pre=True, always=True)
-    def set_is_public(cls, value):  # noqa: B902, N805
-        if not value:
-            return value
-
-        creation_date = DateHandler.str_to_datetime_object(value)
-        current_datetime = datetime.now(timezone.utc)
-        rd = relativedelta(
-            years=Config.config.search_api.num_of_years_determining_public_data,
-        )
-        return creation_date < (current_datetime - rd)
+    @root_validator(pre=True)
+    def set_is_public(cls, values):  # noqa: B902, N805
+        # Hardcoding this to True because anon user is used for querying so all data
+        # returned by it is public
+        values["isPublic"] = True
+        return values
 
     @classmethod
     def from_icat(cls, icat_data, required_related_fields):
@@ -240,17 +232,12 @@ class Document(PaNOSCAttribute):
     def set_pid(cls, value):  # noqa: B902, N805
         return f"pid:{value}" if isinstance(value, int) else value
 
-    @validator("is_public", pre=True, always=True)
-    def set_is_public(cls, value):  # noqa: B902, N805
-        if not value:
-            return value
-
-        creation_date = DateHandler.str_to_datetime_object(value)
-        current_datetime = datetime.now(timezone.utc)
-        rd = relativedelta(
-            years=Config.config.search_api.num_of_years_determining_public_data,
-        )
-        return creation_date < (current_datetime - rd)
+    @root_validator(pre=True)
+    def set_is_public(cls, values):  # noqa: B902, N805
+        # Hardcoding this to True because anon user is used for querying so all data
+        # returned by it is public
+        values["isPublic"] = True
+        return values
 
     @classmethod
     def from_icat(cls, icat_data, required_related_fields):

datagateway_api/src/search_api/query_filter_factory.py

@@ -1,10 +1,7 @@
-from datetime import datetime, timezone
 import logging
 
-from dateutil.relativedelta import relativedelta
 
 from datagateway_api.src.common.base_query_filter_factory import QueryFilterFactory
-from datagateway_api.src.common.config import Config
 from datagateway_api.src.common.exceptions import FilterError, SearchAPIError
 from datagateway_api.src.search_api.filters import (
     SearchAPIIncludeFilter,
@@ -175,16 +172,19 @@ def get_where_filter(where_filter_input, entity_name):
                 )
         else:
             log.info("Basic where filter found, extracting field, value and operation")
-            filter_data = SearchAPIQueryFilterFactory.get_condition_values(
+            field, value, operation = SearchAPIQueryFilterFactory.get_condition_values(
                 where_filter_input,
             )
-            where_filters.append(
-                SearchAPIWhereFilter(
-                    field=filter_data[0],
-                    value=filter_data[1],
-                    operation=filter_data[2],
-                ),
-            )
+
+            # Ignore filters on `isPublic`` fields as data is always public. Ensure that
+            # empty list is returned when filtering for non-public data.
+            if field == "isPublic":
+                value = not value if operation == "neq" else value
+                if not value:
+                    where_filters.append(SearchAPISkipFilter(1))
+                    where_filters.append(SearchAPILimitFilter(0))
+            else:
+                where_filters.append(SearchAPIWhereFilter(field, value, operation))
 
         return where_filters
 
@@ -300,14 +300,6 @@ def get_condition_values(conditions_dict):
                     "Bad Where filter: Invalid operator used with boolean value",
                 )
 
-        if field == "isPublic":
-            (
-                value,
-                operation,
-            ) = SearchAPIQueryFilterFactory.convert_is_public_field_value_and_operation(
-                value, operation,
-            )
-
         return field, value, operation
 
     @staticmethod
@@ -344,36 +336,3 @@ def prefix_where_filter_field_with_entity_name(where_filters, entity_name):
                     )
             if isinstance(where_filter, SearchAPIWhereFilter):
                 where_filter.field = f"{entity_name}.{where_filter.field}"
-
-    @staticmethod
-    def convert_is_public_field_value_and_operation(value, operation):
-        """
-        The ICAT mappings for the isPublic PaNOSC fields are not direct and as a result
-        of this, we calculate whether data is public or not at the Search API level.
-        For example, in the case of ISIS, Dataset's isPublic field maps to Dataset's
-        createTime field in ICAT. We take this datetime value and determine whether it
-        is public or not by checking whether the datetime is more than 3 years ago or
-        not. The isPublic fields are of type boolean so any WHERE filter applied to
-        them will have a value that is boolean too. As a result of this, the value and
-        operation need to be converted to a format appropriate for the mapped ICAT
-        field. In the example above, if the filter asks for all Datasets whose isPublic
-        fields are set to True to be returned, then this method changes the WHERE
-        filter value to a three years ago datetime value and the operator to less than
-        so that all Datasets older than 3 years (which ISIS considers public) are
-        returned.
-        """
-        value = not value if operation == "neq" else value
-        if value is True:
-            operation = "lt"
-        else:
-            operation = "gt"
-
-        current_datetime = datetime.now(timezone.utc)
-        rd = relativedelta(
-            years=Config.config.search_api.num_of_years_determining_public_data,
-        )
-        # The timezone part has a plus sign so replacing
-        # with a blank space to avoid issues
-        value = str(current_datetime - rd).replace("+", " ")
-
-        return value, operation

test/conftest.py

@@ -104,7 +104,6 @@ def test_config_data():
             "extension": "/search-api",
             "icat_url": "https://localhost.testdomain:8181",
             "icat_check_cert": True,
-            "num_of_years_determining_public_data": 3,
         },
         "flask_reloader": False,
         "log_level": "WARN",

test/search_api/conftest.py

@@ -43,7 +43,7 @@ def test_search_api_mappings_data():
             "base_icat_entity": "Dataset",
             "pid": ["doi", "id"],
             "title": "name",
-            "isPublic": "createTime",
+            "isPublic": "",
             "creationDate": "createTime",
             "size": "",
             "documents": {"Document": "investigation"},
@@ -56,7 +56,7 @@ def test_search_api_mappings_data():
         "Document": {
             "base_icat_entity": "Investigation",
             "pid": ["doi", "id"],
-            "isPublic": "releaseDate",
+            "isPublic": "",
             "type": "type.name",
             "title": "name",
             "summary": "summary",

test/search_api/endpoints/test_count_endpoint.py

@@ -1,9 +1,6 @@
-from unittest.mock import patch
-
 import pytest
 
 from datagateway_api.src.common.config import Config
-from datagateway_api.src.common.date_handler import DateHandler
 
 
 class TestSearchAPICountEndpoint:
@@ -95,38 +92,36 @@ def test_valid_count_endpoint(
             pytest.param(
                 "datasets",
                 '{"isPublic": true}',
-                {"count": 462},
-                id="Dataset count with isPublic condition",
+                {"count": 479},
+                id="Dataset count with isPublic condition (True)",
             ),
             pytest.param(
                 "documents",
                 '{"isPublic": true}',
-                {"count": 76},
-                id="Document count with isPublic condition",
+                {"count": 239},
+                id="Document count with isPublic condition (True)",
+            ),
+            pytest.param(
+                "datasets",
+                '{"isPublic": false}',
+                {"count": 0},
+                id="Dataset count with isPublic condition (False)",
+                # Skipped due to skip filter causing issue on count endpoints
+                marks=pytest.mark.skip,
+            ),
+            pytest.param(
+                "documents",
+                '{"isPublic": false}',
+                {"count": 0},
+                id="Document count with isPublic condition (False)",
+                # Skipped due to skip filter causing issue on count endpoints
+                marks=pytest.mark.skip,
             ),
         ],
     )
-    @patch("datagateway_api.src.search_api.query_filter_factory.datetime")
     def test_valid_count_endpoint_is_public_field(
-        self,
-        datetime_mock,
-        flask_test_app_search_api,
-        endpoint_name,
-        request_filter,
-        expected_json,
+        self, flask_test_app_search_api, endpoint_name, request_filter, expected_json,
     ):
-        """
-        The datetime must be mocked here to prevent tests from failing as time passes.
-        A dataset or document that was created or released 2 years and 364 ago would be
-        fall in the not public category, however that same dataset or document would
-        fall in the public category (in the case of ISIS) a few days later because it
-        will be 3 years old. As a result of this, the tests will fail because the actual
-        count will be different to that of the expected. Mocking datetime takes care of
-        this issue because it sets the time to the one provided in this test method.
-        """
-        datetime_mock.now.return_value = DateHandler.str_to_datetime_object(
-            "2022-02-06 00:00:01+00:00",
-        )
         test_response = flask_test_app_search_api.get(
             f"{Config.config.search_api.extension}/{endpoint_name}/count?where="
             f"{request_filter}",

test/search_api/endpoints/test_search_endpoint.py

@@ -416,7 +416,13 @@ class TestSearchAPISearchEndpoint:
                         "samples": [],
                     },
                 ],
-                id="Search datasets with isPublic condition",
+                id="Search datasets with isPublic condition (True)",
+            ),
+            pytest.param(
+                "datasets",
+                '{"where": {"isPublic": false}}',
+                [],
+                id="Search datasets with isPublic condition (False)",
             ),
         ],
     )

test/search_api/test_models.py

@@ -1,5 +1,3 @@
-from datetime import datetime, timezone
-
 from pydantic import ValidationError
 import pytest
 
@@ -88,7 +86,7 @@
 INVESTIGATION_ICAT_DATA = {
     "endDate": "2000-12-31 00:00:00+00:00",
     "name": "Test name",
-    "releaseDate": str(datetime.now(timezone.utc)),
+    "releaseDate": "2000-12-31 00:00:00+00:00",
     "id": 1,
     "modTime": "2000-12-31 00:00:00+00:00",
     "modId": "Test modId",
@@ -216,7 +214,7 @@
 
 DOCUMENT_PANOSC_DATA = {
     "pid": INVESTIGATION_ICAT_DATA["doi"],
-    "isPublic": False,
+    "isPublic": True,
     "type": INVESTIGATION_TYPE_ICAT_DATA["name"],
     "title": INVESTIGATION_ICAT_DATA["name"],
     "summary": INVESTIGATION_ICAT_DATA["summary"],