From f2f53c92229d052ae697787eb80a35dcd2ea3b45 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 4 Jan 2022 10:37:12 +0000 Subject: [PATCH 01/43] feat: implement basic version of `SearchAPIIncludeFilter` #261 --- datagateway_api/src/search_api/filters.py | 27 ++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index 845cb0cc..e206084f 100644 --- a/datagateway_api/src/search_api/filters.py +++ b/datagateway_api/src/search_api/filters.py @@ -178,8 +178,33 @@ def apply_filter(self, query): class SearchAPIIncludeFilter(PythonICATIncludeFilter): - def __init__(self, included_filters): + def __init__(self, included_filters, panosc_entity_name): + self.included_filters = included_filters + self.panosc_entity_name = panosc_entity_name super().__init__(included_filters) def apply_filter(self, query): + icat_field_names = [] + + for panosc_field_name in self.included_filters: + # Need an empty list at start of each iteration to clear field names from + # previous iterations + split_icat_field_name = [] + + panosc_entity_name = self.panosc_entity_name + split_panosc_fields = panosc_field_name.split(".") + + for split_field in split_panosc_fields: + panosc_entity_name, icat_field_name = mappings.get_icat_mapping( + panosc_entity_name, split_field, + ) + split_icat_field_name.append(icat_field_name) + + icat_field_names.append(".".join(split_icat_field_name)) + + # All PaNOSC field names translated to ICAT, so we can overwrite the PaNOSC + # versions with the ICAT mappings so they can be used to apply the filter via + # `super().apply_filter()` + self.included_filters = icat_field_names + return super().apply_filter(query.icat_query.query) From 00ee21d1bf0489b344fb58d092cc57154a3983e3 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 4 Jan 2022 10:38:59 +0000 Subject: [PATCH 02/43] refactor: move `get_icat_mapping()` into `PaNOSCMappings` #261 - I've moved the code which deals with the parameter value field name list away from this function to make the function more generic/reusable. That code has been moved to applying the WHERE filter, where it'll be used. The mapping will not be reached by an include filter (under a valid API request) because the parameter values are for a field name/attribute, not a related entity --- datagateway_api/src/search_api/filters.py | 83 +++++-------------- .../src/search_api/panosc_mappings.py | 46 +++++++++- 2 files changed, 65 insertions(+), 64 deletions(-) diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index e206084f..793e9956 100644 --- a/datagateway_api/src/search_api/filters.py +++ b/datagateway_api/src/search_api/filters.py @@ -42,9 +42,28 @@ def apply_filter(self, query): # Convert PaNOSC field names to ICAT field names for field_name in panosc_field_names: - panosc_mapping_name, icat_field_name = self.get_icat_mapping( + panosc_mapping_name, icat_field_name = mappings.get_icat_mapping( panosc_mapping_name, field_name, ) + + # An edge case for ICAT has been somewhat hardcoded here, to deal with + # ICAT's different parameter value field names. The following mapping is + # assumed (where order matters): + # {"Parameter": {"value": ["numericValue", "stringValue", "dateTimeValue"]}} + if isinstance(icat_field_name, list): + if isinstance(self.value, int) or isinstance(self.value, float): + icat_field_name = icat_field_name[0] + elif isinstance(self.value, datetime): + icat_field_name = icat_field_name[2] + elif isinstance(self.value, str): + if DateHandler.is_str_a_date(self.value): + icat_field_name = icat_field_name[2] + else: + icat_field_name = icat_field_name[1] + else: + self.value = str(self.value) + icat_field_name = icat_field_name[1] + icat_field_names.append(icat_field_name) log.debug( @@ -60,68 +79,6 @@ def apply_filter(self, query): return super().apply_filter(query.icat_query.query) - def get_icat_mapping(self, panosc_entity_name, field_name): - """ - This function searches the PaNOSC mappings (from `search_api_mapping.json`, - maintained/stored by :class:`PaNOSCMappings`) and retrieves the ICAT translation - from the PaNOSC input. Fields in the same entity can be found, as well as fields - from related entities (e.g. Dataset.files.path) via recursion inside this - function. - - An edge case for ICAT has been somewhat hardcoded into this function to dea - with ICAT's different parameter value field names. The following mapping is - assumed (where order matters): - {"Parameter": {"value": ["numericValue", "stringValue", "dateTimeValue"]}} - - :param panosc_entity_name: A PaNOSC entity name e.g. "Dataset" - :type panosc_entity_name: :class:`str` - :param field_name: PaNOSC field name to fetch the ICAT version of e.g. "name" - :type field_name: :class:`str` - :return: Tuple containing the PaNOSC entity name (which will change from the - input if a related entity is found) and the ICAT field name - mapping/translation from the PaNOSC data model - :raises FilterError: If a valid mapping cannot be found - """ - - log.info( - "Searching mapping file to find ICAT translation for %s", - f"{panosc_entity_name}.{field_name}", - ) - - try: - icat_mapping = mappings.mappings[panosc_entity_name][field_name] - log.debug("ICAT mapping/translation found: %s", icat_mapping) - except KeyError as e: - raise FilterError(f"Bad PaNOSC to ICAT mapping: {e.args}") - - if isinstance(icat_mapping, str): - # Field name - icat_field_name = icat_mapping - elif isinstance(icat_mapping, dict): - # Relation - JSON format: {PaNOSC entity name: ICAT related field name} - panosc_entity_name = list(icat_mapping.keys())[0] - icat_field_name = icat_mapping[panosc_entity_name] - elif isinstance(icat_mapping, list): - # Edge case for ICAT's different parameter value field names - if isinstance(self.value, int) or isinstance(self.value, float): - icat_field_name = icat_mapping[0] - elif isinstance(self.value, datetime): - icat_field_name = icat_mapping[2] - elif isinstance(self.value, str): - if DateHandler.is_str_a_date(self.value): - icat_field_name = icat_mapping[2] - else: - icat_field_name = icat_mapping[1] - else: - self.value = str(self.value) - icat_field_name = icat_mapping[1] - - log.debug( - "Output of get_icat_mapping(): %s, %s", panosc_entity_name, icat_field_name, - ) - - return (panosc_entity_name, icat_field_name) - def __str__(self): """ String representation which is also used to apply WHERE filters that are inside diff --git a/datagateway_api/src/search_api/panosc_mappings.py b/datagateway_api/src/search_api/panosc_mappings.py index 60e6b5ba..2b812b84 100644 --- a/datagateway_api/src/search_api/panosc_mappings.py +++ b/datagateway_api/src/search_api/panosc_mappings.py @@ -2,7 +2,7 @@ import logging from pathlib import Path -from datagateway_api.src.common.exceptions import SearchAPIError +from datagateway_api.src.common.exceptions import FilterError, SearchAPIError log = logging.getLogger() @@ -19,6 +19,50 @@ def __init__( except IOError as e: raise SearchAPIError(e) + def get_icat_mapping(self, panosc_entity_name, field_name): + """ + This function searches the PaNOSC mappings and retrieves the ICAT translation + from the PaNOSC input. Fields in the same entity can be found, as well as fields + from related entities (e.g. Dataset.files.path) via recursion inside this + function. + + :param panosc_entity_name: A PaNOSC entity name e.g. "Dataset" + :type panosc_entity_name: :class:`str` + :param field_name: PaNOSC field name to fetch the ICAT version of e.g. "name" + :type field_name: :class:`str` + :return: Tuple containing the PaNOSC entity name (which will change from the + input if a related entity is found) and the ICAT field name + mapping/translation from the PaNOSC data model + :raises FilterError: If a valid mapping cannot be found + """ + + log.info( + "Searching mapping file to find ICAT translation for %s", + f"{panosc_entity_name}.{field_name}", + ) + + try: + 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): + # Field name + icat_field_name = icat_mapping + elif isinstance(icat_mapping, dict): + # Relation - JSON format: {PaNOSC entity name: ICAT related field name} + panosc_entity_name = list(icat_mapping.keys())[0] + icat_field_name = icat_mapping[panosc_entity_name] + elif isinstance(icat_mapping, list): + # If a list of ICAT field names are found, this is likely to be a specific + # need for that entity (e.g. parameter values). Dealing with this should be + # delegated to other code in this repo so the entire list is returned here + icat_field_name = icat_mapping + + return (panosc_entity_name, icat_field_name) + def get_panosc_related_entity_name( self, panosc_entity_name, panosc_related_field_name, ): From fc4556cf6a63715928c847b125e55c187818b281 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 6 Jan 2022 16:48:33 +0000 Subject: [PATCH 03/43] test: add tests for `SearchAPIIncludeFilter` #261 --- .../filters/test_search_api_include_filter.py | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 test/search_api/filters/test_search_api_include_filter.py diff --git a/test/search_api/filters/test_search_api_include_filter.py b/test/search_api/filters/test_search_api_include_filter.py new file mode 100644 index 00000000..9df4bc63 --- /dev/null +++ b/test/search_api/filters/test_search_api_include_filter.py @@ -0,0 +1,109 @@ +import pytest + +from datagateway_api.src.datagateway_api.filter_order_handler import FilterOrderHandler +from datagateway_api.src.search_api.filters import SearchAPIIncludeFilter +from datagateway_api.src.search_api.query import SearchAPIQuery + + +class TestSearchAPIIncludeFilter: + def test_valid_apply_filter_single(self): + filter_input = SearchAPIIncludeFilter("documents", "Dataset") + entity_name = "Dataset" + expected_query = "SELECT o FROM Dataset o INCLUDE o.investigation" + + filter_handler = FilterOrderHandler() + filter_handler.add_filter(filter_input) + test_query = SearchAPIQuery(entity_name) + + filter_handler.apply_filters(test_query) + + assert expected_query == str(test_query.icat_query.query) + + @pytest.mark.parametrize( + "test_include_filter, entity_name, expected_query", + [ + pytest.param( + SearchAPIIncludeFilter(["documents"], "Dataset"), + "Dataset", + "SELECT o FROM Dataset o INCLUDE o.investigation", + id="Dataset.documents", + ), + pytest.param( + SearchAPIIncludeFilter(["datasets"], "Document"), + "Document", + "SELECT o FROM Investigation o INCLUDE o.datasets", + id="Document.datasets", + ), + pytest.param( + SearchAPIIncludeFilter(["samples"], "Dataset"), + "Dataset", + "SELECT o FROM Dataset o INCLUDE o.sample", + id="Dataset.samples", + ), + pytest.param( + SearchAPIIncludeFilter(["members"], "Document"), + "Document", + "SELECT o FROM Investigation o INCLUDE o.investigationUsers", + id="Document.members", + ), + pytest.param( + SearchAPIIncludeFilter(["members.person"], "Document"), + "Document", + "SELECT o FROM Investigation o INCLUDE o.investigationUsers AS iu," + " iu.user", + id="Document.members.person", + ), + # TODO - Below test fails because dataset/investigation parameters issue on + # mapping + pytest.param( + SearchAPIIncludeFilter(["dataset.parameters.document"], "File"), + "File", + "SELECT o FROM Datafile o INCLUDE o.dataset AS d, d.parameters AS p," + " p.investigation", + id="File.dataset.parameters.document", + marks=pytest.mark.skip, + ), + pytest.param( + SearchAPIIncludeFilter(["dataset.samples.datasets"], "File"), + "File", + "SELECT o FROM Datafile o INCLUDE o.dataset AS ds, ds.sample AS s1," + " s1.datasets", + id="File.dataset.samples.datasets", + ), + pytest.param( + SearchAPIIncludeFilter(["dataset.samples.datasets.documents"], "File"), + "File", + "SELECT o FROM Datafile o INCLUDE o.dataset AS ds, ds.sample AS s1," + " s1.datasets AS s2, s2.investigation", + id="File.dataset.samples.datasets.documents", + ), + pytest.param( + SearchAPIIncludeFilter(["datasets", "members"], "Document"), + "Document", + "SELECT o FROM Investigation o INCLUDE o.datasets," + " o.investigationUsers", + id="Document.datasets & Document.members", + ), + pytest.param( + SearchAPIIncludeFilter(["dataset", "dataset.samples"], "File"), + "File", + "SELECT o FROM Datafile o INCLUDE o.dataset AS ds, ds.sample", + id="File.dataset & File.dataset.samples", + ), + pytest.param( + SearchAPIIncludeFilter(["documents", "files", "samples"], "Dataset"), + "Dataset", + "SELECT o FROM Dataset o INCLUDE o.datafiles, o.investigation," + " o.sample", + id="List of three included entities", + ), + ], + ) + def test_valid_apply_filter(self, test_include_filter, entity_name, expected_query): + filter_handler = FilterOrderHandler() + filter_handler.add_filter(test_include_filter) + test_query = SearchAPIQuery(entity_name) + + filter_handler.apply_filters(test_query) + + assert expected_query == str(test_query.icat_query.query) From 85c4bfc94cffc7c0c1664b78bb0824921aa286db Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 7 Jan 2022 15:48:07 +0000 Subject: [PATCH 04/43] conversion proof of concept #265 --- datagateway_api/src/search_api/models.py | 113 ++++++++++++++++++----- test/search_api/test_models.py | 28 ++++++ 2 files changed, 117 insertions(+), 24 deletions(-) create mode 100644 test/search_api/test_models.py diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 4c3fdc56..411f874a 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -1,5 +1,7 @@ -from abc import ABC, abstractclassmethod +import abc +from abc import ABC from datetime import datetime +import sys from typing import ClassVar, List, Optional, Union from pydantic import ( @@ -12,11 +14,74 @@ StrictStr, ) +from datagateway_api.src.search_api.panosc_mappings import mappings + + +# TODO - Merge this with `get_icat_mapping` from src\search_api\filters.py +def _get_icat_mapping(panosc_entity_name, field_name): + icat_mapping = mappings.mappings[panosc_entity_name][field_name] + + if isinstance(icat_mapping, str): + # Field name + icat_field_name = icat_mapping + if isinstance(icat_mapping, dict): + # Relation - JSON format: {PaNOSC entity name: ICAT related field name} + panosc_entity_name = list(icat_mapping.keys())[0] + icat_field_name = icat_mapping[panosc_entity_name] + + return panosc_entity_name, icat_field_name + + +def _get_icat_field_value(icat_field_name, icat_data): + icat_field_name = icat_field_name.split(".") + value = icat_data + for f in icat_field_name: + value = value[f] + + return value + class PaNOSCAttribute(ABC, BaseModel): - @abstractclassmethod - def from_icat(self): - pass + @classmethod + @abc.abstractmethod + def from_icat(cls, icat_data): # noqa: B902, N805 + model_fields = cls.__fields__ + + model_data = {} + for field in model_fields: + # Some fields have aliases so we must use them when creating a model instance. + # If a field does not have an alias then the `alias` property holds the name + # of the field + field_alias = cls.__fields__[field].alias + + panosc_entity_name, icat_field_name = _get_icat_mapping( + cls.__name__, field_alias, + ) + try: + field_value = _get_icat_field_value(icat_field_name, icat_data) + except KeyError: + continue + + if panosc_entity_name != cls.__name__: + # If we are here, it means that the field references another model so we + # have to get hold of its class definition and call its `from_icat` method + # to create an instance of itself with the ICAT data provided. Doing this + # allows for recursion. + data = icat_data[icat_field_name] + if not isinstance(data, list): + data = [data] + + # Get the class of the referenced model + panosc_model_attr = getattr(sys.modules[__name__], panosc_entity_name) + field_value = [panosc_model_attr.from_icat(d) for d in data] + + field_type = cls.__fields__[field].outer_type_._name + if field_type != "List": + field_value = field_value[0] + + model_data[field_alias] = field_value + + return cls(**model_data) class Affiliation(PaNOSCAttribute): @@ -33,8 +98,8 @@ class Affiliation(PaNOSCAttribute): members: Optional[List["Member"]] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Affiliation, cls).from_icat(icat_query_data) class Dataset(PaNOSCAttribute): @@ -59,8 +124,8 @@ class Dataset(PaNOSCAttribute): samples: Optional[List["Sample"]] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Dataset, cls).from_icat(icat_query_data) class Document(PaNOSCAttribute): @@ -87,8 +152,8 @@ class Document(PaNOSCAttribute): parameters: Optional[List["Parameter"]] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Document, cls).from_icat(icat_query_data) class File(PaNOSCAttribute): @@ -104,8 +169,8 @@ class File(PaNOSCAttribute): dataset: Dataset @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(File, cls).from_icat(icat_query_data) class Instrument(PaNOSCAttribute): @@ -120,8 +185,8 @@ class Instrument(PaNOSCAttribute): datasets: Optional[List[Dataset]] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Instrument, cls).from_icat(icat_query_data) class Member(PaNOSCAttribute): @@ -137,8 +202,8 @@ class Member(PaNOSCAttribute): affiliation: Optional[Affiliation] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Member, cls).from_icat(icat_query_data) class Parameter(PaNOSCAttribute): @@ -169,8 +234,8 @@ def validate_dataset_and_document(cls, values): # noqa: B902, N805 return values @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Parameter, cls).from_icat(icat_query_data) class Person(PaNOSCAttribute): @@ -188,8 +253,8 @@ class Person(PaNOSCAttribute): members: Optional[List[Member]] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Person, cls).from_icat(icat_query_data) class Sample(PaNOSCAttribute): @@ -204,8 +269,8 @@ class Sample(PaNOSCAttribute): datasets: Optional[List[Dataset]] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Sample, cls).from_icat(icat_query_data) class Technique(PaNOSCAttribute): @@ -219,8 +284,8 @@ class Technique(PaNOSCAttribute): datasets: Optional[List[Dataset]] @classmethod - def from_icat(cls): - pass + def from_icat(cls, icat_query_data): + return super(Technique, cls).from_icat(icat_query_data) # The below models reference other models that may not be defined during their diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py new file mode 100644 index 00000000..34d52590 --- /dev/null +++ b/test/search_api/test_models.py @@ -0,0 +1,28 @@ +import json + +import datagateway_api.src.search_api.models as models + + +class TestModels: + def test_from_icat_person_model(self): + expected_model_data = { + "id": "1", + "fullName": "Test fullname", + "orcid": "1111", + "researcherId": None, + "firstName": "Test given name", + "lastName": "Test family name", + "members": None, + } + # TODO: `id` is returned as `int` from ICAT whereas Person model expects `str` + icat_data = { + "id": "1", + "fullName": "Test fullname", + "orcidId": "1111", + "givenName": "Test given name", + "familyName": "Test family name", + } + + person_model = models.Person.from_icat(icat_data) + + assert person_model.json(by_alias=True) == json.dumps(expected_model_data) From eab7f3d9d10eb36a998653a492c99476fc7f46d9 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 10 Jan 2022 12:26:50 +0000 Subject: [PATCH 05/43] refactor: make query params more efficient to create less include filter objects for multiple and nested related entities #261 --- datagateway_api/src/search_api/filters.py | 4 --- .../src/search_api/panosc_mappings.py | 1 - .../src/search_api/query_filter_factory.py | 35 +++++++++++++------ .../test_search_api_query_filter_factory.py | 12 +++---- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index 793e9956..425d433f 100644 --- 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 index 2b812b84..e7500735 100644 --- 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 index 292ea9ff..e930475d 100644 --- 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 index 90708c7c..1df109da 100644 --- 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( From b0983bb0baa0bf0848d21d759fd3d5cb9dabddeb Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 13:15:07 +0000 Subject: [PATCH 06/43] refactor: address TODO in `models.py` #265 --- datagateway_api/src/search_api/models.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 411f874a..6e3f0a20 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -17,21 +17,6 @@ from datagateway_api.src.search_api.panosc_mappings import mappings -# TODO - Merge this with `get_icat_mapping` from src\search_api\filters.py -def _get_icat_mapping(panosc_entity_name, field_name): - icat_mapping = mappings.mappings[panosc_entity_name][field_name] - - if isinstance(icat_mapping, str): - # Field name - icat_field_name = icat_mapping - if isinstance(icat_mapping, dict): - # Relation - JSON format: {PaNOSC entity name: ICAT related field name} - panosc_entity_name = list(icat_mapping.keys())[0] - icat_field_name = icat_mapping[panosc_entity_name] - - return panosc_entity_name, icat_field_name - - def _get_icat_field_value(icat_field_name, icat_data): icat_field_name = icat_field_name.split(".") value = icat_data @@ -54,7 +39,7 @@ def from_icat(cls, icat_data): # noqa: B902, N805 # of the field field_alias = cls.__fields__[field].alias - panosc_entity_name, icat_field_name = _get_icat_mapping( + panosc_entity_name, icat_field_name = mappings.get_icat_mapping( cls.__name__, field_alias, ) try: From 3cbf11b5be6aeb371ba7210893db87ab7358dcdf Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 13:27:53 +0000 Subject: [PATCH 07/43] refactor: make field types non-strict #265 --- datagateway_api/src/search_api/models.py | 86 +++++++++++------------- 1 file changed, 41 insertions(+), 45 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 6e3f0a20..23d4a0a2 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -8,10 +8,6 @@ BaseModel, Field, root_validator, - StrictBool, - StrictFloat, - StrictInt, - StrictStr, ) from datagateway_api.src.search_api.panosc_mappings import mappings @@ -74,11 +70,11 @@ class Affiliation(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = [] - name: Optional[StrictStr] - id_: Optional[StrictStr] = Field(alias="id") - address: Optional[StrictStr] - city: Optional[StrictStr] - country: Optional[StrictStr] + name: Optional[str] + id_: Optional[str] = Field(alias="id") + address: Optional[str] + city: Optional[str] + country: Optional[str] members: Optional[List["Member"]] @@ -95,11 +91,11 @@ class Dataset(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = ["title"] - pid: StrictStr - title: StrictStr - is_public: StrictBool = Field(alias="isPublic") + pid: str + title: str + is_public: bool = Field(alias="isPublic") creation_date: datetime = Field(alias="creationDate") - size: Optional[StrictInt] + size: Optional[int] documents: List["Document"] techniques: List["Technique"] @@ -120,17 +116,17 @@ class Document(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = ["title", "summary"] - pid: StrictStr - is_public: StrictBool = Field(alias="isPublic") - type_: StrictStr = Field(alias="type") - title: StrictStr - summary: Optional[StrictStr] - doi: Optional[StrictStr] + pid: str + is_public: bool = Field(alias="isPublic") + type_: str = Field(alias="type") + title: str + summary: Optional[str] + doi: Optional[str] start_date: Optional[datetime] = Field(alias="startDate") end_date: Optional[datetime] = Field(alias="endDate") release_date: Optional[datetime] = Field(alias="releaseDate") - license_: Optional[StrictStr] = Field(alias="license") - keywords: Optional[List[StrictStr]] + license_: Optional[str] = Field(alias="license") + keywords: Optional[List[str]] datasets: List[Dataset] members: Optional[List["Member"]] @@ -146,10 +142,10 @@ class File(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = ["name"] - id_: StrictStr = Field(alias="id") - name: StrictStr - path: Optional[StrictStr] - size: Optional[StrictInt] + id_: str = Field(alias="id") + name: str + path: Optional[str] + size: Optional[int] dataset: Dataset @@ -163,9 +159,9 @@ class Instrument(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = ["name", "facility"] - pid: StrictStr - name: StrictStr - facility: StrictStr + pid: str + name: str + facility: str datasets: Optional[List[Dataset]] @@ -179,8 +175,8 @@ class Member(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = [] - id_: StrictStr = Field(alias="id") - role: Optional[StrictStr] = Field(alias="role") + id_: str = Field(alias="id") + role: Optional[str] = Field(alias="role") document: Document person: Optional["Person"] @@ -199,10 +195,10 @@ class Parameter(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = [] - id_: StrictStr = Field(alias="id") - name: StrictStr - value: Union[StrictFloat, StrictInt, StrictStr] - unit: Optional[StrictStr] + id_: str = Field(alias="id") + name: str + value: Union[float, int, str] + unit: Optional[str] dataset: Optional[Dataset] document: Optional[Document] @@ -228,12 +224,12 @@ class Person(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = [] - id_: StrictStr = Field(alias="id") - full_name: StrictStr = Field(alias="fullName") - orcid: Optional[StrictStr] - researcher_id: Optional[StrictStr] = Field(alias="researcherId") - first_name: Optional[StrictStr] = Field(alias="firstName") - last_name: Optional[StrictStr] = Field(alias="lastName") + id_: str = Field(alias="id") + full_name: str = Field(alias="fullName") + orcid: Optional[str] + researcher_id: Optional[str] = Field(alias="researcherId") + first_name: Optional[str] = Field(alias="firstName") + last_name: Optional[str] = Field(alias="lastName") members: Optional[List[Member]] @@ -247,9 +243,9 @@ class Sample(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = ["name", "description"] - name: StrictStr - pid: StrictStr - description: Optional[StrictStr] + name: str + pid: str + description: Optional[str] datasets: Optional[List[Dataset]] @@ -263,8 +259,8 @@ class Technique(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = ["name"] - pid: StrictStr - name: StrictStr + pid: str + name: str datasets: Optional[List[Dataset]] From c183a850376b213ecce6379a854b4168573e7f99 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 13:42:55 +0000 Subject: [PATCH 08/43] set non-related optional entity fields to default to `None` #265 --- datagateway_api/src/search_api/models.py | 42 ++++++++++++------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 23d4a0a2..fb87011f 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -70,11 +70,11 @@ class Affiliation(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = [] - name: Optional[str] - id_: Optional[str] = Field(alias="id") - address: Optional[str] - city: Optional[str] - country: Optional[str] + name: Optional[str] = None + id_: Optional[str] = Field(None, alias="id") + address: Optional[str] = None + city: Optional[str] = None + country: Optional[str] = None members: Optional[List["Member"]] @@ -95,7 +95,7 @@ class Dataset(PaNOSCAttribute): title: str is_public: bool = Field(alias="isPublic") creation_date: datetime = Field(alias="creationDate") - size: Optional[int] + size: Optional[int] = None documents: List["Document"] techniques: List["Technique"] @@ -120,12 +120,12 @@ class Document(PaNOSCAttribute): is_public: bool = Field(alias="isPublic") type_: str = Field(alias="type") title: str - summary: Optional[str] - doi: Optional[str] - start_date: Optional[datetime] = Field(alias="startDate") - end_date: Optional[datetime] = Field(alias="endDate") - release_date: Optional[datetime] = Field(alias="releaseDate") - license_: Optional[str] = Field(alias="license") + summary: Optional[str] = None + doi: Optional[str] = None + start_date: Optional[datetime] = Field(None, alias="startDate") + end_date: Optional[datetime] = Field(None, alias="endDate") + release_date: Optional[datetime] = Field(None, alias="releaseDate") + license_: Optional[str] = Field(None, alias="license") keywords: Optional[List[str]] datasets: List[Dataset] @@ -144,8 +144,8 @@ class File(PaNOSCAttribute): id_: str = Field(alias="id") name: str - path: Optional[str] - size: Optional[int] + path: Optional[str] = None + size: Optional[int] = None dataset: Dataset @@ -176,7 +176,7 @@ class Member(PaNOSCAttribute): _text_operator_fields: ClassVar[List[str]] = [] id_: str = Field(alias="id") - role: Optional[str] = Field(alias="role") + role: Optional[str] = Field(None, alias="role") document: Document person: Optional["Person"] @@ -198,7 +198,7 @@ class Parameter(PaNOSCAttribute): id_: str = Field(alias="id") name: str value: Union[float, int, str] - unit: Optional[str] + unit: Optional[str] = None dataset: Optional[Dataset] document: Optional[Document] @@ -226,10 +226,10 @@ class Person(PaNOSCAttribute): id_: str = Field(alias="id") full_name: str = Field(alias="fullName") - orcid: Optional[str] - researcher_id: Optional[str] = Field(alias="researcherId") - first_name: Optional[str] = Field(alias="firstName") - last_name: Optional[str] = Field(alias="lastName") + orcid: Optional[str] = None + researcher_id: Optional[str] = Field(None, alias="researcherId") + first_name: Optional[str] = Field(None, alias="firstName") + last_name: Optional[str] = Field(None, alias="lastName") members: Optional[List[Member]] @@ -245,7 +245,7 @@ class Sample(PaNOSCAttribute): name: str pid: str - description: Optional[str] + description: Optional[str] = None datasets: Optional[List[Dataset]] From ea29f47a214bd00f398ad3381ed2b54ed2e2b833 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 13:43:55 +0000 Subject: [PATCH 09/43] set signular related entity fields to default to `None` #265 --- datagateway_api/src/search_api/models.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index fb87011f..1fe4066e 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -99,7 +99,7 @@ class Dataset(PaNOSCAttribute): documents: List["Document"] techniques: List["Technique"] - instrument: Optional["Instrument"] + instrument: Optional["Instrument"] = None files: Optional[List["File"]] parameters: Optional[List["Parameter"]] samples: Optional[List["Sample"]] @@ -147,7 +147,7 @@ class File(PaNOSCAttribute): path: Optional[str] = None size: Optional[int] = None - dataset: Dataset + dataset: Dataset = None @classmethod def from_icat(cls, icat_query_data): @@ -178,9 +178,9 @@ class Member(PaNOSCAttribute): id_: str = Field(alias="id") role: Optional[str] = Field(None, alias="role") - document: Document - person: Optional["Person"] - affiliation: Optional[Affiliation] + document: Document = None + person: Optional["Person"] = None + affiliation: Optional[Affiliation] = None @classmethod def from_icat(cls, icat_query_data): @@ -200,8 +200,8 @@ class Parameter(PaNOSCAttribute): value: Union[float, int, str] unit: Optional[str] = None - dataset: Optional[Dataset] - document: Optional[Document] + dataset: Optional[Dataset] = None + document: Optional[Document] = None @root_validator(skip_on_failure=True) def validate_dataset_and_document(cls, values): # noqa: B902, N805 From ce9e2afe48b45cc4ed61d1e1ae4f4ed4bbef4e78 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 14:03:27 +0000 Subject: [PATCH 10/43] set plural entity fields to default to empty list #265 --- datagateway_api/src/search_api/models.py | 28 ++++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 1fe4066e..36603145 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -76,7 +76,7 @@ class Affiliation(PaNOSCAttribute): city: Optional[str] = None country: Optional[str] = None - members: Optional[List["Member"]] + members: Optional[List["Member"]] = [] @classmethod def from_icat(cls, icat_query_data): @@ -97,12 +97,12 @@ class Dataset(PaNOSCAttribute): creation_date: datetime = Field(alias="creationDate") size: Optional[int] = None - documents: List["Document"] - techniques: List["Technique"] + documents: List["Document"] = [] + techniques: List["Technique"] = [] instrument: Optional["Instrument"] = None - files: Optional[List["File"]] - parameters: Optional[List["Parameter"]] - samples: Optional[List["Sample"]] + files: Optional[List["File"]] = [] + parameters: Optional[List["Parameter"]] = [] + samples: Optional[List["Sample"]] = [] @classmethod def from_icat(cls, icat_query_data): @@ -126,11 +126,11 @@ class Document(PaNOSCAttribute): end_date: Optional[datetime] = Field(None, alias="endDate") release_date: Optional[datetime] = Field(None, alias="releaseDate") license_: Optional[str] = Field(None, alias="license") - keywords: Optional[List[str]] + keywords: Optional[List[str]] = [] - datasets: List[Dataset] - members: Optional[List["Member"]] - parameters: Optional[List["Parameter"]] + datasets: List[Dataset] = [] + members: Optional[List["Member"]] = [] + parameters: Optional[List["Parameter"]] = [] @classmethod def from_icat(cls, icat_query_data): @@ -163,7 +163,7 @@ class Instrument(PaNOSCAttribute): name: str facility: str - datasets: Optional[List[Dataset]] + datasets: Optional[List[Dataset]] = [] @classmethod def from_icat(cls, icat_query_data): @@ -231,7 +231,7 @@ class Person(PaNOSCAttribute): first_name: Optional[str] = Field(None, alias="firstName") last_name: Optional[str] = Field(None, alias="lastName") - members: Optional[List[Member]] + members: Optional[List[Member]] = [] @classmethod def from_icat(cls, icat_query_data): @@ -247,7 +247,7 @@ class Sample(PaNOSCAttribute): pid: str description: Optional[str] = None - datasets: Optional[List[Dataset]] + datasets: Optional[List[Dataset]] = [] @classmethod def from_icat(cls, icat_query_data): @@ -262,7 +262,7 @@ class Technique(PaNOSCAttribute): pid: str name: str - datasets: Optional[List[Dataset]] + datasets: Optional[List[Dataset]] = [] @classmethod def from_icat(cls, icat_query_data): From 956a9023fd56678e5e461a70208898ce566f4138 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 14:15:29 +0000 Subject: [PATCH 11/43] refactor: deal with case when list of ICAT field names are found #265 --- datagateway_api/src/search_api/models.py | 31 ++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 36603145..b74adf2d 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -38,9 +38,21 @@ def from_icat(cls, icat_data): # noqa: B902, N805 panosc_entity_name, icat_field_name = mappings.get_icat_mapping( cls.__name__, field_alias, ) - try: - field_value = _get_icat_field_value(icat_field_name, icat_data) - except KeyError: + + if not isinstance(icat_field_name, list): + icat_field_name = [icat_field_name] + + field_value = None + for field_name in icat_field_name: + try: + value = _get_icat_field_value(field_name, icat_data) + if value: + field_value = value + break + except KeyError: + continue + + if not field_value: continue if panosc_entity_name != cls.__name__: @@ -48,7 +60,7 @@ def from_icat(cls, icat_data): # noqa: B902, N805 # have to get hold of its class definition and call its `from_icat` method # to create an instance of itself with the ICAT data provided. Doing this # allows for recursion. - data = icat_data[icat_field_name] + data = field_value if not isinstance(data, list): data = [data] @@ -56,9 +68,14 @@ def from_icat(cls, icat_data): # noqa: B902, N805 panosc_model_attr = getattr(sys.modules[__name__], panosc_entity_name) field_value = [panosc_model_attr.from_icat(d) for d in data] - field_type = cls.__fields__[field].outer_type_._name - if field_type != "List": - field_value = field_value[0] + field_outer_type = cls.__fields__[field].outer_type_ + if ( + not hasattr(field_outer_type, "_name") + or field_outer_type._name != "List" + ) and isinstance(field_value, list): + # If the field does not hold list of values but `field_value` + # is a list, then just get its first element + field_value = field_value[0] model_data[field_alias] = field_value From 98f9157938abc829493569e32672640b30645304 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 14:34:51 +0000 Subject: [PATCH 12/43] refactor: deal with case when list of ICAT field values are found #265 --- datagateway_api/src/search_api/models.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index b74adf2d..fe147bde 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -15,11 +15,22 @@ def _get_icat_field_value(icat_field_name, icat_data): icat_field_name = icat_field_name.split(".") - value = icat_data - for f in icat_field_name: - value = value[f] - - return value + for field_name in icat_field_name: + if isinstance(icat_data, list): + values = [] + for data in icat_data: + value = _get_icat_field_value(field_name, data) + if isinstance(value, list): + values.extend(value) + else: + values.append(value) + + icat_data = values + + if isinstance(icat_data, dict): + icat_data = icat_data[field_name] + + return icat_data class PaNOSCAttribute(ABC, BaseModel): From 3ceb36549daec7b48bde1084114fd8e395f96263 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 14:49:27 +0000 Subject: [PATCH 13/43] feature: deal with required related entity cases #265 --- datagateway_api/src/search_api/models.py | 78 +++++++++++++++++------- 1 file changed, 56 insertions(+), 22 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index fe147bde..66f76fa0 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -8,7 +8,9 @@ BaseModel, Field, root_validator, + ValidationError, ) +from pydantic.error_wrappers import ErrorWrapper from datagateway_api.src.search_api.panosc_mappings import mappings @@ -36,7 +38,7 @@ def _get_icat_field_value(icat_field_name, icat_data): class PaNOSCAttribute(ABC, BaseModel): @classmethod @abc.abstractmethod - def from_icat(cls, icat_data): # noqa: B902, N805 + def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 model_fields = cls.__fields__ model_data = {} @@ -77,7 +79,10 @@ def from_icat(cls, icat_data): # noqa: B902, N805 # Get the class of the referenced model panosc_model_attr = getattr(sys.modules[__name__], panosc_entity_name) - field_value = [panosc_model_attr.from_icat(d) for d in data] + field_value = [ + panosc_model_attr.from_icat(d, required_related_fields) + for d in data + ] field_outer_type = cls.__fields__[field].outer_type_ if ( @@ -90,12 +95,29 @@ def from_icat(cls, icat_data): # noqa: B902, N805 model_data[field_alias] = field_value + for required_related_field in required_related_fields: + if ( + required_related_field in model_fields + and required_related_field + in cls._related_fields_with_min_cardinality_one + and required_related_field not in model_data + ): + # If we are here, it means that a related entity, which has a minimum + # cardinality of one, has been specified to be included as part of the entity + # but the relevant ICAT data needed for its creation cannot be found in the + # provided ICAT response. Because of this, a ValidationError is raised. + error_wrapper = ErrorWrapper( + TypeError("field required"), loc=required_related_field, + ) + raise ValidationError(errors=[error_wrapper], model=cls) + return cls(**model_data) class Affiliation(PaNOSCAttribute): """Information about which facility a member is located at""" + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = ["members"] _text_operator_fields: ClassVar[List[str]] = [] name: Optional[str] = None @@ -107,8 +129,8 @@ class Affiliation(PaNOSCAttribute): members: Optional[List["Member"]] = [] @classmethod - def from_icat(cls, icat_query_data): - return super(Affiliation, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Affiliation, cls).from_icat(icat_data, required_related_fields) class Dataset(PaNOSCAttribute): @@ -117,6 +139,10 @@ class Dataset(PaNOSCAttribute): and Technique """ + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = [ + "documents", + "techniques", + ] _text_operator_fields: ClassVar[List[str]] = ["title"] pid: str @@ -133,8 +159,8 @@ class Dataset(PaNOSCAttribute): samples: Optional[List["Sample"]] = [] @classmethod - def from_icat(cls, icat_query_data): - return super(Dataset, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Dataset, cls).from_icat(icat_data, required_related_fields) class Document(PaNOSCAttribute): @@ -142,6 +168,7 @@ class Document(PaNOSCAttribute): Proposal which includes the dataset or published paper which references the dataset """ + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = ["datasets"] _text_operator_fields: ClassVar[List[str]] = ["title", "summary"] pid: str @@ -161,13 +188,14 @@ class Document(PaNOSCAttribute): parameters: Optional[List["Parameter"]] = [] @classmethod - def from_icat(cls, icat_query_data): - return super(Document, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Document, cls).from_icat(icat_data, required_related_fields) class File(PaNOSCAttribute): """Name of file and optionally location""" + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = ["dataset"] _text_operator_fields: ClassVar[List[str]] = ["name"] id_: str = Field(alias="id") @@ -178,13 +206,14 @@ class File(PaNOSCAttribute): dataset: Dataset = None @classmethod - def from_icat(cls, icat_query_data): - return super(File, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(File, cls).from_icat(icat_data, required_related_fields) class Instrument(PaNOSCAttribute): """Beam line where experiment took place""" + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = [] _text_operator_fields: ClassVar[List[str]] = ["name", "facility"] pid: str @@ -194,13 +223,14 @@ class Instrument(PaNOSCAttribute): datasets: Optional[List[Dataset]] = [] @classmethod - def from_icat(cls, icat_query_data): - return super(Instrument, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Instrument, cls).from_icat(icat_data, required_related_fields) class Member(PaNOSCAttribute): """Proposal team member or paper co-author""" + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = ["document"] _text_operator_fields: ClassVar[List[str]] = [] id_: str = Field(alias="id") @@ -211,8 +241,8 @@ class Member(PaNOSCAttribute): affiliation: Optional[Affiliation] = None @classmethod - def from_icat(cls, icat_query_data): - return super(Member, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Member, cls).from_icat(icat_data, required_related_fields) class Parameter(PaNOSCAttribute): @@ -221,6 +251,7 @@ class Parameter(PaNOSCAttribute): Note: a parameter is either related to a dataset or a document, but not both. """ + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = [] _text_operator_fields: ClassVar[List[str]] = [] id_: str = Field(alias="id") @@ -243,13 +274,14 @@ def validate_dataset_and_document(cls, values): # noqa: B902, N805 return values @classmethod - def from_icat(cls, icat_query_data): - return super(Parameter, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Parameter, cls).from_icat(icat_data, required_related_fields) class Person(PaNOSCAttribute): """Human who carried out experiment""" + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = [] _text_operator_fields: ClassVar[List[str]] = [] id_: str = Field(alias="id") @@ -262,13 +294,14 @@ class Person(PaNOSCAttribute): members: Optional[List[Member]] = [] @classmethod - def from_icat(cls, icat_query_data): - return super(Person, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Person, cls).from_icat(icat_data, required_related_fields) class Sample(PaNOSCAttribute): """Extract of material used in the experiment""" + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = [] _text_operator_fields: ClassVar[List[str]] = ["name", "description"] name: str @@ -278,13 +311,14 @@ class Sample(PaNOSCAttribute): datasets: Optional[List[Dataset]] = [] @classmethod - def from_icat(cls, icat_query_data): - return super(Sample, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Sample, cls).from_icat(icat_data, required_related_fields) class Technique(PaNOSCAttribute): """Common name of scientific method used""" + _related_fields_with_min_cardinality_one: ClassVar[List[str]] = [] _text_operator_fields: ClassVar[List[str]] = ["name"] pid: str @@ -293,8 +327,8 @@ class Technique(PaNOSCAttribute): datasets: Optional[List[Dataset]] = [] @classmethod - def from_icat(cls, icat_query_data): - return super(Technique, cls).from_icat(icat_query_data) + def from_icat(cls, icat_data, required_related_fields): + return super(Technique, cls).from_icat(icat_data, required_related_fields) # The below models reference other models that may not be defined during their From 94dcfbaed9ea5206979bdc51db466ee2a88790cb Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 14:55:04 +0000 Subject: [PATCH 14/43] disable dataset and document validator in Parameter entity #265 --- datagateway_api/src/search_api/models.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 66f76fa0..701f1ceb 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -7,7 +7,6 @@ from pydantic import ( BaseModel, Field, - root_validator, ValidationError, ) from pydantic.error_wrappers import ErrorWrapper @@ -262,16 +261,16 @@ class Parameter(PaNOSCAttribute): dataset: Optional[Dataset] = None document: Optional[Document] = None - @root_validator(skip_on_failure=True) - def validate_dataset_and_document(cls, values): # noqa: B902, N805 - if values["dataset"] is None and values["document"] is None: - raise TypeError("must have a dataset or document") + # @root_validator(skip_on_failure=True) + # def validate_dataset_and_document(cls, values): # noqa: B902, N805 + # if values["dataset"] is None and values["document"] is None: + # raise TypeError("must have a dataset or document") - if values["dataset"] is not None and values["document"] is not None: - # TODO - Should an exception be raised here instead? - values["Document"] = None + # if values["dataset"] is not None and values["document"] is not None: + # # TODO - Should an exception be raised here instead? + # values["Document"] = None - return values + # return values @classmethod def from_icat(cls, icat_data, required_related_fields): From eca0a46ec6c649cb0786bad7b145efd7f9910775 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 16:53:15 +0000 Subject: [PATCH 15/43] work out if dataset or document is public #265 --- datagateway_api/src/search_api/models.py | 29 +++++++++++++++++++----- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 701f1ceb..200c2460 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -1,14 +1,11 @@ import abc from abc import ABC -from datetime import datetime +from datetime import datetime, timezone import sys from typing import ClassVar, List, Optional, Union -from pydantic import ( - BaseModel, - Field, - ValidationError, -) +from dateutil.relativedelta import relativedelta +from pydantic import BaseModel, Field, ValidationError, validator from pydantic.error_wrappers import ErrorWrapper from datagateway_api.src.search_api.panosc_mappings import mappings @@ -157,6 +154,16 @@ class Dataset(PaNOSCAttribute): parameters: Optional[List["Parameter"]] = [] samples: Optional[List["Sample"]] = [] + @validator("is_public", pre=True, always=True) + def set_is_public(cls, value): # noqa: B902, N805 + if not value: + return value + + creation_date = datetime.fromisoformat(value) + current_datetime = datetime.now(timezone.utc) + three_years_ago = current_datetime - relativedelta(years=3) + return creation_date < three_years_ago + @classmethod def from_icat(cls, icat_data, required_related_fields): return super(Dataset, cls).from_icat(icat_data, required_related_fields) @@ -186,6 +193,16 @@ class Document(PaNOSCAttribute): members: Optional[List["Member"]] = [] parameters: Optional[List["Parameter"]] = [] + @validator("is_public", pre=True, always=True) + def set_is_public(cls, value): # noqa: B902, N805 + if not value: + return value + + creation_date = datetime.fromisoformat(value) + current_datetime = datetime.now(timezone.utc) + three_years_ago = current_datetime - relativedelta(years=3) + return creation_date < three_years_ago + @classmethod def from_icat(cls, icat_data, required_related_fields): return super(Document, cls).from_icat(icat_data, required_related_fields) From 66f026129955fb12e979c145563db5b4995843a1 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:16:03 +0000 Subject: [PATCH 16/43] test: define test data #265 --- test/search_api/test_models.py | 278 +++++++++++++++++++++++++++++++++ 1 file changed, 278 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 34d52590..44922d22 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -1,8 +1,286 @@ +from datetime import datetime, timezone import json import datagateway_api.src.search_api.models as models +AFFILIATION_ICAT_DATA = { + "id": 1, + "name": "Test name", + "fullReference": "Test fullReference", +} + +DATASET_ICAT_DATA = { + "endDate": "2000-12-31 00:00:00+00:00", + "complete": True, + "name": "Test name", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", + "location": "Test location", + "modId": "Test modId", + "description": "Test description", + "createId": "Test createId", + "createTime": "2000-12-31 00:00:00+00:00", + "doi": "Test doi", + "startDate": "2000-12-31 00:00:00+00:00", +} + +DATASET_PARAMETER_ICAT_DATA = { + "error": 1.0, + "stringValue": "Test stringValue", + "id": 1, + "numericValue": None, + "modTime": "2000-12-31 00:00:00+00:00", + "rangeTop": 1.0, + "modId": "Test modId", + "createId": "Test createId", + "createTime": "2000-12-31 00:00:00+00:00", + "rangeBottom": 1.0, + "dateTimeValue": None, +} + +DATAFILE_ICAT_DATA = { + "name": "Test name", + "id": 1, + "datafileModTime": "2000-12-31 00:00:00+00:00", + "modTime": "2000-12-31 00:00:00+00:00", + "fileSize": 1234, + "location": "Test location", + "modId": "Test modId", + "description": "Test description", + "createId": "Test createId", + "createTime": "2000-12-31 00:00:00+00:00", + "doi": "Test doi", + "datafileCreateTime": "2000-12-31 00:00:00+00:00", + "checksum": "Test checksum", +} + +FACILITY_ICAT_DATA = { + "daysUntilRelease": 1, + "name": "Test name", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", + "url": None, + "fullName": None, + "modId": "Test modId", + "description": "Test description", + "createId": "Test createId", + "createTime": "2000-12-31 00:00:00+00:00", +} + +INSTRUMENT_ICAT_DATA = { + "type": "Test type", + "pid": None, + "name": "Test name", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", + "url": "Test url", + "fullName": "Test fullName", + "modId": "Test modId", + "description": "Test description", + "createId": "Test createId", + "createTime": "2000-12-31 00:00:00+00:00", +} + +INVESTIGATION_ICAT_DATA = { + "endDate": "2000-12-31 00:00:00+00:00", + "name": "Test name", + "releaseDate": str(datetime.now(timezone.utc)), + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", + "modId": "Test modId", + "createId": "Test createId", + "summary": "Test summary", + "visitId": "Test visitId", + "createTime": "2000-12-31 00:00:00+00:00", + "doi": "Test doi", + "startDate": "2000-12-31 00:00:00+00:00", + "title": "Test title", +} + +INVESTIGATION_PARAMETER_ICAT_DATA = DATASET_PARAMETER_ICAT_DATA.copy() +INVESTIGATION_PARAMETER_ICAT_DATA["stringValue"] = None +INVESTIGATION_PARAMETER_ICAT_DATA["dateTimeValue"] = "2000-12-31 00:00:00+00:00" + +INVESTIGATION_TYPE_ICAT_DATA = { + "modId": "Test modId", + "description": "Test description", + "createId": "Test createId", + "name": "Test name", + "createTime": "2000-12-31 00:00:00+00:00", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", +} + +INVESTIGATION_USER_ICAT_DATA = { + "id": 1, + "modId": "Test modId", + "createId": "Test createId", + "createTime": "2000-12-31 00:00:00+00:00", + "role": "Test role", + "modTime": "2000-12-31 00:00:00+00:00", +} + +KEYWORD_ICAT_DATA = { + "modId": "Test modId", + "createId": "Test createId", + "name": "Test name", + "createTime": "2000-12-31 00:00:00+00:00", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", +} + +PARAMETER_TYPE_ICAT_DATA = { + "pid": None, + "verified": True, + "unitsFullName": "Test unitsFullName", + "enforced": False, + "maximumNumericValue": 1.0, + "name": "Test name", + "modId": "Test modId", + "minimumNumericValue": 1.0, + "createId": "Test createId", + "applicableToDataCollection": False, + "applicableToInvestigation": False, + "applicableToDatafile": True, + "units": "Test units", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", + "applicableToDataset": True, + "description": "Test description", + "valueType": "Test valueType", + "createTime": "2000-12-31 00:00:00+00:00", + "applicableToSample": False, +} + +SAMPLE_ICAT_DATA = { + "pid": "None", + "modId": "Test modId", + "createId": "Test createId", + "name": "Test name", + "createTime": "2000-12-31 00:00:00+00:00", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", +} + +TECHNIQUE_ICAT_DATA = { + "name": "Test name", + "pid": "Test pid", + "description": "Test description", +} + +USER_ICAT_DATA = { + "email": "Test email", + "name": "Test name", + "id": 1, + "modTime": "2000-12-31 00:00:00+00:00", + "familyName": None, + "modId": "Test modId", + "fullName": "Test fullName", + "orcidId": "Test orcidId", + "createId": "Test createId", + "createTime": "2000-12-31 00:00:00+00:00", + "affiliation": None, + "givenName": None, +} + + +AFFILIATION_PANOSC_DATA = { + "name": AFFILIATION_ICAT_DATA["name"], + "id": str(AFFILIATION_ICAT_DATA["id"]), + "address": AFFILIATION_ICAT_DATA["fullReference"], + "city": None, + "country": None, + "members": [], +} + +DATASET_PANOSC_DATA = { + "pid": DATASET_ICAT_DATA["doi"], + "title": DATASET_ICAT_DATA["name"], + "creationDate": datetime.fromisoformat(DATASET_ICAT_DATA["createTime"]), + "isPublic": True, + "size": None, + "documents": [], + "techniques": [], + "instrument": None, + "files": [], + "parameters": [], + "samples": [], +} + +DOCUMENT_PANOSC_DATA = { + "pid": INVESTIGATION_ICAT_DATA["doi"], + "isPublic": False, + "type": INVESTIGATION_TYPE_ICAT_DATA["name"], + "title": INVESTIGATION_ICAT_DATA["name"], + "summary": INVESTIGATION_ICAT_DATA["summary"], + "doi": INVESTIGATION_ICAT_DATA["doi"], + "startDate": datetime.fromisoformat(INVESTIGATION_ICAT_DATA["startDate"]), + "endDate": datetime.fromisoformat(INVESTIGATION_ICAT_DATA["endDate"]), + "releaseDate": datetime.fromisoformat(INVESTIGATION_ICAT_DATA["releaseDate"]), + "license": None, + "keywords": [KEYWORD_ICAT_DATA["name"]], + "datasets": [], + "members": [], + "parameters": [], +} + +FILE_PANOSC_DATA = { + "id": str(DATAFILE_ICAT_DATA["id"]), + "name": DATAFILE_ICAT_DATA["name"], + "path": DATAFILE_ICAT_DATA["location"], + "size": DATAFILE_ICAT_DATA["fileSize"], + "dataset": None, +} + +INSTRUMENT_PANOSC_DATA = { + "pid": str(INSTRUMENT_ICAT_DATA["id"]), + "name": INSTRUMENT_ICAT_DATA["name"], + "facility": FACILITY_ICAT_DATA["name"], + "datasets": [], +} + +MEMBER_PANOSC_DATA = { + "id": str(INVESTIGATION_USER_ICAT_DATA["id"]), + "role": INVESTIGATION_USER_ICAT_DATA["role"], + "document": None, + "person": None, + "affiliation": None, +} + +PARAMETER_PANOSC_DATA = { + "id": str(INVESTIGATION_PARAMETER_ICAT_DATA["id"]), + "name": PARAMETER_TYPE_ICAT_DATA["name"], + "value": INVESTIGATION_PARAMETER_ICAT_DATA["dateTimeValue"], + "unit": PARAMETER_TYPE_ICAT_DATA["units"], + "dataset": None, + "document": None, +} + +PERSON_PANOSC_DATA = { + "id": str(USER_ICAT_DATA["id"]), + "fullName": USER_ICAT_DATA["fullName"], + "orcid": USER_ICAT_DATA["orcidId"], + "researcherId": None, + "firstName": USER_ICAT_DATA["givenName"], + "lastName": USER_ICAT_DATA["familyName"], + "members": [], +} + +SAMPLE_PANOSC_DATA = { + "name": SAMPLE_ICAT_DATA["name"], + "pid": SAMPLE_ICAT_DATA["pid"], + "description": PARAMETER_TYPE_ICAT_DATA["description"], + "datasets": [], +} + +TECHNIQUE_PANOSC_DATA = { + "pid": TECHNIQUE_ICAT_DATA["pid"], + "name": TECHNIQUE_ICAT_DATA["name"], + "datasets": [], +} + + class TestModels: def test_from_icat_person_model(self): expected_model_data = { From 07058a047ee1dcd48d85c1369393361684aea9ad Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:22:40 +0000 Subject: [PATCH 17/43] test: unit test `from_icat` `Affiliation` entity creation #265 --- test/search_api/test_models.py | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 44922d22..e0376eb0 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -1,5 +1,4 @@ from datetime import datetime, timezone -import json import datagateway_api.src.search_api.models as models @@ -282,25 +281,20 @@ class TestModels: - def test_from_icat_person_model(self): - expected_model_data = { - "id": "1", - "fullName": "Test fullname", - "orcid": "1111", - "researcherId": None, - "firstName": "Test given name", - "lastName": "Test family name", - "members": None, - } - # TODO: `id` is returned as `int` from ICAT whereas Person model expects `str` - icat_data = { - "id": "1", - "fullName": "Test fullname", - "orcidId": "1111", - "givenName": "Test given name", - "familyName": "Test family name", + def test_from_icat_affiliation_entity_without_data_for_related_entities(self): + affiliation_entity = models.Affiliation.from_icat(AFFILIATION_ICAT_DATA, []) + + assert affiliation_entity.dict(by_alias=True) == AFFILIATION_PANOSC_DATA + + def test_from_icat_affiliation_entity_with_data_for_all_related_entities(self): + expected_entity_data = AFFILIATION_PANOSC_DATA.copy() + expected_entity_data["members"] = [MEMBER_PANOSC_DATA] + + icat_data = AFFILIATION_ICAT_DATA.copy() + icat_data["user"] = { + "user": {"investigationUsers": [INVESTIGATION_USER_ICAT_DATA]}, } - person_model = models.Person.from_icat(icat_data) + affiliation_entity = models.Affiliation.from_icat(icat_data, ["members"]) - assert person_model.json(by_alias=True) == json.dumps(expected_model_data) + assert affiliation_entity.dict(by_alias=True) == expected_entity_data From 3631c616e44ffaf4a177f3ba3fec00a1965217f4 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:23:14 +0000 Subject: [PATCH 18/43] test: unit test `from_icat` `Dataset` entity creation #265 --- test/search_api/test_models.py | 71 ++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index e0376eb0..36d2e109 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -298,3 +298,74 @@ def test_from_icat_affiliation_entity_with_data_for_all_related_entities(self): affiliation_entity = models.Affiliation.from_icat(icat_data, ["members"]) assert affiliation_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_dataset_entity_without_data_for_related_entities(self): + dataset_entity = models.Dataset.from_icat(DATASET_ICAT_DATA, []) + + assert dataset_entity.dict(by_alias=True) == DATASET_PANOSC_DATA + + def test_from_icat_dataset_entity_with_data_for_mandatory_related_entities(self): + expected_entity_data = DATASET_PANOSC_DATA.copy() + expected_entity_data["documents"] = [DOCUMENT_PANOSC_DATA] + expected_entity_data["techniques"] = [ + TECHNIQUE_PANOSC_DATA, + TECHNIQUE_PANOSC_DATA, + ] + + icat_data = DATASET_ICAT_DATA.copy() + icat_data["investigation"] = INVESTIGATION_ICAT_DATA.copy() + icat_data["investigation"]["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["investigation"]["keywords"] = [KEYWORD_ICAT_DATA] + icat_data["datasetTechniques"] = [ + {"technique": TECHNIQUE_ICAT_DATA}, + {"technique": TECHNIQUE_ICAT_DATA}, + ] + + dataset_entity = models.Dataset.from_icat( + icat_data, ["documents", "techniques"], + ) + + assert dataset_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_dataset_entity_with_data_for_all_related_entities(self): + expected_entity_data = DATASET_PANOSC_DATA.copy() + expected_entity_data["documents"] = [DOCUMENT_PANOSC_DATA] + expected_entity_data["techniques"] = [TECHNIQUE_PANOSC_DATA] + expected_entity_data["instrument"] = INSTRUMENT_PANOSC_DATA + expected_entity_data["files"] = [FILE_PANOSC_DATA, FILE_PANOSC_DATA] + expected_entity_data["parameters"] = [PARAMETER_PANOSC_DATA.copy()] + expected_entity_data["parameters"][0]["value"] = DATASET_PARAMETER_ICAT_DATA[ + "stringValue" + ] + expected_entity_data["samples"] = [SAMPLE_PANOSC_DATA] + + icat_data = DATASET_ICAT_DATA.copy() + icat_data["investigation"] = INVESTIGATION_ICAT_DATA.copy() + icat_data["investigation"]["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["investigation"]["keywords"] = [KEYWORD_ICAT_DATA] + icat_data["datasetTechniques"] = [{"technique": TECHNIQUE_ICAT_DATA}] + icat_data["datasetInstruments"] = [ + {"instrument": INSTRUMENT_ICAT_DATA.copy()}, + {"instrument": INSTRUMENT_ICAT_DATA.copy()}, + ] + icat_data["datasetInstruments"][0]["instrument"][ + "facility" + ] = FACILITY_ICAT_DATA + icat_data["datasetInstruments"][1]["instrument"][ + "facility" + ] = FACILITY_ICAT_DATA + icat_data["datafiles"] = [DATAFILE_ICAT_DATA, DATAFILE_ICAT_DATA] + icat_data["parameters"] = [DATASET_PARAMETER_ICAT_DATA.copy()] + icat_data["parameters"][0]["type"] = PARAMETER_TYPE_ICAT_DATA + icat_data["sample"] = SAMPLE_ICAT_DATA.copy() + icat_data["sample"]["parameters"] = [ + {"type": PARAMETER_TYPE_ICAT_DATA}, + {"type": PARAMETER_TYPE_ICAT_DATA}, + ] + + dataset_entity = models.Dataset.from_icat( + icat_data, + ["documents", "techniques", "instrument", "files", "parameters", "samples"], + ) + + assert dataset_entity.dict(by_alias=True) == expected_entity_data From 8f2a6c1a5104a38132f173dd131a0a01dec63a26 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:25:39 +0000 Subject: [PATCH 19/43] test: unit test `from_icat` `Dataset` entity creation #265 --- test/search_api/test_models.py | 40 ++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 36d2e109..b49e1590 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -369,3 +369,43 @@ def test_from_icat_dataset_entity_with_data_for_all_related_entities(self): ) assert dataset_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_document_entity_without_data_for_related_entities(self): + icat_data = INVESTIGATION_ICAT_DATA.copy() + icat_data["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["keywords"] = [KEYWORD_ICAT_DATA] + + document_entity = models.Document.from_icat(icat_data, []) + + assert document_entity.dict(by_alias=True) == DOCUMENT_PANOSC_DATA + + def test_from_icat_document_entity_with_data_for_mandatory_related_entities(self): + expected_entity_data = DOCUMENT_PANOSC_DATA.copy() + expected_entity_data["datasets"] = [DATASET_PANOSC_DATA, DATASET_PANOSC_DATA] + + icat_data = INVESTIGATION_ICAT_DATA.copy() + icat_data["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["keywords"] = [KEYWORD_ICAT_DATA] + icat_data["datasets"] = [DATASET_ICAT_DATA, DATASET_ICAT_DATA] + + document_entity = models.Document.from_icat(icat_data, ["datasets"]) + + assert document_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_document_entity_with_data_for_all_related_entities(self): + expected_entity_data = DOCUMENT_PANOSC_DATA.copy() + expected_entity_data["datasets"] = [DATASET_PANOSC_DATA, DATASET_PANOSC_DATA] + expected_entity_data["members"] = [MEMBER_PANOSC_DATA] + expected_entity_data["parameters"] = [PARAMETER_PANOSC_DATA] + + icat_data = INVESTIGATION_ICAT_DATA.copy() + icat_data["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["keywords"] = [KEYWORD_ICAT_DATA] + icat_data["datasets"] = [DATASET_ICAT_DATA, DATASET_ICAT_DATA] + icat_data["investigationUsers"] = [INVESTIGATION_USER_ICAT_DATA] + icat_data["parameters"] = [INVESTIGATION_PARAMETER_ICAT_DATA.copy()] + icat_data["parameters"][0]["type"] = PARAMETER_TYPE_ICAT_DATA + + document_entity = models.Document.from_icat(icat_data, ["datasets"]) + + assert document_entity.dict(by_alias=True) == expected_entity_data From 0554bea8082735780a1c8edef24ebf171df04bea Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:27:14 +0000 Subject: [PATCH 20/43] test: unit test `from_icat` `File` entity creation #265 --- test/search_api/test_models.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index b49e1590..d456c9f1 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -409,3 +409,19 @@ def test_from_icat_document_entity_with_data_for_all_related_entities(self): document_entity = models.Document.from_icat(icat_data, ["datasets"]) assert document_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_file_entity_without_data_for_related_entities(self): + file_entity = models.File.from_icat(DATAFILE_ICAT_DATA, []) + + assert file_entity.dict(by_alias=True) == FILE_PANOSC_DATA + + def test_from_icat_file_entity_with_data_for_all_related_entities(self): + expected_entity_data = FILE_PANOSC_DATA.copy() + expected_entity_data["dataset"] = DATASET_PANOSC_DATA + + icat_data = DATAFILE_ICAT_DATA.copy() + icat_data["dataset"] = DATASET_ICAT_DATA + + file_entity = models.File.from_icat(icat_data, []) + + assert file_entity.dict(by_alias=True) == expected_entity_data From 0d0a1e4875952175ad63dc9882058051f4dcf2d2 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:29:01 +0000 Subject: [PATCH 21/43] test: unit test `from_icat` `Instrument` entity creation #265 --- test/search_api/test_models.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index d456c9f1..01fbe3e8 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -425,3 +425,23 @@ def test_from_icat_file_entity_with_data_for_all_related_entities(self): file_entity = models.File.from_icat(icat_data, []) assert file_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_instrument_entity_without_data_for_related_entities(self): + icat_data = INSTRUMENT_ICAT_DATA.copy() + icat_data["facility"] = FACILITY_ICAT_DATA + + instrument_entity = models.Instrument.from_icat(icat_data, []) + + assert instrument_entity.dict(by_alias=True) == INSTRUMENT_PANOSC_DATA + + def test_from_icat_instrument_entity_with_data_for_all_related_entities(self): + expected_entity_data = INSTRUMENT_PANOSC_DATA.copy() + expected_entity_data["datasets"] = [DATASET_PANOSC_DATA] + + icat_data = INSTRUMENT_ICAT_DATA.copy() + icat_data["facility"] = FACILITY_ICAT_DATA + icat_data["datasetInstruments"] = [{"dataset": DATASET_ICAT_DATA.copy()}] + + instrument_entity = models.Instrument.from_icat(icat_data, ["datasets"]) + + assert instrument_entity.dict(by_alias=True) == expected_entity_data From f2845e31a1530a56e006e5d455e985bf621c1ea9 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:30:23 +0000 Subject: [PATCH 22/43] test: unit test `from_icat` `Member` entity creation #265 --- test/search_api/test_models.py | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 01fbe3e8..16e2e97f 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -445,3 +445,42 @@ def test_from_icat_instrument_entity_with_data_for_all_related_entities(self): instrument_entity = models.Instrument.from_icat(icat_data, ["datasets"]) assert instrument_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_member_entity_without_data_for_related_entities(self): + member_entity = models.Member.from_icat(INVESTIGATION_USER_ICAT_DATA, []) + + assert member_entity.dict(by_alias=True) == MEMBER_PANOSC_DATA + + def test_from_icat_member_entity_with_data_for_mandatory_related_entities(self): + expected_entity_data = MEMBER_PANOSC_DATA.copy() + expected_entity_data["document"] = DOCUMENT_PANOSC_DATA + + icat_data = INVESTIGATION_USER_ICAT_DATA.copy() + icat_data["investigation"] = INVESTIGATION_ICAT_DATA.copy() + icat_data["investigation"]["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["investigation"]["keywords"] = [KEYWORD_ICAT_DATA] + + member_entity = models.Member.from_icat(icat_data, ["document"]) + + assert member_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_member_entity_with_data_for_all_related_entities(self): + expected_entity_data = MEMBER_PANOSC_DATA.copy() + expected_entity_data["document"] = DOCUMENT_PANOSC_DATA + expected_entity_data["person"] = PERSON_PANOSC_DATA + expected_entity_data["affiliation"] = AFFILIATION_PANOSC_DATA + + icat_data = INVESTIGATION_USER_ICAT_DATA.copy() + icat_data["investigation"] = INVESTIGATION_ICAT_DATA.copy() + icat_data["investigation"]["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["investigation"]["keywords"] = [KEYWORD_ICAT_DATA] + icat_data["user"] = USER_ICAT_DATA.copy() + icat_data["user"]["dataPublicationUsers"] = [ + {"affiliations": [AFFILIATION_ICAT_DATA]}, + ] + + member_entity = models.Member.from_icat( + icat_data, ["document", "person", "affiliation"], + ) + + assert member_entity.dict(by_alias=True) == expected_entity_data From faed59477105b9dcfe75bf018d6b8229ff048a2e Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:39:40 +0000 Subject: [PATCH 23/43] test: unit test `from_icat` `Parameter` entity creation #265 --- test/search_api/test_models.py | 46 ++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 16e2e97f..5fa0f29d 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -484,3 +484,49 @@ def test_from_icat_member_entity_with_data_for_all_related_entities(self): ) assert member_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_parameter_entity_without_data_for_related_entities(self): + icat_data = INVESTIGATION_PARAMETER_ICAT_DATA.copy() + icat_data["type"] = PARAMETER_TYPE_ICAT_DATA + + parameter_entity = models.Parameter.from_icat(icat_data, []) + + assert parameter_entity.dict(by_alias=True) == PARAMETER_PANOSC_DATA + + def test_from_icat_parameter_entity_with_investigation_parameter_data(self): + expected_entity_data = PARAMETER_PANOSC_DATA.copy() + expected_entity_data["dataset"] = DATASET_PANOSC_DATA + expected_entity_data["document"] = DOCUMENT_PANOSC_DATA + + icat_data = INVESTIGATION_PARAMETER_ICAT_DATA.copy() + icat_data["type"] = PARAMETER_TYPE_ICAT_DATA + icat_data["investigation"] = INVESTIGATION_ICAT_DATA.copy() + icat_data["investigation"]["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["investigation"]["keywords"] = [KEYWORD_ICAT_DATA] + icat_data["investigation"]["investigationInstruments"] = [ + {"instrument": INSTRUMENT_ICAT_DATA.copy()}, + {"instrument": INSTRUMENT_ICAT_DATA.copy()}, + ] + icat_data["investigation"]["investigationInstruments"][0]["instrument"][ + "datasetInstruments" + ] = [{"dataset": DATASET_ICAT_DATA}, {"dataset": DATASET_ICAT_DATA}] + icat_data["investigation"]["investigationInstruments"][1]["instrument"][ + "datasetInstruments" + ] = [] + + parameter_entity = models.Parameter.from_icat(icat_data, ["dataset"]) + + assert parameter_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_parameter_entity_with_dataset_parameter_data(self): + expected_entity_data = PARAMETER_PANOSC_DATA.copy() + expected_entity_data["value"] = DATASET_PARAMETER_ICAT_DATA["stringValue"] + expected_entity_data["dataset"] = DATASET_PANOSC_DATA + + icat_data = DATASET_PARAMETER_ICAT_DATA.copy() + icat_data["dataset"] = DATASET_ICAT_DATA + icat_data["type"] = PARAMETER_TYPE_ICAT_DATA + + parameter_entity = models.Parameter.from_icat(icat_data, ["dataset"]) + + assert parameter_entity.dict(by_alias=True) == expected_entity_data From c05a70d40e271aede4ae0c4ebdb724536dbb8b33 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:41:05 +0000 Subject: [PATCH 24/43] test: unit test `from_icat` `Person` entity creation #265 --- test/search_api/test_models.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 5fa0f29d..19218804 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -530,3 +530,22 @@ def test_from_icat_parameter_entity_with_dataset_parameter_data(self): parameter_entity = models.Parameter.from_icat(icat_data, ["dataset"]) assert parameter_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_person_entity_without_data_for_related_entities(self): + person_entity = models.Person.from_icat(USER_ICAT_DATA, []) + + assert person_entity.dict(by_alias=True) == PERSON_PANOSC_DATA + + def test_from_icat_person_entity_with_data_for_all_related_entities(self): + expected_entity_data = PERSON_PANOSC_DATA.copy() + expected_entity_data["members"] = [MEMBER_PANOSC_DATA, MEMBER_PANOSC_DATA] + + icat_data = USER_ICAT_DATA.copy() + icat_data["investigationUsers"] = [ + INVESTIGATION_USER_ICAT_DATA, + INVESTIGATION_USER_ICAT_DATA, + ] + + person_entity = models.Person.from_icat(icat_data, ["members"]) + + assert person_entity.dict(by_alias=True) == expected_entity_data From 077c27952d883b96c560cf67e01eb1218ba5a991 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:42:43 +0000 Subject: [PATCH 25/43] test: unit test `from_icat` `Sample` entity creation #265 --- test/search_api/test_models.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 19218804..c2fc88d2 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -549,3 +549,26 @@ def test_from_icat_person_entity_with_data_for_all_related_entities(self): person_entity = models.Person.from_icat(icat_data, ["members"]) assert person_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_sample_entity_without_data_for_related_entities(self): + icat_data = SAMPLE_ICAT_DATA.copy() + icat_data["parameters"] = [ + {"type": PARAMETER_TYPE_ICAT_DATA}, + ] + sample_entity = models.Sample.from_icat(icat_data, []) + + assert sample_entity.dict(by_alias=True) == SAMPLE_PANOSC_DATA + + def test_from_icat_sample_entity_with_data_for_all_related_entities(self): + expected_entity_data = SAMPLE_PANOSC_DATA.copy() + expected_entity_data["datasets"] = [DATASET_PANOSC_DATA, DATASET_PANOSC_DATA] + + icat_data = SAMPLE_ICAT_DATA.copy() + icat_data["parameters"] = [ + {"type": PARAMETER_TYPE_ICAT_DATA}, + ] + icat_data["datasets"] = [DATASET_ICAT_DATA, DATASET_ICAT_DATA] + + sample_entity = models.Sample.from_icat(icat_data, ["datasets"]) + + assert sample_entity.dict(by_alias=True) == expected_entity_data From fb836c7ebb7305d87b05f9322465fea4c7edf9d0 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 17:45:40 +0000 Subject: [PATCH 26/43] test: unit test `from_icat` `Technique` entity creation #265 --- test/search_api/test_models.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index c2fc88d2..a4a4034e 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -572,3 +572,19 @@ def test_from_icat_sample_entity_with_data_for_all_related_entities(self): sample_entity = models.Sample.from_icat(icat_data, ["datasets"]) assert sample_entity.dict(by_alias=True) == expected_entity_data + + def test_from_icat_technique_entity_without_data_for_related_entities(self): + technique_entity = models.Technique.from_icat(TECHNIQUE_ICAT_DATA, []) + + assert technique_entity.dict(by_alias=True) == TECHNIQUE_PANOSC_DATA + + def test_from_icat_technique_entity_with_data_for_all_related_entities(self): + expected_entity_data = TECHNIQUE_PANOSC_DATA.copy() + expected_entity_data["datasets"] = [DATASET_PANOSC_DATA] + + icat_data = TECHNIQUE_ICAT_DATA.copy() + icat_data["datasetTechniques"] = [{"dataset": DATASET_ICAT_DATA}] + + technique_entity = models.Technique.from_icat(icat_data, ["datasets"]) + + assert technique_entity.dict(by_alias=True) == expected_entity_data From 701eb740ce09a98424bf72088a45c2adf8419797 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 17 Jan 2022 18:11:14 +0000 Subject: [PATCH 27/43] test: unit test `from_icat` raises `ValidationError` #265 --- test/search_api/test_models.py | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index a4a4034e..01017dca 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -1,5 +1,8 @@ from datetime import datetime, timezone +from pydantic import ValidationError +import pytest + import datagateway_api.src.search_api.models as models @@ -588,3 +591,76 @@ def test_from_icat_technique_entity_with_data_for_all_related_entities(self): technique_entity = models.Technique.from_icat(icat_data, ["datasets"]) assert technique_entity.dict(by_alias=True) == expected_entity_data + + @pytest.mark.parametrize( + "panosc_entity_name, icat_data, required_related_fields", + [ + pytest.param( + "Affiliation", + AFFILIATION_ICAT_DATA, + ["members"], + id="Affiliation - without data for mandatory related entity", + ), + pytest.param( + "Dataset", {}, [], id="Dataset - without data for mandatory fields", + ), + pytest.param( + "Dataset", + DATASET_ICAT_DATA, + ["documents"], + id="Dataset - without data for mandatory related entity", + ), + pytest.param( + "Document", {}, [], id="Document - without data for mandatory fields", + ), + pytest.param( + "Document", + INVESTIGATION_ICAT_DATA, + ["dataset"], + id="Document - without data for mandatory related entity", + ), + pytest.param( + "File", {}, [], id="File - without data for mandatory fields", + ), + pytest.param( + "File", + DATAFILE_ICAT_DATA, + ["dataset"], + id="File - without data for mandatory related entity", + ), + pytest.param( + "Instrument", + {}, + [], + id="Instrument - without data for mandatory fields", + ), + pytest.param( + "Member", {}, [], id="Member - without data for mandatory fields", + ), + pytest.param( + "Member", + INVESTIGATION_USER_ICAT_DATA, + ["document"], + id="Member - without data for mandatory related entity", + ), + pytest.param( + "Parameter", {}, [], id="Parameter - without data for mandatory fields", + ), + pytest.param( + "Person", {}, [], id="Person - without data for mandatory fields", + ), + pytest.param( + "Sample", {}, [], id="Sample - without data for mandatory fields", + ), + pytest.param( + "Technique", {}, [], id="Technique - without data for mandatory fields", + ), + ], + ) + def test_from_icat_raises_validation_error( + self, panosc_entity_name, icat_data, required_related_fields, + ): + with pytest.raises(ValidationError): + getattr(models, panosc_entity_name).from_icat( + icat_data, required_related_fields, + ) From e691f249d5a0b13534396cf0fef9868726b1a562 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Tue, 18 Jan 2022 08:59:31 +0000 Subject: [PATCH 28/43] update `Parameter` mappings in example file #265 --- datagateway_api/search_api_mapping.json.example | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/datagateway_api/search_api_mapping.json.example b/datagateway_api/search_api_mapping.json.example index 57161585..f93c2a04 100644 --- a/datagateway_api/search_api_mapping.json.example +++ b/datagateway_api/search_api_mapping.json.example @@ -63,12 +63,12 @@ "affiliation": {"Affiliation": "user.dataPublicationUsers.affiliations"} }, "Parameter": { - "base_icat_entity": "InvestigationParameter", + "base_icat_entity": ["InvestigationParameter", "DatasetParameter"], "id": "id", - "name": "name", + "name": "type.name", "value": ["numericValue", "stringValue", "dateTimeValue"], "unit": "type.units", - "dataset": {"Dataset": "investigation.investigationInstruments.instrument.datasetInstruments.dataset"}, + "dataset": {"Dataset": ["investigation.investigationInstruments.instrument.datasetInstruments.dataset", "dataset"]}, "document": {"Document": "investigation"} }, "Person": { From d606d6b1514e95aecc423b9aa33fae3e0eb86136 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 24 Jan 2022 15:50:42 +0000 Subject: [PATCH 29/43] add logic for dealing with nested required related fields #265 --- datagateway_api/src/search_api/models.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 200c2460..16f341f5 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -73,10 +73,23 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 if not isinstance(data, list): data = [data] + required_related_fields_for_next_entity = [] + for required_related_field in required_related_fields: + required_related_field = required_related_field.split(".") + if ( + len(required_related_field) > 1 + and field_alias in required_related_field + ): + required_related_fields_for_next_entity.extend( + required_related_field[1:], + ) + # Get the class of the referenced model panosc_model_attr = getattr(sys.modules[__name__], panosc_entity_name) field_value = [ - panosc_model_attr.from_icat(d, required_related_fields) + panosc_model_attr.from_icat( + d, required_related_fields_for_next_entity, + ) for d in data ] @@ -92,6 +105,8 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 model_data[field_alias] = field_value for required_related_field in required_related_fields: + required_related_field = required_related_field.split(".")[0] + if ( required_related_field in model_fields and required_related_field From e52d5958bd8c8fe0b1dab79f91f2e95787723cc7 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 24 Jan 2022 15:51:31 +0000 Subject: [PATCH 30/43] test: unit test logic for nested required related fields #265 --- test/search_api/test_models.py | 79 ++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 01017dca..2887236b 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -592,6 +592,85 @@ def test_from_icat_technique_entity_with_data_for_all_related_entities(self): assert technique_entity.dict(by_alias=True) == expected_entity_data + def test_from_icat_multiple_and_nested_relations(self): + expected_entity_data = DOCUMENT_PANOSC_DATA.copy() + expected_entity_data["keywords"] = [] + expected_entity_data["datasets"] = [DATASET_PANOSC_DATA.copy()] + expected_entity_data["datasets"][0]["instrument"] = INSTRUMENT_PANOSC_DATA + expected_entity_data["datasets"][0]["files"] = [ + FILE_PANOSC_DATA, + FILE_PANOSC_DATA, + ] + expected_entity_data["datasets"][0]["parameters"] = [ + PARAMETER_PANOSC_DATA.copy(), + ] + expected_entity_data["datasets"][0]["parameters"][0][ + "value" + ] = DATASET_PARAMETER_ICAT_DATA["stringValue"] + expected_entity_data["datasets"][0]["samples"] = [SAMPLE_PANOSC_DATA.copy()] + expected_entity_data["datasets"][0]["samples"][0]["description"] = None + expected_entity_data["members"] = [MEMBER_PANOSC_DATA.copy()] + expected_entity_data["members"][0]["affiliation"] = AFFILIATION_PANOSC_DATA + expected_entity_data["members"][0]["person"] = PERSON_PANOSC_DATA + expected_entity_data["parameters"] = [PARAMETER_PANOSC_DATA.copy()] + expected_entity_data["parameters"][0]["value"] + expected_entity_data["parameters"][0]["dataset"] = DATASET_PANOSC_DATA.copy() + expected_entity_data["parameters"][0]["document"] = DOCUMENT_PANOSC_DATA.copy() + expected_entity_data["parameters"][0]["document"]["keywords"] = [] + expected_entity_data["parameters"][0]["dataset"]["techniques"] = [ + TECHNIQUE_PANOSC_DATA, + ] + + icat_data = INVESTIGATION_ICAT_DATA.copy() + icat_data["type"] = INVESTIGATION_TYPE_ICAT_DATA + icat_data["datasets"] = [DATASET_ICAT_DATA.copy()] + icat_data["datasets"][0]["datasetInstruments"] = [ + {"instrument": INSTRUMENT_ICAT_DATA.copy()}, + ] + icat_data["datasets"][0]["datasetInstruments"][0]["instrument"][ + "facility" + ] = FACILITY_ICAT_DATA + icat_data["datasets"][0]["datafiles"] = [DATAFILE_ICAT_DATA, DATAFILE_ICAT_DATA] + icat_data["datasets"][0]["parameters"] = [DATASET_PARAMETER_ICAT_DATA.copy()] + icat_data["datasets"][0]["parameters"][0]["type"] = PARAMETER_TYPE_ICAT_DATA + icat_data["datasets"][0]["sample"] = SAMPLE_ICAT_DATA + icat_data["investigationUsers"] = [INVESTIGATION_USER_ICAT_DATA.copy()] + icat_data["investigationUsers"][0]["user"] = USER_ICAT_DATA.copy() + icat_data["investigationUsers"][0]["user"]["dataPublicationUsers"] = [ + {"affiliations": [AFFILIATION_ICAT_DATA]}, + ] + icat_data["parameters"] = [INVESTIGATION_PARAMETER_ICAT_DATA.copy()] + icat_data["parameters"][0]["type"] = PARAMETER_TYPE_ICAT_DATA + icat_data["parameters"][0]["investigation"] = INVESTIGATION_ICAT_DATA.copy() + icat_data["parameters"][0]["investigation"][ + "type" + ] = INVESTIGATION_TYPE_ICAT_DATA + dataset_with_techniques_icat = DATASET_ICAT_DATA.copy() + dataset_with_techniques_icat.update( + {"datasetTechniques": [{"technique": TECHNIQUE_ICAT_DATA}]}, + ) + icat_data["parameters"][0]["investigation"]["investigationInstruments"] = [ + { + "instrument": { + "datasetInstruments": [{"dataset": dataset_with_techniques_icat}], + }, + }, + ] + + relations = [ + "datasets.instrument", + "datasets.files", + "datasets.parameters", + "datasets.samples", + "members.affiliation", + "members.person", + "parameters.dataset.techniques", + ] + + document_entity = models.Document.from_icat(icat_data, relations) + + assert document_entity.dict(by_alias=True) == expected_entity_data + @pytest.mark.parametrize( "panosc_entity_name, icat_data, required_related_fields", [ From 601c51b29cd29fbad687d83084202996e7eb4714 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 10:55:50 +0000 Subject: [PATCH 31/43] refactor: update PaNOSC `Parameter` mappings in example file #265 --- datagateway_api/search_api_mapping.json.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datagateway_api/search_api_mapping.json.example b/datagateway_api/search_api_mapping.json.example index f93c2a04..19a81cfa 100644 --- a/datagateway_api/search_api_mapping.json.example +++ b/datagateway_api/search_api_mapping.json.example @@ -68,7 +68,7 @@ "name": "type.name", "value": ["numericValue", "stringValue", "dateTimeValue"], "unit": "type.units", - "dataset": {"Dataset": ["investigation.investigationInstruments.instrument.datasetInstruments.dataset", "dataset"]}, + "dataset": {"Dataset": "dataset"}, "document": {"Document": "investigation"} }, "Person": { From 1e8badb9fae458ee5d0be2a67d3774ef9a198e10 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 10:56:31 +0000 Subject: [PATCH 32/43] test: update `Parameter` mappings in mappings fixture #265 --- test/search_api/conftest.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/search_api/conftest.py b/test/search_api/conftest.py index e5bfb508..e8a52212 100644 --- a/test/search_api/conftest.py +++ b/test/search_api/conftest.py @@ -89,15 +89,12 @@ def test_search_api_mappings_data(): "affiliation": {"Affiliation": "user.dataPublicationUsers.affiliations"}, }, "Parameter": { - "base_icat_entity": "InvestigationParameter", + "base_icat_entity": ["InvestigationParameter", "DatasetParameter"], "id": "id", - "name": "name", + "name": "type.name", "value": ["numericValue", "stringValue", "dateTimeValue"], "unit": "type.units", - "dataset": { - "Dataset": "investigation.investigationInstruments.instrument." - "datasetInstruments.dataset", - }, + "dataset": {"Dataset": "dataset"}, "document": {"Document": "investigation"}, }, "Person": { From d595cebf354e95cd3d4fa229229594ec131e4441 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 10:57:56 +0000 Subject: [PATCH 33/43] test: fix tests following updates to PaNOSC `Parameter` mappings #265 --- test/search_api/test_models.py | 35 ++++------------------------------ 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 2887236b..3c4e04a0 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -498,7 +498,6 @@ def test_from_icat_parameter_entity_without_data_for_related_entities(self): def test_from_icat_parameter_entity_with_investigation_parameter_data(self): expected_entity_data = PARAMETER_PANOSC_DATA.copy() - expected_entity_data["dataset"] = DATASET_PANOSC_DATA expected_entity_data["document"] = DOCUMENT_PANOSC_DATA icat_data = INVESTIGATION_PARAMETER_ICAT_DATA.copy() @@ -506,18 +505,8 @@ def test_from_icat_parameter_entity_with_investigation_parameter_data(self): icat_data["investigation"] = INVESTIGATION_ICAT_DATA.copy() icat_data["investigation"]["type"] = INVESTIGATION_TYPE_ICAT_DATA icat_data["investigation"]["keywords"] = [KEYWORD_ICAT_DATA] - icat_data["investigation"]["investigationInstruments"] = [ - {"instrument": INSTRUMENT_ICAT_DATA.copy()}, - {"instrument": INSTRUMENT_ICAT_DATA.copy()}, - ] - icat_data["investigation"]["investigationInstruments"][0]["instrument"][ - "datasetInstruments" - ] = [{"dataset": DATASET_ICAT_DATA}, {"dataset": DATASET_ICAT_DATA}] - icat_data["investigation"]["investigationInstruments"][1]["instrument"][ - "datasetInstruments" - ] = [] - parameter_entity = models.Parameter.from_icat(icat_data, ["dataset"]) + parameter_entity = models.Parameter.from_icat(icat_data, ["document"]) assert parameter_entity.dict(by_alias=True) == expected_entity_data @@ -613,13 +602,7 @@ def test_from_icat_multiple_and_nested_relations(self): expected_entity_data["members"][0]["affiliation"] = AFFILIATION_PANOSC_DATA expected_entity_data["members"][0]["person"] = PERSON_PANOSC_DATA expected_entity_data["parameters"] = [PARAMETER_PANOSC_DATA.copy()] - expected_entity_data["parameters"][0]["value"] - expected_entity_data["parameters"][0]["dataset"] = DATASET_PANOSC_DATA.copy() - expected_entity_data["parameters"][0]["document"] = DOCUMENT_PANOSC_DATA.copy() - expected_entity_data["parameters"][0]["document"]["keywords"] = [] - expected_entity_data["parameters"][0]["dataset"]["techniques"] = [ - TECHNIQUE_PANOSC_DATA, - ] + expected_entity_data["parameters"][0]["document"] = DOCUMENT_PANOSC_DATA icat_data = INVESTIGATION_ICAT_DATA.copy() icat_data["type"] = INVESTIGATION_TYPE_ICAT_DATA @@ -645,17 +628,7 @@ def test_from_icat_multiple_and_nested_relations(self): icat_data["parameters"][0]["investigation"][ "type" ] = INVESTIGATION_TYPE_ICAT_DATA - dataset_with_techniques_icat = DATASET_ICAT_DATA.copy() - dataset_with_techniques_icat.update( - {"datasetTechniques": [{"technique": TECHNIQUE_ICAT_DATA}]}, - ) - icat_data["parameters"][0]["investigation"]["investigationInstruments"] = [ - { - "instrument": { - "datasetInstruments": [{"dataset": dataset_with_techniques_icat}], - }, - }, - ] + icat_data["parameters"][0]["investigation"]["keywords"] = [KEYWORD_ICAT_DATA] relations = [ "datasets.instrument", @@ -664,7 +637,7 @@ def test_from_icat_multiple_and_nested_relations(self): "datasets.samples", "members.affiliation", "members.person", - "parameters.dataset.techniques", + "parameters.document", ] document_entity = models.Document.from_icat(icat_data, relations) From 0bbcc0d4f762c52b9fa4a4b558fff1a44fbd40b7 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 11:00:55 +0000 Subject: [PATCH 34/43] refactor: replace `fromisoformat` with `str_to_datetime_object` #265 --- datagateway_api/src/search_api/models.py | 5 +++-- test/search_api/test_models.py | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 16f341f5..504478ab 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -8,6 +8,7 @@ from pydantic import BaseModel, Field, ValidationError, validator from pydantic.error_wrappers import ErrorWrapper +from datagateway_api.src.common.date_handler import DateHandler from datagateway_api.src.search_api.panosc_mappings import mappings @@ -174,7 +175,7 @@ def set_is_public(cls, value): # noqa: B902, N805 if not value: return value - creation_date = datetime.fromisoformat(value) + 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 @@ -213,7 +214,7 @@ def set_is_public(cls, value): # noqa: B902, N805 if not value: return value - creation_date = datetime.fromisoformat(value) + 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 diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 3c4e04a0..66eb1485 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -3,6 +3,7 @@ from pydantic import ValidationError import pytest +from datagateway_api.src.common.date_handler import DateHandler import datagateway_api.src.search_api.models as models @@ -199,7 +200,9 @@ DATASET_PANOSC_DATA = { "pid": DATASET_ICAT_DATA["doi"], "title": DATASET_ICAT_DATA["name"], - "creationDate": datetime.fromisoformat(DATASET_ICAT_DATA["createTime"]), + "creationDate": DateHandler.str_to_datetime_object( + DATASET_ICAT_DATA["createTime"], + ).replace(tzinfo=timezone.utc), "isPublic": True, "size": None, "documents": [], @@ -217,9 +220,15 @@ "title": INVESTIGATION_ICAT_DATA["name"], "summary": INVESTIGATION_ICAT_DATA["summary"], "doi": INVESTIGATION_ICAT_DATA["doi"], - "startDate": datetime.fromisoformat(INVESTIGATION_ICAT_DATA["startDate"]), - "endDate": datetime.fromisoformat(INVESTIGATION_ICAT_DATA["endDate"]), - "releaseDate": datetime.fromisoformat(INVESTIGATION_ICAT_DATA["releaseDate"]), + "startDate": DateHandler.str_to_datetime_object( + INVESTIGATION_ICAT_DATA["startDate"], + ).replace(tzinfo=timezone.utc), + "endDate": DateHandler.str_to_datetime_object( + INVESTIGATION_ICAT_DATA["endDate"], + ).replace(tzinfo=timezone.utc), + "releaseDate": DateHandler.str_to_datetime_object( + INVESTIGATION_ICAT_DATA["releaseDate"], + ).replace(tzinfo=timezone.utc), "license": None, "keywords": [KEYWORD_ICAT_DATA["name"]], "datasets": [], From 299cbcdd46672eb4424066d44891e03fd7c41115 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 28 Jan 2022 12:07:20 +0000 Subject: [PATCH 35/43] Apply suggestions from code review #265 Co-authored-by: Matthew Richards <32678030+MRichards99@users.noreply.github.com> --- datagateway_api/src/search_api/models.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 16f341f5..a2a2211d 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -1,5 +1,4 @@ -import abc -from abc import ABC +from abc import ABC, abstractmethod from datetime import datetime, timezone import sys from typing import ClassVar, List, Optional, Union @@ -33,7 +32,7 @@ def _get_icat_field_value(icat_field_name, icat_data): class PaNOSCAttribute(ABC, BaseModel): @classmethod - @abc.abstractmethod + @abstractmethod def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 model_fields = cls.__fields__ @@ -54,9 +53,8 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 field_value = None for field_name in icat_field_name: try: - value = _get_icat_field_value(field_name, icat_data) - if value: - field_value = value + field_value = _get_icat_field_value(field_name, icat_data) + if field_value: break except KeyError: continue From 3f1b1cffdd1e57ab4eb1227b13e0906424adefd0 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 12:58:47 +0000 Subject: [PATCH 36/43] docs: add new comments and fix existing #265 --- datagateway_api/src/search_api/models.py | 62 ++++++++++++++++-------- 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index e16b559b..dea517a2 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -39,9 +39,9 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 model_data = {} for field in model_fields: - # Some fields have aliases so we must use them when creating a model instance. - # If a field does not have an alias then the `alias` property holds the name - # of the field + # Some fields have aliases so we must use them when creating a model + # instance. If a field does not have an alias then the `alias` property + # holds the name of the field field_alias = cls.__fields__[field].alias panosc_entity_name, icat_field_name = mappings.get_icat_mapping( @@ -58,16 +58,26 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 if field_value: break except KeyError: + # If an icat value cannot be found for the ICAT field name in the + # provided ICAT data then ignore the error. The field name could + # simply be a mapping of an optional PaNOSC entity field so ICAT + # may not return data for it which is fine. It could also be a list + # of mappings which is the case with the `value` field of the + # PaNOSC entity. When this is the case, ICAT only returns data for + # one of the mappings from the list so we can ignore the error. + # This also ignores errors for mandatory fields but this is not a + # problem because pydantic is responsible for validating whether + # data for mandatory fields is missing. continue if not field_value: continue if panosc_entity_name != cls.__name__: - # If we are here, it means that the field references another model so we - # have to get hold of its class definition and call its `from_icat` method - # to create an instance of itself with the ICAT data provided. Doing this - # allows for recursion. + # If we are here, it means that the field references another model so + # we have to get hold of its class definition and call its `from_icat` + # method to create an instance of itself with the ICAT data provided. + # Doing this allows for recursion. data = field_value if not isinstance(data, list): data = [data] @@ -113,9 +123,10 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 and required_related_field not in model_data ): # If we are here, it means that a related entity, which has a minimum - # cardinality of one, has been specified to be included as part of the entity - # but the relevant ICAT data needed for its creation cannot be found in the - # provided ICAT response. Because of this, a ValidationError is raised. + # cardinality of one, has been specified to be included as part of the + # entity but the relevant ICAT data needed for its creation cannot be + # found in the provided ICAT response. Because of this, a + # `ValidationError` is raised. error_wrapper = ErrorWrapper( TypeError("field required"), loc=required_related_field, ) @@ -292,16 +303,27 @@ class Parameter(PaNOSCAttribute): dataset: Optional[Dataset] = None document: Optional[Document] = None - # @root_validator(skip_on_failure=True) - # def validate_dataset_and_document(cls, values): # noqa: B902, N805 - # if values["dataset"] is None and values["document"] is None: - # raise TypeError("must have a dataset or document") - - # if values["dataset"] is not None and values["document"] is not None: - # # TODO - Should an exception be raised here instead? - # values["Document"] = None - - # return values + """ + Validator commented as it was decided to be disabled for the time being. The Data + Model states that a Parameter must be related to a Dataset or Document, however + considering that there is not a Parameter endpoint, it means that a Parameter can + only be included via Dataset or Document. It's unclear why anyone would query for + a Dataset or Document that includes Parameters which in turn includes a Dataset or + Document that are the same as the top level ones. To avoid errors being raised + as a result of Parameters not containing ICAT data for a Dataset or Document, the + validator has been disabled. + + @root_validator(skip_on_failure=True) + def validate_dataset_and_document(cls, values): # noqa: B902, N805 + if values["dataset"] is None and values["document"] is None: + raise TypeError("must have a dataset or document") + + if values["dataset"] is not None and values["document"] is not None: + # TODO - Should an exception be raised here instead? + values["Document"] = None + + return values + """ @classmethod def from_icat(cls, icat_data, required_related_fields): From 1d730d1d07a523d2b636d905911a5073aa51d305 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 12:59:43 +0000 Subject: [PATCH 37/43] refactor: refactor `_get_icat_field_value` logic #265 --- datagateway_api/src/search_api/models.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index dea517a2..81f8d568 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -18,14 +18,10 @@ def _get_icat_field_value(icat_field_name, icat_data): values = [] for data in icat_data: value = _get_icat_field_value(field_name, data) - if isinstance(value, list): - values.extend(value) - else: - values.append(value) - + value = [value] if not isinstance(value, list) else value + values.extend(value) icat_data = values - - if isinstance(icat_data, dict): + elif isinstance(icat_data, dict): icat_data = icat_data[field_name] return icat_data From 557172212c07efea7a7d7ea85793446cc0f80566 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 13:28:40 +0000 Subject: [PATCH 38/43] test: remove code for replacing datetime timezone #265 --- test/search_api/test_models.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/search_api/test_models.py b/test/search_api/test_models.py index 66eb1485..df6aa84b 100644 --- a/test/search_api/test_models.py +++ b/test/search_api/test_models.py @@ -202,7 +202,7 @@ "title": DATASET_ICAT_DATA["name"], "creationDate": DateHandler.str_to_datetime_object( DATASET_ICAT_DATA["createTime"], - ).replace(tzinfo=timezone.utc), + ), "isPublic": True, "size": None, "documents": [], @@ -222,13 +222,11 @@ "doi": INVESTIGATION_ICAT_DATA["doi"], "startDate": DateHandler.str_to_datetime_object( INVESTIGATION_ICAT_DATA["startDate"], - ).replace(tzinfo=timezone.utc), - "endDate": DateHandler.str_to_datetime_object( - INVESTIGATION_ICAT_DATA["endDate"], - ).replace(tzinfo=timezone.utc), + ), + "endDate": DateHandler.str_to_datetime_object(INVESTIGATION_ICAT_DATA["endDate"]), "releaseDate": DateHandler.str_to_datetime_object( INVESTIGATION_ICAT_DATA["releaseDate"], - ).replace(tzinfo=timezone.utc), + ), "license": None, "keywords": [KEYWORD_ICAT_DATA["name"]], "datasets": [], From 71765a590d996b9935299ebfb3de305660542ca1 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 28 Jan 2022 13:41:40 +0000 Subject: [PATCH 39/43] refactor: refactor `from_icat` abstract method #265 --- datagateway_api/src/search_api/models.py | 44 +++++++++++------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 81f8d568..f67e7f04 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -31,17 +31,17 @@ class PaNOSCAttribute(ABC, BaseModel): @classmethod @abstractmethod def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 - model_fields = cls.__fields__ + entity_fields = cls.__fields__ - model_data = {} - for field in model_fields: + entity_data = {} + for entity_field in entity_fields: # Some fields have aliases so we must use them when creating a model # instance. If a field does not have an alias then the `alias` property # holds the name of the field - field_alias = cls.__fields__[field].alias + entity_field_alias = cls.__fields__[entity_field].alias - panosc_entity_name, icat_field_name = mappings.get_icat_mapping( - cls.__name__, field_alias, + entity_name, icat_field_name = mappings.get_icat_mapping( + cls.__name__, entity_field_alias, ) if not isinstance(icat_field_name, list): @@ -69,54 +69,52 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 if not field_value: continue - if panosc_entity_name != cls.__name__: + if entity_name != cls.__name__: # If we are here, it means that the field references another model so # we have to get hold of its class definition and call its `from_icat` # method to create an instance of itself with the ICAT data provided. # Doing this allows for recursion. - data = field_value - if not isinstance(data, list): - data = [data] + data = ( + [field_value] if not isinstance(field_value, list) else field_value + ) required_related_fields_for_next_entity = [] for required_related_field in required_related_fields: required_related_field = required_related_field.split(".") if ( len(required_related_field) > 1 - and field_alias in required_related_field + and entity_field_alias in required_related_field ): required_related_fields_for_next_entity.extend( required_related_field[1:], ) - # Get the class of the referenced model - panosc_model_attr = getattr(sys.modules[__name__], panosc_entity_name) + # Get the class of the referenced entity + entity_attr = getattr(sys.modules[__name__], entity_name) field_value = [ - panosc_model_attr.from_icat( - d, required_related_fields_for_next_entity, - ) + entity_attr.from_icat(d, required_related_fields_for_next_entity) for d in data ] - field_outer_type = cls.__fields__[field].outer_type_ + entity_field_outer_type = cls.__fields__[entity_field].outer_type_ if ( - not hasattr(field_outer_type, "_name") - or field_outer_type._name != "List" + not hasattr(entity_field_outer_type, "_name") + or entity_field_outer_type._name != "List" ) and isinstance(field_value, list): # If the field does not hold list of values but `field_value` # is a list, then just get its first element field_value = field_value[0] - model_data[field_alias] = field_value + entity_data[entity_field_alias] = field_value for required_related_field in required_related_fields: required_related_field = required_related_field.split(".")[0] if ( - required_related_field in model_fields + required_related_field in entity_fields and required_related_field in cls._related_fields_with_min_cardinality_one - and required_related_field not in model_data + and required_related_field not in entity_data ): # If we are here, it means that a related entity, which has a minimum # cardinality of one, has been specified to be included as part of the @@ -128,7 +126,7 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 ) raise ValidationError(errors=[error_wrapper], model=cls) - return cls(**model_data) + return cls(**entity_data) class Affiliation(PaNOSCAttribute): From 691a59ea3f850475572c3a877fb739e5216c6fe7 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 31 Jan 2022 13:39:21 +0000 Subject: [PATCH 40/43] fix: fix list type field checking in Python 3.6 #265 --- datagateway_api/src/search_api/models.py | 25 ++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index f67e7f04..2781dec1 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -11,6 +11,25 @@ from datagateway_api.src.search_api.panosc_mappings import mappings +def _is_panosc_entity_field_of_type_list(entity_field): + entity_field_outer_type = entity_field.outer_type_ + if ( + hasattr(entity_field_outer_type, "_name") + and entity_field_outer_type._name == "List" + ): + is_list = True + # The `_name` `outer_type_` attribute was introduced in Python 3.7 so to check + # whether the field is of type list in Python 3.6, we are checking the type of its + # defualt value. We must ensure that any new list fields that get added in future + # are assigned a list by default. + elif isinstance(entity_field.default, list): + is_list = True + else: + is_list = False + + return is_list + + def _get_icat_field_value(icat_field_name, icat_data): icat_field_name = icat_field_name.split(".") for field_name in icat_field_name: @@ -96,10 +115,8 @@ def from_icat(cls, icat_data, required_related_fields): # noqa: B902, N805 for d in data ] - entity_field_outer_type = cls.__fields__[entity_field].outer_type_ - if ( - not hasattr(entity_field_outer_type, "_name") - or entity_field_outer_type._name != "List" + if not _is_panosc_entity_field_of_type_list( + cls.__fields__[entity_field], ) and isinstance(field_value, list): # If the field does not hold list of values but `field_value` # is a list, then just get its first element From a2534223af50b68c4195f843e8bac13d8d988bef Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 31 Jan 2022 13:40:57 +0000 Subject: [PATCH 41/43] style: address linting warnings #265 --- datagateway_api/src/search_api/filters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index 1ebbf579..cd840204 100644 --- a/datagateway_api/src/search_api/filters.py +++ b/datagateway_api/src/search_api/filters.py @@ -3,7 +3,6 @@ import logging from datagateway_api.src.common.date_handler import DateHandler -from datagateway_api.src.common.exceptions import FilterError from datagateway_api.src.datagateway_api.icat.filters import ( PythonICATIncludeFilter, PythonICATLimitFilter, From 3f1b2d6a1d216d9596e5cda2c261e0911324f541 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 31 Jan 2022 16:31:45 +0000 Subject: [PATCH 42/43] style: fix typo #265 --- datagateway_api/src/search_api/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datagateway_api/src/search_api/models.py b/datagateway_api/src/search_api/models.py index 2781dec1..f9c8ae9a 100644 --- a/datagateway_api/src/search_api/models.py +++ b/datagateway_api/src/search_api/models.py @@ -20,7 +20,7 @@ def _is_panosc_entity_field_of_type_list(entity_field): is_list = True # The `_name` `outer_type_` attribute was introduced in Python 3.7 so to check # whether the field is of type list in Python 3.6, we are checking the type of its - # defualt value. We must ensure that any new list fields that get added in future + # default value. We must ensure that any new list fields that get added in future # are assigned a list by default. elif isinstance(entity_field.default, list): is_list = True From 358ac22504b6869686c4caf6b47282fda916cb60 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Mon, 31 Jan 2022 17:53:36 +0000 Subject: [PATCH 43/43] refactor: update PaNOSC Parameter name mapping in example file #265 --- datagateway_api/search_api_mapping.json.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datagateway_api/search_api_mapping.json.example b/datagateway_api/search_api_mapping.json.example index 02b4ffc8..19a81cfa 100644 --- a/datagateway_api/search_api_mapping.json.example +++ b/datagateway_api/search_api_mapping.json.example @@ -65,7 +65,7 @@ "Parameter": { "base_icat_entity": ["InvestigationParameter", "DatasetParameter"], "id": "id", - "name": "name", + "name": "type.name", "value": ["numericValue", "stringValue", "dateTimeValue"], "unit": "type.units", "dataset": {"Dataset": "dataset"},