diff --git a/semantica/export/rdf_exporter.py b/semantica/export/rdf_exporter.py index 6f07d619..ce64a3a2 100644 --- a/semantica/export/rdf_exporter.py +++ b/semantica/export/rdf_exporter.py @@ -187,16 +187,53 @@ def _escape_literal(value: str) -> str: def _escape_xml(value: str) -> str: - return value.replace("&", "&").replace("<", "<").replace(">", ">") + """Escape a string for either XML element text or an attribute value. + + The quotes matter. This helper feeds `rdf:about`, `rdf:resource` and + `xmlns:` attribute values, which are delimited by double quotes, so a value + carrying one would close the attribute early and produce a document that + does not parse. Escaping them in element text as well is harmless and + means one helper cannot be used in the wrong place. + """ + return ( + value.replace("&", "&") + .replace("<", "<") + .replace(">", ">") + .replace('"', """) + .replace("'", "'") + ) + + +def _is_ncname(value: str) -> bool: + """Whether a string can be an XML NCName, which is what RDF/XML requires. + + Checked over the ASCII range rather than the full XML production: the + grammar also admits combining characters and extenders, so this is + deliberately conservative. It refuses names it could have accepted, and it + never accepts one that would produce a document a parser rejects. The + earlier check tested only that the first character was not a digit, which + let through every other way a local name can fail to be a name. + """ + if not value: + return False + if not (value[0].isascii() and (value[0].isalpha() or value[0] == "_")): + return False + return all(c.isascii() and (c.isalnum() or c in "._-") for c in value[1:]) def _split_iri(iri: str) -> Optional[tuple]: - """Split an IRI into (namespace, local name) for RDF/XML's QName syntax.""" + """Split an IRI into (namespace, local name) for RDF/XML's QName syntax. + + Returns None when no split yields a usable local name. RDF/XML is the only + serialization here that cannot write an arbitrary predicate IRI, so this is + the one place a term can be unrepresentable, and the caller reports it + rather than dropping it quietly. + """ for sep in ("#", "/"): index = iri.rfind(sep) if index != -1 and index + 1 < len(iri): local = iri[index + 1 :] - if local and not local[0].isdigit(): + if _is_ncname(local): return iri[: index + 1], local return None @@ -260,7 +297,24 @@ def _typed_literal_parts(term: str, value: Any) -> tuple: if isinstance(value, int): return "literal", str(value), f"{_XSD_NS}integer" if isinstance(value, float): - return "literal", repr(value), f"{_XSD_NS}decimal" + # xsd:double, not xsd:decimal. `repr(1e-05)` is "1e-05" and + # `repr(float("nan"))` is "nan", and xsd:decimal admits neither the + # exponent form nor the special values, so typing a float as decimal + # produced lexicals a strict parser rejects. A Python float is an IEEE + # 754 double; xsd:double has legal lexicals for all of them, and it is + # also the honest claim, since nothing that arrived as a float was ever + # exact. `normalize_confidence` keeps xsd:decimal for confidence + # deliberately: that is a bounded score where exactness is meaningful + # and NaN is not a confidence at all. + if value != value: + lexical = "NaN" + elif value == float("inf"): + lexical = "INF" + elif value == float("-inf"): + lexical = "-INF" + else: + lexical = repr(value) + return "literal", lexical, f"{_XSD_NS}double" return "literal", str(value), None @@ -284,12 +338,28 @@ def _ntriples_metadata_lines(subject: str, statements: List[tuple]) -> List[str] ] -def _rdfxml_metadata_lines(statements: List[tuple], indent: str) -> List[str]: - """RDF/XML needs a QName, so an unprefixed term declares its own prefix.""" +def _rdfxml_metadata_lines( + statements: List[tuple], indent: str, logger: Any = None +) -> List[str]: + """RDF/XML needs a QName, so an unprefixed term declares its own prefix. + + A term with no QName form has no RDF/XML representation at all, and this is + the only serialization with that restriction. Skipping it quietly would + reintroduce, in one format, exactly the silent metadata loss this module + was changed to stop, so it is reported and the other three formats still + carry the statement in full. + """ lines: List[str] = [] for position, (term, value) in enumerate(statements): split = _split_iri(term) if split is None: + if logger is not None: + logger.warning( + "Term %r has no QName form, so it cannot be written in " + "RDF/XML and was omitted from that serialization only. " + "Turtle, N-Triples and JSON-LD carry it in full.", + term, + ) continue namespace, local = split kind, lexical, datatype = _typed_literal_parts(term, value) @@ -881,8 +951,15 @@ class RDFSerializer: confidence = normalize_confidence(entity.get("confidence", 1.0)) # RDF/XML syntax: rdf:Description with rdf:about - lines.append(f' ') - lines.append(f' ') + # Attribute values are delimited by quotes, and both of these + # are caller input. Element text is left alone deliberately: that + # is #1098, and it is being fixed on its own path. + lines.append( + f' ' + ) + lines.append( + f' ' + ) lines.append(f" {text}") if confidence is None: self.logger.warning( @@ -900,6 +977,7 @@ class RDFSerializer: entity.get("metadata"), metadata_terms, self.logger ), " ", + self.logger, ) ) lines.append(" ") @@ -913,8 +991,12 @@ class RDFSerializer: rel_type = rel.get("type", "semantica:related_to") # Relationship as property on source entity - lines.append(f' ') - lines.append(f' <{rel_type} rdf:resource="{target_id}"/>') + lines.append( + f' ' + ) + lines.append( + f' <{rel_type} rdf:resource="{_escape_xml(str(target_id))}"/>' + ) lines.append(" ") lines.append("") @@ -924,6 +1006,7 @@ class RDFSerializer: rdf_data.get("metadata"), metadata_terms, self.logger ), " ", + self.logger, ) if graph_uri else [] diff --git a/tests/export/test_metadata_passthrough.py b/tests/export/test_metadata_passthrough.py index 15fbcb7d..cdb73227 100644 --- a/tests/export/test_metadata_passthrough.py +++ b/tests/export/test_metadata_passthrough.py @@ -98,7 +98,9 @@ def test_every_format_writes_the_same_metadata_triples(): for fmt in FORMATS: g = _serialize(RDFSerializer(), fmt, GRAPH_WITH_METADATA) per_format[fmt] = { - (p, o) for s, p, o in g if str(p).startswith(SEMANTICA_NS) and "numEntities" in str(p) + (p, o) + for s, p, o in g + if str(p).startswith(SEMANTICA_NS) and "numEntities" in str(p) } assert len(set(map(frozenset, per_format.values()))) == 1, per_format @@ -181,7 +183,11 @@ def test_an_iri_valued_key_is_written_as_a_node_not_a_string(): sem:uri: the key names a field, the term names a relation.""" data = { "entities": [ - {"id": ENTITY_IRI, "text": "Acme", "metadata": {"uri": "https://example.org/db"}} + { + "id": ENTITY_IRI, + "text": "Acme", + "metadata": {"uri": "https://example.org/db"}, + } ], "relationships": [], } @@ -195,11 +201,17 @@ def test_an_iri_valued_key_is_written_as_a_node_not_a_string(): def test_output_is_unchanged_when_no_metadata_is_present(): plain = { - "entities": [{"id": ENTITY_IRI, "type": "https://example.org/Org", "text": "Acme"}], - "relationships": [{"source_id": ENTITY_IRI, "target_id": "https://example.org/e2"}], + "entities": [ + {"id": ENTITY_IRI, "type": "https://example.org/Org", "text": "Acme"} + ], + "relationships": [ + {"source_id": ENTITY_IRI, "target_id": "https://example.org/e2"} + ], } serializer = RDFSerializer() - assert serializer.serialize_to_turtle(plain) == serializer.serialize_to_turtle(plain) + assert serializer.serialize_to_turtle(plain) == serializer.serialize_to_turtle( + plain + ) g = _parse(serializer.serialize_to_turtle(plain), "turtle") assert len(g) == 4 @@ -226,3 +238,116 @@ def test_jsonld_metadata_survives_a_real_jsonld_processor(): json.loads(raw) # must be valid JSON before it can be valid JSON-LD g = _parse(raw, "json-ld") assert (URIRef(ENTITY_IRI), NUM_ENTITIES, Literal(3)) in g + + +# --- Findings from the Qodo review of PR #1165 ----------------------------- + + +@pytest.mark.parametrize("fmt", FORMATS) +@pytest.mark.parametrize( + "value", [1e-05, 1e300, 0.1, -0.0, float("nan"), float("inf"), float("-inf")] +) +def test_a_float_metadata_value_is_a_double_and_keeps_a_legal_lexical(fmt, value): + """`repr()` of a float is not an xsd:decimal lexical. + + `repr(1e-05)` is "1e-05" and `repr(float("nan"))` is "nan", neither of which + xsd:decimal admits, so typing a float as decimal produced RDF a strict + parser rejects. A Python float is an IEEE 754 double, xsd:double has legal + lexicals for the exponent form and for the three special values, and saying + double is also the honest claim: nothing here was ever exact. + """ + data = { + "entities": [ + {"id": ENTITY_IRI, "text": "Acme", "metadata": {"num_entities": value}} + ], + "relationships": [], + } + g = _serialize(RDFSerializer(), fmt, data) + objects = list(g.objects(URIRef(ENTITY_IRI), NUM_ENTITIES)) + assert len(objects) == 1, f"{fmt}: {objects}" + (written,) = objects + assert written.datatype == XSD.double, written.datatype + parsed = written.toPython() + if value != value: # NaN + assert parsed != parsed + else: + assert parsed == value + + +def test_every_format_agrees_on_a_float_metadata_value(): + per_format = {} + for fmt in FORMATS: + g = _serialize( + RDFSerializer(), + fmt, + { + "entities": [ + {"id": ENTITY_IRI, "text": "A", "metadata": {"num_entities": 1e-05}} + ], + "relationships": [], + }, + ) + per_format[fmt] = {(p, o) for s, p, o in g if p == NUM_ENTITIES} + assert len(set(map(frozenset, per_format.values()))) == 1, per_format + + +def test_a_term_rdfxml_cannot_name_is_refused_out_loud(caplog): + """RDF/XML needs a QName, and the PR's whole point is no silent drops. + + A term whose local part is not an XML NCName has no RDF/XML form at all. + Skipping it quietly reintroduces, in one format, exactly the loss this + change exists to stop. + """ + unnameable = "http://example.org/ns/123" + data = { + "entities": [ + {"id": ENTITY_IRI, "text": "Acme", "metadata": {"reviewed_by": "fabio"}} + ], + "relationships": [], + } + with caplog.at_level("WARNING"): + xml = RDFSerializer().serialize_to_rdfxml( + data, metadata_terms={"reviewed_by": unnameable} + ) + _parse(xml, "xml") # must still be well-formed + messages = " ".join(r.getMessage() for r in caplog.records) + assert unnameable in messages + assert "RDF/XML" in messages + + +@pytest.mark.parametrize("fmt", ["turtle", "ntriples", "jsonld"]) +def test_the_other_formats_still_carry_a_term_rdfxml_cannot_name(fmt): + """Only RDF/XML has the QName restriction; the rest write the full IRI.""" + unnameable = "http://example.org/ns/123" + data = { + "entities": [ + {"id": ENTITY_IRI, "text": "Acme", "metadata": {"reviewed_by": "fabio"}} + ], + "relationships": [], + } + g = _serialize( + RDFSerializer(), fmt, data, metadata_terms={"reviewed_by": unnameable} + ) + assert (URIRef(ENTITY_IRI), URIRef(unnameable), Literal("fabio")) in g + + +def test_a_quote_in_an_attribute_value_cannot_break_the_document(): + """`_escape_xml` feeds attribute values, which are delimited by quotes. + + Escaping only &, < and > leaves a caller-supplied value able to close the + attribute early and produce XML that does not parse. + """ + data = { + "entities": [ + { + "id": 'https://example.org/e"1', + "text": "Acme", + "metadata": {"uri": 'https://example.org/db"x'}, + } + ], + "relationships": [], + } + xml = RDFSerializer().serialize_to_rdfxml(data) + from xml.dom.minidom import parseString + + parseString(xml) # well-formedness is the assertion