Merge pull request #324 from ral-facilities/bugfix/validation-error-s…

…ample-pid-panosc-field-#314

Validation error for Sample pid field when ICAT value is None

datagateway_api/search_api_mapping.json.example

@@ -10,7 +10,7 @@
     },
     "Dataset": {
         "base_icat_entity": "Dataset",
-        "pid": "doi",
+        "pid": ["doi", "id"],
         "title": "name",
         "isPublic": "createTime",
         "creationDate": "createTime",
@@ -24,7 +24,7 @@
     },
     "Document": {
         "base_icat_entity": "Investigation",
-        "pid": "doi",
+        "pid": ["doi", "id"],
         "isPublic": "releaseDate",
         "type": "type.name",
         "title": "name",
@@ -49,7 +49,7 @@
     },
     "Instrument": {
         "base_icat_entity": "Instrument",
-        "pid": "id",
+        "pid": ["pid", "id"],
         "name": "name",
         "facility": "facility.name",
         "datasets": {"Dataset": "datasetInstruments.dataset"}
@@ -84,13 +84,13 @@
     "Sample": {
         "base_icat_entity": "Sample",
         "name": "name",
-        "pid": "pid",
+        "pid": ["pid", "id"],
         "description": "parameters.type.description",
         "datasets": {"Dataset": "datasets"}
     },
     "Technique": {
         "base_icat_entity": "Technique",
-        "pid": "pid",
+        "pid": ["pid", "id"],
         "name": "name",
         "datasets": {"Dataset": "datasetTechniques.dataset"}
     }

datagateway_api/src/search_api/filters.py

@@ -42,23 +42,33 @@ def apply_filter(self, query):
                 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"]}}
+            # Edge cases for ICAT have been somewhat hardcoded here, to deal with
+            # ICAT's different parameter value and sample pid field names.
             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):
+                # The following mapping is assumed for parameter value (where order
+                # matters):
+                # {"Parameter": {"value": ["numericValue", "stringValue", "dateTimeValue"]}} # noqa: B950
+                if field_name == "value":
+                    if isinstance(self.value, (int, 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]
-                else:
-                    self.value = str(self.value)
-                    icat_field_name = icat_field_name[1]
+                # The following mapping is assumed for sample pid (where order matters):
+                # {"Sample": {"pid": ["pid", "id"]}}
+                elif field_name == "pid":
+                    if "pid:" in self.value:
+                        icat_field_name = icat_field_name[1]
+                        self.value = self.value.replace("pid:", "")
+                    else:
+                        icat_field_name = icat_field_name[0]
 
             icat_field_names.append(icat_field_name)
 

datagateway_api/src/search_api/models.py

@@ -191,6 +191,10 @@ class Dataset(PaNOSCAttribute):
     parameters: Optional[List["Parameter"]] = []
     samples: Optional[List["Sample"]] = []
 
+    @validator("pid", pre=True, always=True)
+    def set_pid(cls, value):  # noqa: B902, N805
+        return f"pid:{value}" if isinstance(value, int) else value
+
     @validator("is_public", pre=True, always=True)
     def set_is_public(cls, value):  # noqa: B902, N805
         if not value:
@@ -232,6 +236,10 @@ class Document(PaNOSCAttribute):
     members: Optional[List["Member"]] = []
     parameters: Optional[List["Parameter"]] = []
 
+    @validator("pid", pre=True, always=True)
+    def set_pid(cls, value):  # noqa: B902, N805
+        return f"pid:{value}" if isinstance(value, int) else value
+
     @validator("is_public", pre=True, always=True)
     def set_is_public(cls, value):  # noqa: B902, N805
         if not value:
@@ -279,6 +287,10 @@ class Instrument(PaNOSCAttribute):
 
     datasets: Optional[List[Dataset]] = []
 
+    @validator("pid", pre=True, always=True)
+    def set_pid(cls, value):  # noqa: B902, N805
+        return f"pid:{value}" if isinstance(value, int) else value
+
     @classmethod
     def from_icat(cls, icat_data, required_related_fields):
         return super(Instrument, cls).from_icat(icat_data, required_related_fields)
@@ -378,6 +390,10 @@ class Sample(PaNOSCAttribute):
 
     datasets: Optional[List[Dataset]] = []
 
+    @validator("pid", pre=True, always=True)
+    def set_pid(cls, value):  # noqa: B902, N805
+        return f"pid:{value}" if isinstance(value, int) else value
+
     @classmethod
     def from_icat(cls, icat_data, required_related_fields):
         return super(Sample, cls).from_icat(icat_data, required_related_fields)
@@ -394,6 +410,10 @@ class Technique(PaNOSCAttribute):
 
     datasets: Optional[List[Dataset]] = []
 
+    @validator("pid", pre=True, always=True)
+    def set_pid(cls, value):  # noqa: B902, N805
+        return f"pid:{value}" if isinstance(value, int) else value
+
     @classmethod
     def from_icat(cls, icat_data, required_related_fields):
         return super(Technique, cls).from_icat(icat_data, required_related_fields)

test/search_api/conftest.py

@@ -41,7 +41,7 @@ def test_search_api_mappings_data():
         },
         "Dataset": {
             "base_icat_entity": "Dataset",
-            "pid": "doi",
+            "pid": ["doi", "id"],
             "title": "name",
             "isPublic": "createTime",
             "creationDate": "createTime",
@@ -55,7 +55,7 @@ def test_search_api_mappings_data():
         },
         "Document": {
             "base_icat_entity": "Investigation",
-            "pid": "doi",
+            "pid": ["doi", "id"],
             "isPublic": "releaseDate",
             "type": "type.name",
             "title": "name",
@@ -80,7 +80,7 @@ def test_search_api_mappings_data():
         },
         "Instrument": {
             "base_icat_entity": "Instrument",
-            "pid": "id",
+            "pid": ["pid", "id"],
             "name": "name",
             "facility": "facility.name",
             "datasets": {"Dataset": "datasetInstruments.dataset"},
@@ -115,13 +115,13 @@ def test_search_api_mappings_data():
         "Sample": {
             "base_icat_entity": "Sample",
             "name": "name",
-            "pid": "pid",
+            "pid": ["pid", "id"],
             "description": "parameters.type.description",
             "datasets": {"Dataset": "datasets"},
         },
         "Technique": {
             "base_icat_entity": "Technique",
-            "pid": "pid",
+            "pid": ["pid", "id"],
             "name": "name",
             "datasets": {"Dataset": "datasetTechniques.dataset"},
         },