Refactor data models and mapper for improved role handling and validation; update tests accordingly

Author: Zalfsten

middleware/sql_to_arc/src/middleware/sql_to_arc/mapper.py

@@ -1,6 +1,5 @@
 """Mapper module to convert database rows to ARCTRL objects."""
 
-import json
 from datetime import datetime
 from typing import Any
 
@@ -116,17 +115,12 @@ def map_contact(row: ContactRow) -> Person:
     """Map a database row to a Person object."""
     # Person(lastName, firstName, midInitials, email, phone, fax, address, affiliation, roles)
 
-    # Parse roles JSON
-    roles_json = row.roles
+    # row.roles is now already a list (it was Json[JsonList] and validated/parsed by Pydantic)
     roles = []
-    if roles_json:
-        try:
-            roles_list = json.loads(roles_json)
-            if isinstance(roles_list, list):
-                for r in roles_list:
-                    roles.append(_make_oa(r.get("term"), r.get("uri"), r.get("version")))
-        except json.JSONDecodeError:
-            pass  # Logger?
+    if row.roles:
+        for r in row.roles:
+            if isinstance(r, dict):
+                roles.append(_make_oa(r.get("term"), r.get("uri"), r.get("version")))
 
     return Person(
         last_name=row.last_name,

middleware/sql_to_arc/src/middleware/sql_to_arc/models.py

@@ -7,11 +7,17 @@
 from types import NoneType
 from typing import Any, get_args, get_origin
 
-from pydantic import BaseModel, ConfigDict, Field, model_validator
+from pydantic import BaseModel, ConfigDict, Field, Json, model_validator
 from pydantic_core import PydanticUndefined
 
 logger = logging.getLogger(__name__)
 
+# JSON types representing the expected structure after parsing
+type JsonList = list[Any]
+
+# Cache for schema-related warnings to avoid per-row logging
+_SCHEMA_WARNING_CACHE: set[tuple[str, str, str]] = set()
+
 
 def spec_field(
     *,
@@ -190,11 +196,14 @@ def _report_validation_issues(
             )
 
         if missing_optional:
-            logger.warning(
-                'Table "%s" is missing optional columns: %s. Using default values.',
-                cls.__name__,
-                ", ".join(missing_optional),
-            )
+            cache_key = (cls.__name__, "missing_optional", ",".join(sorted(missing_optional)))
+            if cache_key not in _SCHEMA_WARNING_CACHE:
+                logger.warning(
+                    'Table "%s" is missing optional columns: %s. Using default values.',
+                    cls.__name__,
+                    ", ".join(missing_optional),
+                )
+                _SCHEMA_WARNING_CACHE.add(cache_key)
 
     @model_validator(mode="before")
     @classmethod
@@ -208,11 +217,15 @@ def validate_row(cls, data: Any) -> Any:
         # 1. Check for extra columns
         extra_columns = sorted(set(row_mapping.keys()) - set(cls.model_fields.keys()))
         if extra_columns:
-            logger.warning(
-                'Table "%s": Input contains extra columns not defined in model: %s. Accepting due to extra="allow".',
-                cls.__name__,
-                ", ".join(extra_columns),
-            )
+            cache_key = (cls.__name__, "extra_columns", ",".join(extra_columns))
+            if cache_key not in _SCHEMA_WARNING_CACHE:
+                logger.warning(
+                    'Table "%s": Input contains extra columns not defined in model: %s. '
+                    'Accepting due to extra="allow".',
+                    cls.__name__,
+                    ", ".join(extra_columns),
+                )
+                _SCHEMA_WARNING_CACHE.add(cache_key)
 
         # 2. Process field values (NULLs, Coercion, Overrides)
         coerced_fields: list[str] = []
@@ -274,7 +287,7 @@ class AssayRow(BaseRow):
 
     identifier: str = spec_field()
     investigation_ref: str = spec_field()
-    study_ref: str | None = spec_field(default=None)
+    study_ref: Json[JsonList] | None = spec_field(default=None)
     title: str | None = spec_field(default=None)
     description_text: str | None = spec_field(default=None)
     measurement_type_term: str | None = spec_field(default=None)
@@ -314,5 +327,5 @@ class ContactRow(BaseRow):
     fax: str | None = spec_field(default=None)
     postal_address: str | None = spec_field(default=None)
     affiliation: str | None = spec_field(default=None)
-    roles: str | None = spec_field(default=None)  # JSON string
+    roles: Json[JsonList] | None = spec_field(default=None)
     target_ref: str | None = spec_field(default=None)

middleware/sql_to_arc/tests/unit/test_database_validation.py

@@ -6,6 +6,7 @@
 
 from middleware.sql_to_arc.database import Database
 from middleware.sql_to_arc.models import (
+    _SCHEMA_WARNING_CACHE,
     BaseRow,
     InvestigationRow,
     spec_field,
@@ -35,6 +36,9 @@ async def test_validate_row_missing_required_aborts(caplog: pytest.LogCaptureFix
 @pytest.mark.asyncio
 async def test_validate_row_missing_optional_warns(caplog: pytest.LogCaptureFixture) -> None:
     """Test that missing optional columns cause a warning but proceed."""
+    # Ensure cache is clear for this test
+    _SCHEMA_WARNING_CACHE.clear()
+
     # submission_date is optional
     row = {"identifier": "1", "title": "Test", "description_text": "Present"}
 

middleware/sql_to_arc/tests/unit/test_mapper.py

@@ -2,13 +2,15 @@
 
 import datetime
 
+import pytest
 from arctrl import (  # type: ignore[import-untyped, import-not-found]
     ArcAssay,
     ArcInvestigation,
     ArcStudy,
     Person,
     Publication,
 )
+from pydantic import ValidationError
 
 from middleware.sql_to_arc.mapper import (
     map_annotation,
@@ -105,7 +107,7 @@ def test_map_assay() -> None:
     """Test mapping of assay data."""
     row = AssayRow(
         identifier="1",
-        study_ref="sty1",
+        study_ref=["sty1"],
         investigation_ref="inv1",
         measurement_type_term="Proteomics",
         measurement_type_uri="http://example.org/prot",
@@ -130,7 +132,7 @@ def test_map_assay_with_platform() -> None:
     """Test mapping of assay data including technology platform."""
     row = AssayRow(
         identifier="2",
-        study_ref="sty1",
+        study_ref=["sty1"],
         investigation_ref="inv1",
         technology_platform="Orbitrap",
     )
@@ -171,7 +173,7 @@ def test_map_contact() -> None:
         last_name="Doe",
         first_name="John",
         email="john@example.com",
-        roles='[{"term": "Principal Investigator", "uri": "http://roles", "version": "1.0"}]',
+        roles=[{"term": "Principal Investigator", "uri": "http://roles", "version": "1.0"}],
     )
 
     person = map_contact(row)
@@ -188,15 +190,13 @@ def test_map_contact() -> None:
 
 def test_map_contact_invalid_roles() -> None:
     """Test mapping of contact data with invalid roles JSON."""
-    row = ContactRow(
-        investigation_ref="inv1",
-        target_type="investigation",
-        last_name="Smith",
-        roles="invalid json string",
-    )
-    person = map_contact(row)
-    assert person.LastName == "Smith"
-    assert person.Roles == []
+    with pytest.raises(ValidationError):
+        ContactRow(
+            investigation_ref="inv1",
+            target_type="investigation",
+            last_name="Smith",
+            roles=None,
+        )
 
 
 def test_map_annotation() -> None: