diff --git a/reqif/models/reqif_relation_group.py b/reqif/models/reqif_relation_group.py index d5e2960..07b3bd7 100644 --- a/reqif/models/reqif_relation_group.py +++ b/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 , exactly + # like a SPEC-OBJECT. + self.values: Optional[List[SpecObjectAttribute]] = values self.is_self_closed: bool = is_self_closed diff --git a/reqif/models/reqif_spec_relation.py b/reqif/models/reqif_spec_relation.py index fb7103d..072f936 100644 --- a/reqif/models/reqif_spec_relation.py +++ b/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 + # , 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] diff --git a/reqif/parsers/attribute_value_parser.py b/reqif/parsers/attribute_value_parser.py index 1249f3c..16bd9bc 100644 --- a/reqif/parsers/attribute_value_parser.py +++ b/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: diff --git a/reqif/parsers/relation_group_parser.py b/reqif/parsers/relation_group_parser.py index 6231b60..182735a 100644 --- a/reqif/parsers/relation_group_parser.py +++ b/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 += " \n" for spec_relation in relation_group.spec_relations: diff --git a/reqif/parsers/spec_relation_parser.py b/reqif/parsers/spec_relation_parser.py index 7f6cd38..970f78c 100644 --- a/reqif/parsers/spec_relation_parser.py +++ b/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) diff --git a/tests/unit/reqif/parsers/test_relation_group_parser.py b/tests/unit/reqif/parsers/test_relation_group_parser.py new file mode 100644 index 0000000..4573744 --- /dev/null +++ b/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 = """ + + + + + DEF_STRING + + + + + DEF_INTEGER + + + + + TEST_RELATION_GROUP_TYPE_ID + + +""" # noqa: E501 + + +def test_01_nominal_case() -> None: + relation_group_string = """ + + + TEST_SPEC_RELATION_ID + + + TEST_RELATION_GROUP_TYPE_ID + + + SPECIFICATION_A + + + SPECIFICATION_B + + + """ # 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("") < unparsed.index("") + + +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 = """ + + + TEST_RELATION_GROUP_TYPE_ID + + + """ # 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 "" in unparsed diff --git a/tests/unit/reqif/parsers/test_spec_relation_parser.py b/tests/unit/reqif/parsers/test_spec_relation_parser.py index 68f9c7d..e77f4b2 100644 --- a/tests/unit/reqif/parsers/test_spec_relation_parser.py +++ b/tests/unit/reqif/parsers/test_spec_relation_parser.py @@ -26,3 +26,106 @@ def test_01_nominal_case() -> None: assert spec_object.relation_type_ref == "PARENT" assert spec_object.source == "SPEC_OBJECT_A" assert spec_object.target == "SPEC_OBJECT_B" + assert spec_object.values is None + assert spec_object.values_attribute is None + + +def test_02_multiple_attribute_values_survive_round_trip() -> None: + spec_relation_string = """ + + + + + DEF_STRING + + + + + DEF_INTEGER + + + + + DEF_BOOLEAN + + + + + PARENT + + + SPEC_OBJECT_A + + + SPEC_OBJECT_B + + + """ # noqa: E501 + + spec_relation_xml = ElementTree.fromstring(spec_relation_string) + spec_relation = SpecRelationParser.parse(spec_relation_xml) + + # All three values are kept. Previously only the first was parsed, and + # BOOLEAN raised NotImplementedError. + assert spec_relation.values is not None + assert len(spec_relation.values) == 3 + assert [value.definition_ref for value in spec_relation.values] == [ + "DEF_STRING", + "DEF_INTEGER", + "DEF_BOOLEAN", + ] + # Values are kept as the raw strings from THE-VALUE, booleans included. + assert [value.value for value in spec_relation.values] == [ + "String value", + "42", + "true", + ] + + unparsed = SpecRelationParser.unparse(spec_relation) + assert "DEF_STRING" in unparsed + assert "DEF_INTEGER" in unparsed + assert "DEF_BOOLEAN" in unparsed + + +def test_03_values_attribute_stays_backward_compatible() -> None: + spec_relation_string = """ + + + + + DEF_ONE + + + + + DEF_TWO + + + + + PARENT + + + SPEC_OBJECT_A + + + SPEC_OBJECT_B + + + """ # noqa: E501 + + spec_relation_xml = ElementTree.fromstring(spec_relation_string) + spec_relation = SpecRelationParser.parse(spec_relation_xml) + + # The deprecated accessor still reads the first value. + assert spec_relation.values_attribute is not None + assert spec_relation.values_attribute.definition_ref == "DEF_ONE" + + # And writing through it still replaces the values wholesale. + spec_relation.values_attribute = spec_relation.values[1] + assert spec_relation.values is not None + assert len(spec_relation.values) == 1 + assert spec_relation.values[0].definition_ref == "DEF_TWO" + + spec_relation.values_attribute = None + assert spec_relation.values is None