Skip to content

Commit

Permalink
Merge branch 'master' into bugfix/validation-error-sample-pid-panosc-…
Browse files Browse the repository at this point in the history
…field-#314
  • Loading branch information
VKTB committed Feb 10, 2022
2 parents e7fa1b1 + 80222ee commit bed6dec
Show file tree
Hide file tree
Showing 12 changed files with 214 additions and 33 deletions.
24 changes: 24 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,30 @@

<!--next-version-placeholder-->

## v4.0.0 (2022-02-10)
### Feature
* **config:** Add configuration option for determining public data #312 ([`58e777b`](https://github.com/ral-facilities/datagateway-api/commit/58e777b5c4a562f6945adcd1b55ce1d470f5d816))

### Breaking
* add configuration option for determining public data #312 ([`58e777b`](https://github.com/ral-facilities/datagateway-api/commit/58e777b5c4a562f6945adcd1b55ce1d470f5d816))

## v3.6.1 (2022-02-07)
### Fix
* Convert `isPublic` PaNOSC filter to appropriate ICAT filter #308 ([`6a40307`](https://github.com/ral-facilities/datagateway-api/commit/6a40307ba19d5818bdb6bf1acc79d98abd6a3f83))
* Make WHERE filter without operator work with int and bool #322 ([`6988a5a`](https://github.com/ral-facilities/datagateway-api/commit/6988a5aa5d6dfa71fd4b90a73b050864e8530955))

## v3.6.0 (2022-02-07)
### Feature
* Implement search API endpoints #266, #267, #268 ([`dcc332e`](https://github.com/ral-facilities/datagateway-api/commit/dcc332e352ded8af25dce7dae635bd62417d2c13))

### Fix
* Make get by pid endpoints return data in PaNOSC format #266 ([`0de2b5b`](https://github.com/ral-facilities/datagateway-api/commit/0de2b5b2b713699b66164ca5732888f997230aa5))
* Add logic to deal with `PythonICATIncludeFilter` that could be related for ICAT relations for non-related PaNOSC fields #268 ([`29232c6`](https://github.com/ral-facilities/datagateway-api/commit/29232c6b2c032c61999118b2f69177f3b9bd5d57))
* Correct order of arguments for where filter #266 ([`1e38eae`](https://github.com/ral-facilities/datagateway-api/commit/1e38eaee9f201a45742cf868ad2fa28f4adee065))

### Documentation
* Add docstrings for Flask resource classes #268 ([`a5aee61`](https://github.com/ral-facilities/datagateway-api/commit/a5aee61cc55e70931163371f3c1abecb31b1fb3a))

## v3.5.3 (2022-02-02)
### Fix
* Retrieve non-related fields that have a list of ICAT relations #265 ([`2c5edc5`](https://github.com/ral-facilities/datagateway-api/commit/2c5edc50f9f713b0d15e137ad4a307a90a86b5aa))
Expand Down
3 changes: 2 additions & 1 deletion datagateway_api/config.json.example
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"search_api": {
"extension": "/search-api",
"icat_url": "https://localhost:8181",
"icat_check_cert": false
"icat_check_cert": false,
"num_of_years_determining_public_data": 3
},
"flask_reloader": false,
"log_level": "WARN",
Expand Down
1 change: 1 addition & 0 deletions datagateway_api/src/common/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ 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)

