Add robust validation for list-type fields #2714

data-serving/reusable-data-service/data_service/day_zero_fields.json

@@ -53,6 +53,21 @@
             "other"
         ]
     },
+    {
+        "key": "gender",
+        "type": "string",
+        "data_dictionary_text": "Gender identity of the individual; can select multiple options.",
+        "required": true,
+        "values": [
+            "man",
+            "woman",
+            "transgender",
+            "non-binary",
+            "other",
+            "unknown"
+        ],
+        "is_list": true
+    },
     {
         "key": "confirmationDate",
         "type": "date",

data-serving/reusable-data-service/data_service/model/document.py

@@ -127,7 +127,7 @@ def field_values(self) -> List[str]:
                     fields += value.custom_field_values()
                 else:
                     fields += f.type.custom_none_field_values()
-            elif issubclass(f.type, list):
+            elif isinstance(value, list):
                 fields.append(",".join(value))
             else:
                 fields.append(str(value) if value is not None else "")
@@ -203,11 +203,17 @@ def validate(self):
                 raise ValidationError(f"{field.key} must have a value")
             if field.key in self.document_fields() and value is not None:
                 getter(self).validate()
+            if field.is_list:
+                for element in value:
+                    if not isinstance(element, field.element_type()):
+                        raise ValidationError(f"{field.key} member {element} is of the wrong type")
             if field.values is not None:
-                if value is not None and value not in field.values:
-                    raise ValidationError(
-                        f"{field.key} value {value} not in permissible values {field.values}"
-                    )
+                test_collection = value if field.is_list is True else [value]
+                for a_value in test_collection:
+                    if a_value is not None and a_value not in field.values:
+                        raise ValidationError(
+                            f"{field.key} value {a_value} not in permissible values {field.values}"
+                        )
 
     def _internal_set_value(self, key, value):
         self._internal_ensure_containers_exist(key)

data-serving/reusable-data-service/data_service/model/field.py

@@ -12,13 +12,16 @@
 
 @dataclasses.dataclass
 class Field(Document):
-    """Represents a custom field in a Document object."""
+    """Represents a custom field in a Document object.
+    Note for future architects: I learned today that dataclasses fields can carry
+    custom metadata. It might make sense to reorganise things so that information
+    currently in the Field class is carried in that metadata instead."""
 
     key: str = dataclasses.field(init=True, default=None)
     type: str = dataclasses.field(init=True, default=None)
     data_dictionary_text: str = dataclasses.field(init=True, default=None)
     required: bool = dataclasses.field(init=True, default=False)
-    default: Optional[Union[bool, str, int, date]] = dataclasses.field(
+    default: Optional[Union[bool, str, int, date, Feature, AgeRange, CaseReference, CaseExclusionMetadata]] = dataclasses.field(
         init=True, default=None
     )
     values: Optional[List[Any]] = dataclasses.field(init=True, default=None)
@@ -63,7 +66,10 @@ def python_type(self) -> type:
         if self.is_list:
             return list
         else:
-            return self.model_type(self.type)
+            return self.element_type()
+
+    def element_type(self) -> type:
+        return self.model_type(self.type)
 
     def dataclasses_tuples(self):
         # Note that the default value here is always None, even if I have a default value!

data-serving/reusable-data-service/tests/data/case.excluded.json

@@ -9,5 +9,6 @@
     },
     "caseStatus": "omit_error",
     "pathogenStatus": "endemic",
-    "sexAtBirth": "female"
+    "sexAtBirth": "female",
+    "gender": ["woman"]
 }

data-serving/reusable-data-service/tests/data/case.minimal.json

@@ -5,5 +5,6 @@
     },
     "caseStatus": "probable",
     "pathogenStatus": "emerging",
-    "sexAtBirth": "female"
+    "sexAtBirth": "female",
+    "gender": []
 }

data-serving/reusable-data-service/tests/data/case.with_location.json

@@ -18,5 +18,7 @@
     },
     "caseStatus": "probable",
     "pathogenStatus": "unknown",
-    "sexAtBirth": "female"
+    "sexAtBirth": "female",
+    "gender": ["non-binary", "other"],
+    "gender_other": "womxn"
 }

data-serving/reusable-data-service/tests/test_case_model.py

@@ -86,3 +86,19 @@ def test_apply_update_that_unsets_value():
     update = DocumentUpdate.from_dict({"confirmationDate": None})
     case.apply_update(update)
     assert case.confirmationDate is None
+
+
+def test_cannot_put_wrong_type_in_list():
+    with open("./tests/data/case.minimal.json", "r") as minimal_file:
+        case = Case.from_json(minimal_file.read())
+    case.gender = ["man", True]
+    with pytest.raises(ValidationError):
+        case.validate()
+
+
+def test_list_elements_must_come_from_acceptable_values():
+    with open("./tests/data/case.minimal.json", "r") as minimal_file:
+        case = Case.from_json(minimal_file.read())
+    case.gender = ["woman", "dalek"]
+    with pytest.raises(ValidationError):
+        case.validate()