Issue LIF-Initiative#35: Fix MDR bug with required entities not being included in '… (LIF-Initiative#37)

…required' array for schema export

Author: groybal

components/lif/mdr_services/schema_generation_service.py

@@ -38,7 +38,7 @@
 async def find_children(
     tree,
     parent,
-    parent_properties,
+    parent_schema,
     df_entity,
     session,
     include_attr_md,
@@ -49,6 +49,7 @@ async def find_children(
     full_export,
 ):
     if parent in tree:
+        parent_properties = parent_schema["properties"]
         for x in tree[parent]:
             if data_model.Type in ["OrgLIF", "PartnerLIF"]:
                 child_association_query = select(EntityAssociation).where(
@@ -89,11 +90,12 @@ async def find_children(
                     entity_data.UseConsiderations if entity_data.UseConsiderations else ""
                 )  # Using empty string instead of null to make it easier to diff w/ P1 lif.json schema
                 required_elements = []
-                if include_entity_md:
-                    for key, value in entity_data.__dict__.items():
+                for key, value in entity_data.__dict__.items():
+                    if include_entity_md:
                         parent_properties[entity_name][key] = value
-                        if key == "Required" and value == "Yes":
-                            required_elements.append(entity_name)
+                    if key == "Required" and value == "Yes" and entity_name not in parent_schema["required"]:
+                        parent_schema["required"].append(entity_name)
+                if include_entity_md:
                     if full_export:
                         parent_properties[entity_name]["EntityAssociationId"] = child_association.Id
                         parent_properties[entity_name]["EntityAssociationParentEntityId"] = (
@@ -252,7 +254,7 @@ async def find_children(
                 await find_children(
                     tree,
                     x,
-                    parent_properties[entity_name]["properties"],
+                    parent_properties[entity_name],
                     df_entity=df_entity,
                     session=session,
                     include_attr_md=include_attr_md,
@@ -323,13 +325,7 @@ async def find_ancestors(session, child_id, data_model_type, data_model_id, incl
 
 
 async def add_ref(
-    parent_ancestors,
-    child_ancestors,
-    df_entity,
-    parent_entity_name,
-    child_entity_name,
-    openapi_spec,
-    key
+    parent_ancestors, child_ancestors, df_entity, parent_entity_name, child_entity_name, openapi_spec, key
 ):
     """
     Inline the schema for a referenced child entity under the parent's OpenAPI schema entry.
@@ -366,17 +362,19 @@ async def add_ref(
     ref_data = deepcopy(referenced_schema)
     properties = ref_data.get("properties")
     if isinstance(properties, dict):
-        # In LIF, "Reference" is intended to be instantiated as only the required fields of the referenced entity. 
+        # In LIF, "Reference" is intended to be instantiated as only the required fields of the referenced entity.
         # These can be used to look up the full entity.
         required_fields = ref_data.get("required", [])
         if isinstance(required_fields, list) and required_fields:
             required_fields_set = set(required_fields)
             ref_data["properties"] = {
-                prop_name: prop_value for prop_name, prop_value in properties.items() if prop_name in required_fields_set
+                prop_name: prop_value
+                for prop_name, prop_value in properties.items()
+                if prop_name in required_fields_set
             }
         else:
             ref_data["properties"] = {}
-    ref_data["type"] = "object" # A reference should always be to a single object, not an array of objects.
+    ref_data["type"] = "object"  # A reference should always be to a single object, not an array of objects.
     logger.info(f"ref_data : {ref_data}")
 
     if len(parent_ancestors) == 0:
@@ -714,10 +712,11 @@ async def generate_openapi_schema(
                                 ]["ValueSet"]["Values"] = valueset_values_full
 
             openapi_spec["components"]["schemas"][parent_entity.Name]["required"] = required_elements
+
         await find_children(
             tree,
             parent,
-            openapi_spec["components"]["schemas"][parent_entity.Name]["properties"],
+            openapi_spec["components"]["schemas"][parent_entity.Name],
             session=session,
             df_entity=df_entity,
             include_attr_md=include_attr_md,
@@ -818,7 +817,7 @@ async def generate_openapi_schema(
             parent_entity_name=parent_entity_name,
             child_entity_name=child_entity_name,
             openapi_spec=openapi_spec,
-            key=key
+            key=key,
         )
 
     if "Common" in openapi_spec["components"]["schemas"]:

test/bases/lif/mdr_restapi/data_model_example_datasource_full_openapi_schema.json

@@ -12,7 +12,8 @@
         "type": "object",
         "required": [
           "person",
-          "id"
+          "id",
+          "employment"
         ],
         "use_recommendations": "",
         "Description": null,
@@ -70,7 +71,7 @@
           },
           "employment": {
             "type": "object",
-            "required": [],
+            "required": ["preferences"],
             "use_recommendations": "",
             "Description": null,
             "ActivationDate": "2025-10-08T22:12:00+00:00",
@@ -109,7 +110,6 @@
               "preferences": {
                 "type": "object",
                 "required": [
-                  "preferences"
                 ],
                 "use_recommendations": "",
                 "Description": null,

test/components/lif/mdr_client/test_core.py

@@ -252,7 +252,7 @@ def _assert_openapi_data_model_results_from_file(data_model: dict):
     assert data_model["openapi"] == "3.0.0"
     assert data_model["components"]["schemas"] != {}
     assert "Person" in data_model["components"]["schemas"]
-    assert "Organization" in data_model["components"]["schemas"]
+    assert "OrganizationCode" in data_model["components"]["schemas"]
 
 
 def _assert_openapi_data_model_results_from_mdr(data_model: dict):

test/components/lif/mdr_services/test_schema_generation_service.py

@@ -44,6 +44,30 @@ def first(self):
         return self._obj
 
 
+class _Attr:
+    def __init__(self, id, name, required):
+        self.Id = id
+        self.Name = name
+        self.Required = required
+        self.DataType = "string"
+        self.Array = "No"
+        self.ValueSetId = None
+
+    def dict(self):
+        return {
+            "Description": None,
+            "UseConsiderations": None,
+            "Example": None,
+            "Format": None,
+            "Id": self.Id,
+            "Name": self.Name,
+            "Required": self.Required,
+            "DataType": self.DataType,
+            "Array": self.Array,
+            "ValueSetId": self.ValueSetId,
+        }
+
+
 @pytest.fixture
 def fake_session():
     s = MagicMock()
@@ -170,7 +194,9 @@ async def test_add_ref_nested_parent_path():
         key="ChildRef",
     )
 
-    parent_props = openapi_spec["components"]["schemas"]["Root"]["properties"]["Intermediate"]["properties"]["Parent"]["properties"]
+    parent_props = openapi_spec["components"]["schemas"]["Root"]["properties"]["Intermediate"]["properties"]["Parent"][
+        "properties"
+    ]
     child_ref_inline = parent_props["ChildRef"]
     assert child_ref_inline["type"] == "object"
     assert set(child_ref_inline["properties"].keys()) == {"a", "b"}
@@ -344,24 +370,6 @@ def get_entity_by_id_side_effect(session=None, id=None, **_):
 
     monkeypatch.setattr(svc, "get_entity_by_id", AsyncMock(side_effect=get_entity_by_id_side_effect))
 
-    class _Attr:
-        def __init__(self, id, name, required):
-            self.Id = id
-            self.Name = name
-            self.Required = required
-            self.DataType = "string"
-            self.Array = "No"
-            self.ValueSetId = None
-
-        def dict(self):
-            return {
-                "Description": None,
-                "UseConsiderations": None,
-                "Example": None,
-                "Format": None,
-                "DataType": self.DataType,
-            }
-
     def get_attributes_with_association_metadata_for_entity_side_effect(entity_id, **_):
         if entity_id == 101:  # CompetencyFramework
             return [_Attr(1, "uri", "Yes"), _Attr(2, "name", "Yes"), _Attr(3, "description", "No")]
@@ -405,6 +413,79 @@ def get_attributes_with_association_metadata_for_entity_side_effect(entity_id, *
     assert assoc_schema["required"] == ["competencyFrameworkId", "competencyFrameworkType"]
 
 
+async def test_generate_openapi_schema_has_sub_entity_required_entity(fake_session, monkeypatch):
+    """
+    Given:
+      - BaseLIF CompetencyFramework entity with child entity Association in EntityAssociations
+      - The Association Entity has Required="Yes" in EntityAssociations
+    Result:
+      - The generated schema for CompetencyFramework includes Association with its required fields listed
+    """
+    dm = types.SimpleNamespace(
+        Id=1,
+        Name="BaseLIF",
+        DataModelVersion="3.0",
+        Type="BaseLIF",
+        BaseDataModelId=None,
+        ContributorOrganization="LIF",
+    )
+    monkeypatch.setattr(svc, "get_datamodel_by_id", AsyncMock(return_value=dm))
+
+    def get_entity_by_id_side_effect(session=None, id=None, **_):
+        if id == 101:
+            return types.SimpleNamespace(
+                Id=101, Name="CompetencyFramework", Array="No", UseConsiderations=None, Deleted=False
+            )
+        elif id == 202:
+            return types.SimpleNamespace(
+                Id=202, Name="Association", Array="Yes", UseConsiderations=None, Deleted=False, Required="Yes"
+            )
+        else:
+            raise ValueError(f"Unexpected entity_id {id}")
+
+    monkeypatch.setattr(svc, "get_entity_by_id", AsyncMock(side_effect=get_entity_by_id_side_effect))
+
+    def get_attributes_with_association_metadata_for_entity_side_effect(entity_id, **_):
+        if entity_id == 101:  # CompetencyFramework
+            return [_Attr(1, "uri", "Yes"), _Attr(2, "name", "Yes"), _Attr(3, "description", "No")]
+        elif entity_id == 202:  # Association
+            return [_Attr(1, "uri", "Yes"), _Attr(2, "name", "Yes"), _Attr(3, "description", "No")]
+        else:
+            raise ValueError(f"Unexpected entity_id {entity_id}")
+
+    monkeypatch.setattr(
+        svc,
+        "get_attributes_with_association_metadata_for_entity",
+        AsyncMock(side_effect=get_attributes_with_association_metadata_for_entity_side_effect),
+    )
+
+    RowIN = namedtuple("RowIN", ["Id", "Name"])
+    fake_session.execute.side_effect = [
+        # get embedded entity associations
+        _FetchallResult([(101, 202)]),
+        # get entities in DM not in union of associations
+        _FetchallResult([]),
+        # build df_entity (include both entities)
+        _FetchallResult([RowIN(101, "CompetencyFramework"), RowIN(202, "Association")]),
+        # find_children: get detailed association rows via .scalars().all()
+        _ScalarListResult([types.SimpleNamespace(Relationship=None)]),
+        # enums for attributes (none)
+        _FetchallResult([]),
+        _FetchallResult([]),
+        # inter-entity "Reference" links
+        _FetchallResult([]),
+    ]
+
+    out = await svc.generate_openapi_schema(
+        fake_session, data_model_id=1, include_attr_md=False, include_entity_md=False
+    )
+    cf_schema = out["components"]["schemas"]["CompetencyFramework"]
+    assert cf_schema["type"] == "object"
+    assert "Association" in cf_schema["required"]
+    assert "uri" in cf_schema["required"]
+    assert "name" in cf_schema["required"]
+
+
 async def test_generate_openapi_schema_full_export(fake_session, monkeypatch):
     """
     Given: