feat: keep SPEC-RELATION and RELATION-GROUP attribute values

reqif/models/reqif_relation_group.py

@@ -1,5 +1,7 @@
 from typing import List, Optional
 
+from reqif.models.reqif_spec_object import SpecObjectAttribute
+
 
 class ReqIFRelationGroup:  # pylint: disable=too-many-instance-attributes
     def __init__(  # pylint: disable=too-many-arguments
@@ -12,6 +14,7 @@ def __init__(  # pylint: disable=too-many-arguments
         source_specification_ref: Optional[str] = None,
         target_specification_ref: Optional[str] = None,
         spec_relations: Optional[List[str]] = None,
+        values: Optional[List[SpecObjectAttribute]] = None,
         is_self_closed: bool = True,
         alternative_id: Optional[str] = None,
     ):
@@ -24,4 +27,7 @@ def __init__(  # pylint: disable=too-many-arguments
         self.source_specification_ref: Optional[str] = source_specification_ref
         self.target_specification_ref: Optional[str] = target_specification_ref
         self.spec_relations: Optional[List[str]] = spec_relations
+        # A RELATION-GROUP may carry attribute values under <VALUES>, exactly
+        # like a SPEC-OBJECT.
+        self.values: Optional[List[SpecObjectAttribute]] = values
         self.is_self_closed: bool = is_self_closed

reqif/models/reqif_spec_relation.py

@@ -1,4 +1,4 @@
-from typing import Any, Optional
+from typing import Any, List, Optional
 
 from reqif.helpers.debug import auto_described
 from reqif.models.reqif_spec_object import SpecObjectAttribute
@@ -16,9 +16,15 @@ def __init__(  # pylint: disable=too-many-arguments
         description: Optional[str] = None,
         last_change: Optional[str] = None,
         long_name: Optional[str] = None,
+        values: Optional[List[SpecObjectAttribute]] = None,
         values_attribute: Optional[SpecObjectAttribute] = None,
         alternative_id: Optional[str] = None,
     ):
+        if values is not None and values_attribute is not None:
+            raise ValueError(
+                "ReqIFSpecRelation: pass either values or the deprecated "
+                "values_attribute, not both."
+            )
         self.identifier: str = identifier
         self.alternative_id: Optional[str] = alternative_id
         self.relation_type_ref: str = relation_type_ref
@@ -29,4 +35,25 @@ def __init__(  # pylint: disable=too-many-arguments
         self.description: Optional[str] = description
         self.last_change: Optional[str] = last_change
         self.long_name: Optional[str] = long_name
-        self.values_attribute: Optional[SpecObjectAttribute] = values_attribute
+
+        # A SPEC-RELATION may carry any number of attribute values under
+        # <VALUES>, exactly like a SPEC-OBJECT.
+        self.values: Optional[List[SpecObjectAttribute]] = values
+        if values_attribute is not None:
+            self.values = [values_attribute]
+
+    @property
+    def values_attribute(self) -> Optional[SpecObjectAttribute]:
+        """Deprecated: the first attribute value, or None.
+
+        SPEC-RELATION used to be modelled as carrying at most one attribute
+        value. Use .values instead — this discards every value after the first.
+        """
+        if self.values is None or len(self.values) == 0:
+            return None
+        return self.values[0]
+
+    @values_attribute.setter
+    def values_attribute(self, value: Optional[SpecObjectAttribute]) -> None:
+        """Deprecated: replaces all attribute values with a single one."""
+        self.values = None if value is None else [value]

reqif/parsers/attribute_value_parser.py

@@ -199,7 +199,7 @@ def parse_attribute_values(
     @staticmethod
     def unparse_attribute_values(
         attribute_values: Optional[List[SpecObjectAttribute]],
-    ):
+    ) -> str:
         if attribute_values is None:
             return ""
         if len(attribute_values) == 0:

reqif/parsers/relation_group_parser.py

@@ -1,7 +1,12 @@
+import logging
 from typing import List, Optional
 
 from reqif.models.reqif_relation_group import ReqIFRelationGroup
+from reqif.models.reqif_spec_object import SpecObjectAttribute
 from reqif.parsers.alternative_id_parser import AlternativeIDParser
+from reqif.parsers.attribute_value_parser import AttributeValueParser
+
+logger = logging.getLogger(__name__)
 
 
 class ReqIFRelationGroupParser:
@@ -30,6 +35,12 @@ def parse(xml_relation_group) -> ReqIFRelationGroup:
             attributes["LONG-NAME"] if "LONG-NAME" in attributes else None
         )
 
+        values: Optional[List[SpecObjectAttribute]] = (
+            AttributeValueParser.parse_attribute_values(
+                xml_relation_group.find("VALUES")
+            )
+        )
+
         spec_relations: Optional[List[str]] = None
         xml_spec_relations = xml_relation_group.find("SPEC-RELATIONS")
         if xml_spec_relations is not None:
@@ -68,6 +79,7 @@ def parse(xml_relation_group) -> ReqIFRelationGroup:
             source_specification_ref=source_specification_ref,
             target_specification_ref=target_specification_ref,
             spec_relations=spec_relations,
+            values=values,
             is_self_closed=False,
         )
 
@@ -91,6 +103,36 @@ def unparse(relation_group: ReqIFRelationGroup) -> str:
             relation_group.alternative_id, "          "
         )
 
+        # Emitted first, mirroring SPEC-OBJECT's ["VALUES", "TYPE"] child order.
+        # Ordering is not the reason for the position: RELATION-GROUP is an
+        # xsd:all, so its children are unordered.
+        #
+        # The bundled reqif.xsd does not permit VALUES on RELATION-GROUP at all,
+        # although the ReqIF spec PDF's MOF model (10.8.33) defines it. That is a
+        # known defect of the ReqIF XML schema, called out as such in the prostep
+        # ivip ReqIF Implementation Guide v1.8, section 2.11, whose advice is
+        # addressed to whoever authors the content.
+        #
+        # Writing the element is therefore a deliberate deviation from the
+        # schema, and whether to ship such a file is the caller's call to make:
+        # they know which tools will read the output. So warn rather than refuse,
+        # naming the group so the caller can find it. There is no matching
+        # warning on the parse side, because sections 2.1, 2.3 and 2.13 of the
+        # same guide ask importing tools to be forgiving about what they read and
+        # keep the strictness on the export side.
+        if relation_group.values is not None and len(relation_group.values) > 0:
+            logger.warning(
+                "RELATION-GROUP %s: writing a VALUES element, which the ReqIF "
+                "XML schema does not permit on RELATION-GROUP (a known schema "
+                "defect; see prostep ivip ReqIF Implementation Guide v1.8, "
+                "section 2.11). Other ReqIF tools may reject this file or drop "
+                "these values silently. Silence this by raising the level of "
+                "the %s logger.",
+                relation_group.identifier,
+                __name__,
+            )
+        output += AttributeValueParser.unparse_attribute_values(relation_group.values)
+
         if relation_group.spec_relations is not None:
             output += "          <SPEC-RELATIONS>\n"
             for spec_relation in relation_group.spec_relations:

reqif/parsers/spec_relation_parser.py

@@ -6,7 +6,6 @@
 from reqif.models.reqif_spec_relation import (
     ReqIFSpecRelation,
 )
-from reqif.models.reqif_types import SpecObjectAttributeType
 from reqif.parsers.alternative_id_parser import AlternativeIDParser
 from reqif.parsers.attribute_value_parser import AttributeValueParser
 
@@ -55,35 +54,13 @@ def parse(xml_spec_relation) -> ReqIFSpecRelation:
             xml_spec_relation.find("TARGET").find("SPEC-OBJECT-REF").text
         )
 
-        values_attribute: Optional[SpecObjectAttribute] = None
+        # Delegate to the shared helper rather than hand-rolling the loop: it
+        # keeps every value instead of only the first, and it covers all seven
+        # ATTRIBUTE-VALUE-* kinds rather than STRING/INTEGER/XHTML alone.
         xml_values = xml_spec_relation.find("VALUES")
-        if xml_values is not None:
-            for xml_value in xml_values:
-                if xml_value.tag == "ATTRIBUTE-VALUE-STRING":
-                    attribute_value = xml_value.attrib["THE-VALUE"]
-                    definition_ref = xml_value[0][0].text
-                    values_attribute = SpecObjectAttribute(
-                        xml_node=xml_value,
-                        attribute_type=SpecObjectAttributeType.STRING,
-                        definition_ref=definition_ref,
-                        value=attribute_value,
-                    )
-                elif xml_value.tag == "ATTRIBUTE-VALUE-XHTML":
-                    values_attribute = AttributeValueParser.parse_xhtml_attribute_value(
-                        xml_value
-                    )
-                elif xml_value.tag == "ATTRIBUTE-VALUE-INTEGER":
-                    attribute_value = xml_value.attrib["THE-VALUE"]
-                    definition_ref = xml_value[0][0].text
-                    values_attribute = SpecObjectAttribute(
-                        xml_node=xml_value,
-                        attribute_type=SpecObjectAttributeType.INTEGER,
-                        definition_ref=definition_ref,
-                        value=attribute_value,
-                    )
-                else:
-                    raise NotImplementedError
-                break
+        values: Optional[List[SpecObjectAttribute]] = (
+            AttributeValueParser.parse_attribute_values(xml_values)
+        )
 
         spec_relation = ReqIFSpecRelation(
             alternative_id=AlternativeIDParser.parse(xml_spec_relation),
@@ -95,7 +72,7 @@ def parse(xml_spec_relation) -> ReqIFSpecRelation:
             relation_type_ref=relation_type_ref,
             source=spec_relation_source,
             target=spec_relation_target,
-            values_attribute=values_attribute,
+            values=values,
         )
         return spec_relation
 
@@ -132,10 +109,9 @@ def unparse(spec_relation: ReqIFSpecRelation):
                     spec_relation
                 )
             elif tag == "VALUES":
-                if spec_relation.values_attribute is not None:
-                    output += AttributeValueParser.unparse_attribute_values(
-                        [spec_relation.values_attribute]
-                    )
+                output += AttributeValueParser.unparse_attribute_values(
+                    spec_relation.values
+                )
             else:
                 raise NotImplementedError(tag)
 

tests/unit/reqif/parsers/test_relation_group_parser.py