Expand Down
3 changes: 0 additions & 3 deletions datagateway_api/src/common/filter_order_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ def add_icat_relations_for_panosc_non_related_fields(
:param panosc_entity_name: A PaNOSC entity name e.g. "Dataset"
:type panosc_entity_name: :class:`str`
:return: ICAT relations for the non related fields of the given PaNOSC entity
"""

icat_relations = mappings.get_icat_relations_for_panosc_non_related_fields(
Expand All @@ -129,8 +128,6 @@ def add_icat_relations_for_panosc_non_related_fields(
if icat_relations:
self.filters.append(PythonICATIncludeFilter(icat_relations))

return icat_relations

def merge_python_icat_limit_skip_filters(self):
"""
When there are both limit and skip filters in a request, merge them into the
Expand Down
16 changes: 11 additions & 5 deletions datagateway_api/src/search_api/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

from datagateway_api.src.common.exceptions import MissingRecordError
from datagateway_api.src.common.filter_order_handler import FilterOrderHandler
from datagateway_api.src.search_api.filters import SearchAPIWhereFilter
from datagateway_api.src.search_api.filters import (
SearchAPIIncludeFilter,
SearchAPIWhereFilter,
)
import datagateway_api.src.search_api.models as models
from datagateway_api.src.search_api.query import SearchAPIQuery
from datagateway_api.src.search_api.session_handler import (
Expand Down Expand Up @@ -32,13 +35,16 @@ def get_search(entity_name, filters):
log.info("Searching for %s using request's filters", entity_name)
log.debug("Entity Name: %s, Filters: %s", entity_name, filters)

entity_relations = []
for filter_ in filters:
if isinstance(filter_, SearchAPIIncludeFilter):
entity_relations.extend(filter_.included_filters)

query = SearchAPIQuery(entity_name)

filter_handler = FilterOrderHandler()
filter_handler.add_filters(filters)
icat_relations = filter_handler.add_icat_relations_for_panosc_non_related_fields(
entity_name,
)
filter_handler.add_icat_relations_for_panosc_non_related_fields(entity_name)
filter_handler.add_icat_relations_for_non_related_fields_of_panosc_related_entities(
entity_name,
)
Expand All @@ -51,7 +57,7 @@ def get_search(entity_name, filters):
panosc_data = []
for icat_data in icat_query_data:
panosc_model = getattr(models, entity_name)
panosc_record = panosc_model.from_icat(icat_data, icat_relations).json(
panosc_record = panosc_model.from_icat(icat_data, entity_relations).json(
by_alias=True,
)
panosc_data.append(json.loads(panosc_record))
Expand Down
13 changes: 9 additions & 4 deletions datagateway_api/src/search_api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from pydantic import BaseModel, Field, 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

Expand Down Expand Up @@ -197,8 +198,10 @@ def set_is_public(cls, value): # noqa: B902, N805

creation_date = DateHandler.str_to_datetime_object(value)
current_datetime = datetime.now(timezone.utc)
three_years_ago = current_datetime - relativedelta(years=3)
return creation_date < three_years_ago
rd = relativedelta(
years=Config.config.search_api.num_of_years_determining_public_data,
)
return creation_date < (current_datetime - rd)

@classmethod
def from_icat(cls, icat_data, required_related_fields):
Expand Down Expand Up @@ -236,8 +239,10 @@ def set_is_public(cls, value): # noqa: B902, N805

creation_date = DateHandler.str_to_datetime_object(value)
current_datetime = datetime.now(timezone.utc)
three_years_ago = current_datetime - relativedelta(years=3)
return creation_date < three_years_ago
rd = relativedelta(
years=Config.config.search_api.num_of_years_determining_public_data,
)
return creation_date < (current_datetime - rd)

@classmethod
def from_icat(cls, icat_data, required_related_fields):
Expand Down
52 changes: 51 additions & 1 deletion datagateway_api/src/search_api/query_filter_factory.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
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,
Expand Down Expand Up @@ -277,7 +281,7 @@ def get_condition_values(conditions_dict):
field = list(conditions_dict.keys())[0]
filter_data = list(conditions_dict.values())[0]

if isinstance(filter_data, str):
if isinstance(filter_data, (bool, int, str)):
# Format: {"where": {"property": "value"}}
log.debug("Format of WHERE filter: {'where': {'property': 'value'}}")
value = conditions_dict[field]
Expand All @@ -291,6 +295,19 @@ def get_condition_values(conditions_dict):
value = list(conditions_dict[field].values())[0]
operation = list(conditions_dict[field].keys())[0]

if isinstance(value, bool) and operation not in ["eq", "neq"]:
raise FilterError(
"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
Expand Down Expand Up @@ -327,3 +344,36 @@ 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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "datagateway-api"
version = "3.5.3"
version = "4.0.0"
description = "ICAT API to interface with the DataGateway"
license = "Apache-2.0"
readme = "README.md"
Expand Down
1 change: 1 addition & 0 deletions test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ 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",
Expand Down
61 changes: 47 additions & 14 deletions test/search_api/endpoints/test_count_endpoint.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
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:
Expand Down Expand Up @@ -67,33 +70,63 @@ class TestSearchAPICountEndpoint:
{"count": 13},
id="Instrument count with where (operator specified)",
),
pytest.param(
"instruments",
'{"facility": {"like": "LILS"}}',
{"count": 14},
id="Instrument count with where using related ICAT mapping",
),
],
)
def test_valid_count_endpoint(
self, flask_test_app_search_api, endpoint_name, request_filter, expected_json,
):
test_response = flask_test_app_search_api.get(
f"{Config.config.search_api.extension}/{endpoint_name}/count?where="
f"{request_filter}",
)

assert test_response.status_code == 200
assert test_response.json == expected_json

@pytest.mark.parametrize(
"endpoint_name, request_filter, expected_json",
[
pytest.param(
"datasets",
'{"isPublic": true}',
{"count": 479},
{"count": 462},
id="Dataset count with isPublic condition",
# Skipped because the where for isPublic doesn't work
marks=pytest.mark.skip,
),
pytest.param(
"documents",
'{"isPublic": true}',
{"count": 239},
{"count": 76},
id="Document count with isPublic condition",
# Skipped because the where for isPublic doesn't work
marks=pytest.mark.skip,
),
pytest.param(
"instruments",
'{"facility": {"like": "LILS"}}',
{"count": 14},
id="Instrument count with where using related ICAT mapping",
),
],
)
def test_valid_count_endpoint(
self, flask_test_app_search_api, endpoint_name, request_filter, expected_json,
@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,
):
"""
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}",
Expand Down
18 changes: 15 additions & 3 deletions test/search_api/endpoints/test_search_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,22 @@ class TestSearchAPISearchEndpoint:
pytest.param(
"datasets",
'{"limit": 1, "where": {"isPublic": true}}',
[{}],
[
{
"pid": "0-449-78690-0",
"title": "DATASET 1",
"creationDate": "2002-11-27T06:20:36+00:00",
"isPublic": True,
"size": None,
"documents": [],
"techniques": [],
"instrument": None,
"files": [],
"parameters": [],
"samples": [],
},
],
id="Search datasets with isPublic condition",
# Skipped because the where for isPublic doesn't work
marks=pytest.mark.skip,
),
],
)
Expand Down
Loading

0 comments on commit bed6dec

Please sign in to comment.