@@ -0,0 +1,139 @@
+import logging
+from xml.etree import ElementTree
+
+import pytest
+
+from reqif.parsers.relation_group_parser import (
+    ReqIFRelationGroupParser,
+)
+
+RELATION_GROUP_WITH_VALUES = """
+<RELATION-GROUP IDENTIFIER="TEST_RELATION_GROUP_ID">
+  <VALUES>
+    <ATTRIBUTE-VALUE-STRING THE-VALUE="String value">
+      <DEFINITION>
+        <ATTRIBUTE-DEFINITION-STRING-REF>DEF_STRING</ATTRIBUTE-DEFINITION-STRING-REF>
+      </DEFINITION>
+    </ATTRIBUTE-VALUE-STRING>
+    <ATTRIBUTE-VALUE-INTEGER THE-VALUE="42">
+      <DEFINITION>
+        <ATTRIBUTE-DEFINITION-INTEGER-REF>DEF_INTEGER</ATTRIBUTE-DEFINITION-INTEGER-REF>
+      </DEFINITION>
+    </ATTRIBUTE-VALUE-INTEGER>
+  </VALUES>
+  <TYPE>
+    <RELATION-GROUP-TYPE-REF>TEST_RELATION_GROUP_TYPE_ID</RELATION-GROUP-TYPE-REF>
+  </TYPE>
+</RELATION-GROUP>
+"""  # noqa: E501
+
+
+def test_01_nominal_case() -> None:
+    relation_group_string = """
+<RELATION-GROUP IDENTIFIER="TEST_RELATION_GROUP_ID" LONG-NAME="Relation group">
+  <SPEC-RELATIONS>
+    <SPEC-RELATION-REF>TEST_SPEC_RELATION_ID</SPEC-RELATION-REF>
+  </SPEC-RELATIONS>
+  <TYPE>
+    <RELATION-GROUP-TYPE-REF>TEST_RELATION_GROUP_TYPE_ID</RELATION-GROUP-TYPE-REF>
+  </TYPE>
+  <SOURCE-SPECIFICATION>
+    <SPECIFICATION-REF>SPECIFICATION_A</SPECIFICATION-REF>
+  </SOURCE-SPECIFICATION>
+  <TARGET-SPECIFICATION>
+    <SPECIFICATION-REF>SPECIFICATION_B</SPECIFICATION-REF>
+  </TARGET-SPECIFICATION>
+</RELATION-GROUP>
+    """  # noqa: E501
+
+    relation_group_xml = ElementTree.fromstring(relation_group_string)
+    relation_group = ReqIFRelationGroupParser.parse(relation_group_xml)
+    assert relation_group.identifier == "TEST_RELATION_GROUP_ID"
+    assert relation_group.spec_relations == ["TEST_SPEC_RELATION_ID"]
+    assert relation_group.values is None
+
+    # A RELATION-GROUP without VALUES must not grow one on unparse.
+    assert "VALUES" not in ReqIFRelationGroupParser.unparse(relation_group)
+
+
+def test_02_attribute_values_survive_round_trip() -> None:
+    relation_group_xml = ElementTree.fromstring(RELATION_GROUP_WITH_VALUES)
+    relation_group = ReqIFRelationGroupParser.parse(relation_group_xml)
+
+    assert relation_group.values is not None
+    assert len(relation_group.values) == 2
+    assert [value.definition_ref for value in relation_group.values] == [
+        "DEF_STRING",
+        "DEF_INTEGER",
+    ]
+    assert [value.value for value in relation_group.values] == ["String value", "42"]
+
+    unparsed = ReqIFRelationGroupParser.unparse(relation_group)
+    assert "DEF_STRING" in unparsed
+    assert "DEF_INTEGER" in unparsed
+    # VALUES is emitted first, as SPEC-OBJECT does. RELATION-GROUP is an
+    # xsd:all, so child order carries no meaning; this just pins the output.
+    assert unparsed.index("<VALUES>") < unparsed.index("<TYPE>")
+
+
+LOGGER_NAME = "reqif.parsers.relation_group_parser"
+
+
+def test_03_writing_values_warns_once_per_group(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    relation_group = ReqIFRelationGroupParser.parse(
+        ElementTree.fromstring(RELATION_GROUP_WITH_VALUES)
+    )
+
+    # Parsing is silent: importing tools should be forgiving about what they read.
+    assert caplog.records == []
+
+    with caplog.at_level(logging.WARNING, logger=LOGGER_NAME):
+        ReqIFRelationGroupParser.unparse(relation_group)
+
+    # One warning for the group, naming it, whatever the number of values.
+    warnings = [
+        record for record in caplog.records if record.levelno == logging.WARNING
+    ]
+    assert len(warnings) == 1
+    message = warnings[0].getMessage()
+    assert "TEST_RELATION_GROUP_ID" in message
+    assert "2.11" in message
+
+
+def test_04_no_warning_without_values(caplog: pytest.LogCaptureFixture) -> None:
+    relation_group_string = """
+<RELATION-GROUP IDENTIFIER="TEST_RELATION_GROUP_ID">
+  <TYPE>
+    <RELATION-GROUP-TYPE-REF>TEST_RELATION_GROUP_TYPE_ID</RELATION-GROUP-TYPE-REF>
+  </TYPE>
+</RELATION-GROUP>
+    """  # noqa: E501
+
+    relation_group = ReqIFRelationGroupParser.parse(
+        ElementTree.fromstring(relation_group_string)
+    )
+    with caplog.at_level(logging.WARNING, logger=LOGGER_NAME):
+        ReqIFRelationGroupParser.unparse(relation_group)
+    assert caplog.records == []
+
+
+def test_05_warning_is_suppressible(caplog: pytest.LogCaptureFixture) -> None:
+    """A caller who has decided to write these values can silence the warning
+    through the standard logging hierarchy, without any reqif-specific API."""
+    relation_group = ReqIFRelationGroupParser.parse(
+        ElementTree.fromstring(RELATION_GROUP_WITH_VALUES)
+    )
+
+    logger = logging.getLogger(LOGGER_NAME)
+    original_level = logger.level
+    logger.setLevel(logging.ERROR)
+    try:
+        unparsed = ReqIFRelationGroupParser.unparse(relation_group)
+    finally:
+        logger.setLevel(original_level)
+
+    # Warning suppressed, output unchanged.
+    assert caplog.records == []
+    assert "<VALUES>" in unparsed