From ad2e8dec5ab072d07273634ff1dadfd566aa0a31 Mon Sep 17 00:00:00 2001 From: Kyoung Min Kim Date: Fri, 16 Oct 2026 14:00:24 +0900 Subject: [PATCH 1/3] feat(validation): cross-check a mapping document against the documents it references A mapping document names an ontology and a semantic model by iri, but validate.py checked only its own shape, so a mapping whose references name the wrong document, whose concepts are not in the ontology, or whose expressions name fields the model does not declare still passed. Read each iri as a URI reference and follow its path from the mapping file's own directory, as ontology.md says. Check that the document found is named as the reference says, that every concept the mapping names is declared there or built in, and that every dataset.field an expression names is a declared field of the model. A dataset that declares no fields list is not checked, since the core spec does not require one. A reference with no iri, with a scheme, or with a relative path and no location to resolve against is left to a catalog and reported as a warning, so documents that only a catalog can resolve still pass. Generated-by: Claude Code --- validation/tests/test_ontology.py | 305 ++++++++++++++++++++++++++++++ validation/tests/test_validate.py | 2 +- validation/validate.py | 186 +++++++++++++++++- 3 files changed, 489 insertions(+), 4 deletions(-) diff --git a/validation/tests/test_ontology.py b/validation/tests/test_ontology.py index 93db12f4..5e53c7cb 100644 --- a/validation/tests/test_ontology.py +++ b/validation/tests/test_ontology.py @@ -434,3 +434,308 @@ def test_mapping_document_accepts_both_sides_of_each_pair(offline, mapping_schem ) assert not _VALIDATE.validate_schema(document, mapping_schema) + + +# --------------------------------------------------------------------------- +# Mapping documents: cross-checks against the referenced ontology and model. +# --------------------------------------------------------------------------- + + +def _sales_ontology() -> dict: + return { + "version": "0.2.0.dev0", + "name": "sales", + "ontology": [ + {"concept": "Order", "type": "EntityType"}, + {"concept": "Customer", "type": "EntityType"}, + ], + } + + +def _sales_model() -> dict: + expression = {"dialects": [{"dialect": "ANSI_SQL", "expression": "order_id"}]} + return { + "version": "0.2.0.dev0", + "name": "sales_model", + "datasets": [ + { + "name": "orders", + "source": "sales.public.orders", + "fields": [ + {"name": "order_id", "expression": expression}, + {"name": "customer_id", "expression": expression}, + ], + } + ], + } + + +def _write_mapping_set(tmp_path, mapping=None, ontology=None, model=None) -> dict: + """Write the three documents next to each other and return the mapping.""" + mapping = _mapping_document() if mapping is None else mapping + (tmp_path / "sales.ontology.yaml").write_text( + yaml.safe_dump(_sales_ontology() if ontology is None else ontology) + ) + (tmp_path / "sales.semantic_model.yaml").write_text( + yaml.safe_dump(_sales_model() if model is None else model) + ) + return mapping + + +def _cross_check(tmp_path, mapping_schema, mapping) -> list[str]: + return _VALIDATE.validate_document(mapping, mapping_schema, tmp_path) + + +def test_split_flights_mapping_cross_checks_against_its_documents(offline, mapping_schema): + mapping = yaml.safe_load((_REPO / "examples/flights.mapping.yaml").read_text()) + + assert _VALIDATE.validate_document(mapping, mapping_schema, _REPO / "examples") == [] + + +def test_mapping_cross_checks_accept_resolving_references(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + + assert _cross_check(tmp_path, mapping_schema, mapping) == [] + + +@pytest.mark.parametrize("reference", ["ontology_ref", "semantic_model_ref"]) +def test_mapping_reference_must_name_the_document_it_resolves_to( + offline, mapping_schema, tmp_path, reference +): + mapping = _write_mapping_set(tmp_path) + mapping[reference]["name"] = "something_else" + mapping["concept_mappings"][0]["concept"] = "Ordr" + iri = mapping[reference]["iri"] + actual = {"ontology_ref": "sales", "semantic_model_ref": "sales_model"}[reference] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + # The misnamed document is still the one the mapping points at, so the + # checks against its content run as well. + assert errors == [ + f"[Reference] {reference} names 'something_else' but the document at iri '{iri}' " + f"is named '{actual}'", + "[Reference] concept_mappings -> 0: concept 'Ordr' is not declared in the ontology", + ] + + +def test_mapping_concepts_must_be_declared_in_the_ontology(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"] = [ + {"concept": "Ordr", "object_mappings": [{"expression": "orders.order_id"}]}, + { + "concept": "Order", + "link_mappings": [ + { + "object_mapping": {"expression": "orders.order_id"}, + "relationship": "placed_by", + "children": [ + { + "object_mapping": { + "concept": "Custmer", + "expression": "orders.customer_id", + } + }, + { + "object_mapping": { + "concept": "Integer", + "expression": "orders.customer_id", + } + }, + ], + } + ], + }, + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ + "[Reference] concept_mappings -> 0: concept 'Ordr' is not declared in the ontology", + "[Reference] concept_mappings -> 1 -> link_mappings -> 0 -> children -> 0 -> object_mapping: " + "concept 'Custmer' is not declared in the ontology", + ] + + +def test_mapping_expressions_must_name_declared_fields(offline, mapping_schema, tmp_path): + model = _sales_model() + # The core spec does not require a fields list, so a dataset without one + # cannot be checked and is left alone. + model["datasets"].append({"name": "customers", "source": "sales.public.customers"}) + mapping = _write_mapping_set(tmp_path, model=model) + mapping["concept_mappings"][0]["object_mappings"] = [ + {"expression": "orders.order_id"}, + {"expression": "orders.order_nr + orders.order_nr"}, + {"expression": "ORDERS.order_id"}, + {"expression": "CONCAT(orders.order_id, '.', orders.customer_id)"}, + {"expression": "CASE WHEN orders.order_id > 0 THEN 'a.b' END"}, + {"expression": "customers.anything"}, + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ + "[Reference] concept_mappings -> 0 -> object_mappings -> 1: expression names " + "'orders.order_nr' but dataset 'orders' declares no field 'order_nr'", + "[Reference] concept_mappings -> 0 -> object_mappings -> 2: expression names " + "'ORDERS.order_id' but the semantic model declares no dataset 'ORDERS'", + ] + + +@pytest.mark.parametrize( + "expression, expected", + [ + ("orders.order_id -- see orders.nope", [("orders", "order_id")]), + ("/* orders.nope */ orders.order_id", [("orders", "order_id")]), + ('"a.b" + orders.order_id', [("orders", "order_id")]), + ("`a.b` + orders.order_id", [("orders", "order_id")]), + ("'it''s a.b' || orders.order_id", [("orders", "order_id")]), + ("cat.sch.orders.order_id", []), + ("sch.fn(orders.order_id)", [("orders", "order_id")]), + ("orders.order_id + orders.order_id", [("orders", "order_id")]), + ("1.5 * orders.order_id", [("orders", "order_id")]), + ("orders.order_id > 0 AND orders.amount > 1.5", [("orders", "order_id"), ("orders", "amount")]), + ], +) +def test_field_references_read_two_part_identifiers_only(expression, expected): + assert _VALIDATE.field_references(expression) == expected + + +def test_mapping_expressions_in_referent_mappings_are_checked(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["object_mappings"] = [ + { + "referent_mappings": [ + { + "relationship": "customer", + "referent_mappings": [ + {"relationship": "nr", "expression": "orders.customer_nr"} + ], + } + ] + } + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ + "[Reference] concept_mappings -> 0 -> object_mappings -> 0 -> referent_mappings -> 0 " + "-> referent_mappings -> 0: expression names 'orders.customer_nr' but dataset 'orders' " + "declares no field 'customer_nr'" + ] + + +@pytest.mark.parametrize( + "mutate, expected", + [ + ( + lambda ref: ref.pop("iri"), + "ontology_ref 'sales' has no iri, so only a catalog can resolve it", + ), + ( + lambda ref: ref.update(iri="https://example.org/sales.ontology.yaml"), + "ontology_ref iri 'https://example.org/sales.ontology.yaml' is not a path reference", + ), + ], +) +def test_mapping_reference_this_validator_does_not_follow_is_a_warning( + offline, mapping_schema, tmp_path, mutate, expected +): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["concept"] = "Ordr" + mapping["concept_mappings"][0]["object_mappings"] = [{"expression": "orders.order_nr"}] + mutate(mapping["ontology_ref"]) + + diagnostics = _cross_check(tmp_path, mapping_schema, mapping) + + # The ontology half is skipped with a warning, the model half still runs. + assert [type(d) for d in diagnostics] == [_VALIDATE.ValidationWarning, str] + assert diagnostics[0].startswith(f"[Reference] Warning: {expected}") + assert diagnostics[0].endswith("cross-document checks for ontology_ref skipped") + assert "declares no field 'order_nr'" in diagnostics[1] + + +def test_mapping_without_a_location_skips_cross_checks_with_a_warning(offline, mapping_schema): + diagnostics = _VALIDATE.validate_document(_mapping_document(), mapping_schema) + + assert [type(d) for d in diagnostics] == [_VALIDATE.ValidationWarning] * 2 + assert all(d.startswith("[Reference] Warning: ") for d in diagnostics) + assert all("has no location to resolve it against" in d for d in diagnostics) + + +def test_mapping_iri_is_read_as_a_uri_reference(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + (tmp_path / "sales ontology.yaml").write_text(yaml.safe_dump(_sales_ontology())) + mapping["ontology_ref"]["iri"] = "./sales%20ontology.yaml#concepts" + mapping["semantic_model_ref"]["iri"] = "sales.semantic_model.yaml?v=1" + + assert _cross_check(tmp_path, mapping_schema, mapping) == [] + + +def test_mapping_absolute_path_iri_is_followed_without_a_location(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["ontology_ref"]["iri"] = str(tmp_path / "sales.ontology.yaml") + mapping["semantic_model_ref"]["iri"] = str(tmp_path / "sales.semantic_model.yaml") + mapping["concept_mappings"][0]["concept"] = "Ordr" + + errors = _VALIDATE.validate_document(mapping, mapping_schema) + + assert errors == [ + "[Reference] concept_mappings -> 0: concept 'Ordr' is not declared in the ontology" + ] + + +def test_mapping_reference_to_the_wrong_kind_of_document_is_reported_once( + offline, mapping_schema, tmp_path +): + mapping = _write_mapping_set(tmp_path) + mapping["ontology_ref"] = {"name": "sales_model", "iri": "./sales.semantic_model.yaml"} + mapping["concept_mappings"].append( + {"concept": "Customer", "object_mappings": [{"expression": "orders.customer_id"}]} + ) + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + # One line for the document, not one per concept the mapping names. + assert errors == [ + "[Reference] ontology_ref iri './sales.semantic_model.yaml' does not hold an ontology " + "document, so its concepts cannot be checked" + ] + + +@pytest.mark.parametrize( + "content, expected", + [ + (None, "does not resolve: No such file or directory"), + ("a: [", "is not valid YAML"), + ("- just\n- a list\n", "does not hold a document"), + ], +) +def test_mapping_reference_that_does_not_read_as_a_document_is_an_error( + offline, mapping_schema, tmp_path, content, expected +): + mapping = _write_mapping_set(tmp_path) + target = tmp_path / "sales.semantic_model.yaml" + if content is None: + target.unlink() + else: + target.write_text(content) + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert len(errors) == 1 + assert not isinstance(errors[0], _VALIDATE.ValidationWarning) + assert errors[0].startswith("[Reference] semantic_model_ref iri './sales.semantic_model.yaml' ") + assert expected in errors[0] + + +def test_validate_file_resolves_mapping_references_from_the_file_location( + offline, mapping_schema, tmp_path, capsys +): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["object_mappings"] = [{"expression": "orders.order_nr"}] + mapping_path = tmp_path / "sales.mapping.yaml" + mapping_path.write_text(yaml.safe_dump(mapping)) + + assert _VALIDATE.validate_file(mapping_path, mapping_schema) is False + assert "declares no field 'order_nr'" in capsys.readouterr().out diff --git a/validation/tests/test_validate.py b/validation/tests/test_validate.py index 5226526c..e2ad61b3 100644 --- a/validation/tests/test_validate.py +++ b/validation/tests/test_validate.py @@ -1073,7 +1073,7 @@ def test_cli_reports_a_check_that_raises_and_goes_on(tmp_path, monkeypatch, caps good = _write_document(tmp_path, "good.json", _document([_CUSTOMERS], [])) bad = _write_document(tmp_path, "bad.json", _document([_ORDERS], [])) - def explode(data, schema): + def explode(data, schema, base=None): if "orders" in json.dumps(data): raise KeyError("dialect") return [] diff --git a/validation/validate.py b/validation/validate.py index 548bf127..f8a3ac33 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -40,6 +40,11 @@ concept and relationship names, and for extends, identify_by, roles and QName prefixes that resolve to something declared. +Mapping documents (--schema ontology/mapping.json) are cross-checked against the +ontology and semantic model each iri points at: the reference names the document +it resolves to, every concept the mapping names is declared there or built in, +and every dataset.field an expression names is a declared field of the model. + Usage: python validation/validate.py [--schema ] [--] [ ...] python validation/validate.py --schema ontology/ontology.json @@ -54,11 +59,13 @@ import json import os +import re import sys import traceback from collections import Counter from collections.abc import Hashable from pathlib import Path +from urllib.parse import unquote, urlsplit try: import yaml @@ -617,6 +624,172 @@ def inherited_relationships(concept: str) -> set[str]: return errors +_QUOTED_OR_COMMENT = re.compile( + r"'(?:[^']|'')*'" # string literal + r'|"(?:[^"]|"")*"' # quoted identifier + r"|`[^`]*`" # backtick identifier + r"|--[^\n]*" # line comment + r"|/\*.*?\*/", # block comment + re.DOTALL, +) +_FIELD_REFERENCE = re.compile(r"(? list[tuple[str, str]]: + """The dataset.field pairs a mapping expression names, each once. + + Mapping expressions reach the semantic model through two-part identifiers + (ontology.md, Object mappings). String literals, quoted identifiers and + comments are blanked first, so a dot inside them is not a reference, and + neither is a longer dotted chain or a schema-qualified function call. + Quoted identifiers are therefore not resolved. + """ + text = _QUOTED_OR_COMMENT.sub(" ", expression) + return list(dict.fromkeys(_FIELD_REFERENCE.findall(text))) + + +def resolve_document_reference( + reference: dict, base: Path | None, label: str +) -> tuple[dict | None, list[str]]: + """Follow a mapping document's DocumentReference and return the document. + + The iri is read as a URI reference. One with a scheme or an authority is + left to a catalog, and the path of any other is followed, relative to + base, the mapping document's own directory, as ontology.md specifies. A + reference with no iri, with a scheme, or with a relative path and no base + comes back as None with a ValidationWarning. A path that does not read as + a YAML document is an error. So is a document not named as the reference + says, but that document is still returned for the checks that follow. + """ + name = reference.get("name") + iri = reference.get("iri") + skipped = f"cross-document checks for {label} skipped" + if not isinstance(iri, str) or not iri: + return None, [ValidationWarning( + f"[Reference] Warning: {label} '{name}' has no iri, so only a catalog can " + f"resolve it: {skipped}" + )] + parts = urlsplit(iri) + if parts.scheme or parts.netloc: + return None, [ValidationWarning( + f"[Reference] Warning: {label} iri '{iri}' is not a path reference, so it is " + f"not read: {skipped}" + )] + path = Path(unquote(parts.path)) + if not path.is_absolute(): + if base is None: + return None, [ValidationWarning( + f"[Reference] Warning: {label} iri '{iri}' is relative but the mapping " + f"document has no location to resolve it against: {skipped}" + )] + path = base / path + + try: + text = path.read_text(encoding="utf-8") + except OSError as e: + return None, [f"[Reference] {label} iri '{iri}' does not resolve: {e.strerror or e}"] + except UnicodeDecodeError: + return None, [f"[Reference] {label} iri '{iri}' is not valid UTF-8 text"] + try: + document = load_yaml_named(text, str(path)) + except yaml.YAMLError as e: + return None, [f"[Reference] {label} iri '{iri}' is not valid YAML: {str(e).split(chr(10))[0]}"] + except RecursionError: + return None, [f"[Reference] {label} iri '{iri}' is too deeply nested to parse"] + if not isinstance(document, dict): + return None, [f"[Reference] {label} iri '{iri}' does not hold a document"] + + errors = [] + if document.get("name") != name: + errors.append( + f"[Reference] {label} names '{name}' but the document at iri '{iri}' is named " + f"'{document.get('name', '')}'" + ) + return document, errors + + +def validate_mapping(data: dict, base: Path | None) -> list[str]: + """Cross-check a mapping document against the two documents it references. + + ontology_ref and semantic_model_ref must name the documents they point at, + every concept the mapping names must be declared in that ontology or be a + built-in concept, and every dataset.field an expression names must be a + field of that semantic model. A dataset that declares no fields list is not + checked, since the core spec does not require one. Relative iri are read + from base, the mapping document's own directory. A reference that cannot + be followed skips its half of the checks with a warning, see + resolve_document_reference. + """ + if not isinstance(data, dict) or "concept_mappings" not in data: + return [] + + errors = [] + ontology_ref = data.get("ontology_ref") or {} + model_ref = data.get("semantic_model_ref") or {} + ontology, diagnostics = resolve_document_reference(ontology_ref, base, "ontology_ref") + errors.extend(diagnostics) + model, diagnostics = resolve_document_reference(model_ref, base, "semantic_model_ref") + errors.extend(diagnostics) + + # None means that half of the checks is off: the document was not followed + # or is not the kind of document the reference promises. + concepts = None + if ontology is not None: + if isinstance(ontology.get("ontology"), list): + concepts = { + c.get("concept") for c in ontology["ontology"] if isinstance(c, dict) + } | BUILT_IN_CONCEPTS + else: + errors.append( + f"[Reference] ontology_ref iri '{ontology_ref.get('iri')}' does not hold an " + f"ontology document, so its concepts cannot be checked" + ) + fields_by_dataset = None + if model is not None: + if isinstance(model.get("datasets"), list): + fields_by_dataset = { + d.get("name"): ( + {f.get("name") for f in d["fields"] if isinstance(f, dict)} + if isinstance(d.get("fields"), list) + else None + ) + for d in model["datasets"] + if isinstance(d, dict) + } + else: + errors.append( + f"[Reference] semantic_model_ref iri '{model_ref.get('iri')}' does not hold a " + f"semantic model document, so its fields cannot be checked" + ) + + def visit(node: object, path: str) -> None: + if isinstance(node, dict): + concept = node.get("concept") + if concepts is not None and isinstance(concept, str) and concept not in concepts: + errors.append(f"[Reference] {path}: concept '{concept}' is not declared in the ontology") + expression = node.get("expression") + if fields_by_dataset is not None and isinstance(expression, str): + for dataset, field in field_references(expression): + if dataset not in fields_by_dataset: + errors.append( + f"[Reference] {path}: expression names '{dataset}.{field}' but " + f"the semantic model declares no dataset '{dataset}'" + ) + elif fields_by_dataset[dataset] is not None and field not in fields_by_dataset[dataset]: + errors.append( + f"[Reference] {path}: expression names '{dataset}.{field}' but " + f"dataset '{dataset}' declares no field '{field}'" + ) + for key, value in node.items(): + visit(value, f"{path} -> {key}") + elif isinstance(node, list): + for index, item in enumerate(node): + visit(item, f"{path} -> {index}") + + visit(data["concept_mappings"], "concept_mappings") + return errors + + USAGE = "Usage: python validation/validate.py [--schema ] [--] [ ...]" @@ -669,8 +842,13 @@ def load_schema_or_report(schema_path: Path) -> dict | None: return schema -def validate_document(data, schema: dict) -> list[str]: - """Run every check on a parsed document and return the diagnostics.""" +def validate_document(data, schema: dict, base: Path | None = None) -> list[str]: + """Run every check on a parsed document and return the diagnostics. + + base is the directory the document was read from. A mapping document + follows its relative references from there, and without it those checks + are skipped with a warning. + """ errors = [] errors.extend(validate_schema(data, schema)) @@ -683,6 +861,8 @@ def validate_document(data, schema: dict) -> list[str]: errors.extend(validate_sql(data)) if not errors and isinstance(data, dict) and "ontology" in data: errors.extend(validate_ontology(data)) + if not errors and isinstance(data, dict) and "concept_mappings" in data: + errors.extend(validate_mapping(data, base)) return errors @@ -707,7 +887,7 @@ def validate_file(yaml_path: Path, schema: dict) -> bool: return False try: - errors = validate_document(data, schema) + errors = validate_document(data, schema, yaml_path.parent) except Exception as e: # A check that raises on this document is a bug in the validator, not in the # model. Report it like any other failure so the remaining files still run, From 47997692d6113ab6fc4f87681a7eed05885d3624 Mon Sep 17 00:00:00 2001 From: Kyoung Min Kim Date: Sat, 17 Oct 2026 06:44:40 +0900 Subject: [PATCH 2/3] feat(validation): resolve the relationships a mapping names on its concept A referent mapping names a relationship of the concept it maps, and a link mapping names one of the concept mapping's concept, but the cross-checks read only the concept and expression of each node, so a relationship the ontology does not declare passed. Resolve each relationship on its concept or on a concept it extends, the way validate_ontology resolves identify_by. Nested referent mappings resolve on the concept that plays the last role of the enclosing relationship, and a relationship with no such role cannot carry them. A link mapping's children map the objects in the last position of their relationship's links unless their object mapping names a concept, and a link mapping's level has to match the arity of the relationship it names, as ontology.md describes. A name written as Concept.name names the concept being mapped or one it extends, as in the spec examples. Generated-by: Claude Code --- validation/tests/test_ontology.py | 248 +++++++++++++++++++++++++++++- validation/validate.py | 181 ++++++++++++++++++++-- 2 files changed, 417 insertions(+), 12 deletions(-) diff --git a/validation/tests/test_ontology.py b/validation/tests/test_ontology.py index 5e53c7cb..435ed039 100644 --- a/validation/tests/test_ontology.py +++ b/validation/tests/test_ontology.py @@ -442,12 +442,43 @@ def test_mapping_document_accepts_both_sides_of_each_pair(offline, mapping_schem def _sales_ontology() -> dict: + def relationship(name: str, concept: str) -> dict: + return { + "name": name, + "roles": [{"concept": concept}], + "verbalizes": [f"{{_}} has {{{concept}}}"], + } + return { "version": "0.2.0.dev0", "name": "sales", "ontology": [ - {"concept": "Order", "type": "EntityType"}, - {"concept": "Customer", "type": "EntityType"}, + { + "concept": "Party", + "type": "EntityType", + "relationships": [relationship("nr", "Integer")], + }, + # Customer inherits nr from Party. + { + "concept": "Customer", + "type": "EntityType", + "extends": ["Party"], + "identify_by": ["nr"], + }, + { + "concept": "Order", + "type": "EntityType", + "identify_by": ["nr"], + "relationships": [ + relationship("nr", "Integer"), + relationship("customer", "Customer"), + # Named at a link mapping root in the undeclared-concept test, so unary. + {"name": "placed_by", "verbalizes": ["{Order} is placed"]}, + relationship("total", "Decimal"), + # Unary: a link at level 1, and nothing for a referent to resolve on. + {"name": "active", "verbalizes": ["{Order} is active"]}, + ], + }, ], } @@ -625,6 +656,219 @@ def test_mapping_expressions_in_referent_mappings_are_checked(offline, mapping_s ] +def test_sales_ontology_fixture_is_a_valid_ontology(): + assert _VALIDATE.validate_ontology(_sales_ontology()) == [] + + +def test_mapping_relationships_resolve_on_their_concept(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"] = [ + { + "concept": "Order", + "object_mappings": [ + { + "referent_mappings": [ + {"relationship": "nr", "expression": "orders.order_id"}, + { + # Resolves on Customer, which inherits nr from Party. + "relationship": "customer", + "referent_mappings": [ + {"relationship": "Party.nr", "expression": "orders.customer_id"} + ], + }, + ] + } + ], + "link_mappings": [ + { + # The root maps Order itself, at level 1 for a unary relationship. + "object_mapping": { + "referent_mappings": [ + {"relationship": "nr", "expression": "orders.order_id"} + ] + }, + "relationship": "active", + "children": [ + { + "object_mapping": { + "concept": "Decimal", + "expression": "orders.order_id", + }, + "relationship": "total", + }, + { + # Maps the Customer in the last position of customer links. + "object_mapping": { + "referent_mappings": [ + {"relationship": "nr", "expression": "orders.customer_id"} + ] + }, + "relationship": "customer", + }, + ], + } + ], + } + ] + + assert _cross_check(tmp_path, mapping_schema, mapping) == [] + + +def test_mapping_relationships_must_be_declared_on_their_concept(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"] = [ + { + "concept": "Order", + "object_mappings": [ + { + "referent_mappings": [ + {"relationship": "nmbr", "expression": "orders.order_id"}, + { + "relationship": "customer", + "referent_mappings": [ + {"relationship": "code", "expression": "orders.customer_id"} + ], + }, + ] + } + ], + "link_mappings": [ + { + # The root maps Order even though its own relationship does not resolve. + "object_mapping": { + "referent_mappings": [ + {"relationship": "nmbr", "expression": "orders.order_id"} + ] + }, + "relationship": "placed", + "children": [ + { + # Maps a Customer through the customer relationship's role. + "object_mapping": { + "referent_mappings": [ + {"relationship": "code", "expression": "orders.customer_id"} + ], + }, + "relationship": "customer", + }, + { + # An undeclared concept is reported once, by the concept check. + "object_mapping": { + "concept": "Custmer", + "referent_mappings": [ + {"relationship": "nr", "expression": "orders.customer_id"} + ], + }, + "relationship": "customer", + }, + ], + } + ], + }, + { + "concept": "Ordr", + "object_mappings": [ + {"referent_mappings": [{"relationship": "nr", "expression": "orders.order_id"}]} + ], + }, + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ + "[Reference] concept_mappings -> 0 -> link_mappings -> 0 -> children -> 1 " + "-> object_mapping: concept 'Custmer' is not declared in the ontology", + "[Reference] concept_mappings -> 1: concept 'Ordr' is not declared in the ontology", + "[Reference] concept_mappings -> 0 -> object_mappings -> 0 -> referent_mappings -> 0: " + "relationship 'nmbr' is not declared on concept 'Order' or a concept it extends", + "[Reference] concept_mappings -> 0 -> object_mappings -> 0 -> referent_mappings -> 1 " + "-> referent_mappings -> 0: relationship 'code' is not declared on concept 'Customer' " + "or a concept it extends", + "[Reference] concept_mappings -> 0 -> link_mappings -> 0: relationship 'placed' is not " + "declared on concept 'Order' or a concept it extends", + "[Reference] concept_mappings -> 0 -> link_mappings -> 0 -> object_mapping " + "-> referent_mappings -> 0: relationship 'nmbr' is not declared on concept 'Order' or a " + "concept it extends", + "[Reference] concept_mappings -> 0 -> link_mappings -> 0 -> children -> 0 " + "-> object_mapping -> referent_mappings -> 0: relationship 'code' is not declared on " + "concept 'Customer' or a concept it extends", + ] + + +def test_mapping_relationship_prefix_names_the_mapped_concept(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["object_mappings"] = [ + { + "referent_mappings": [ + {"relationship": "Custmer.nr", "expression": "orders.order_id"}, + { + "relationship": "customer", + "referent_mappings": [ + # Order is neither Customer nor a concept Customer extends. + {"relationship": "Order.nr", "expression": "orders.customer_id"} + ], + }, + {"relationship": "Order.nmbr", "expression": "orders.order_id"}, + ] + } + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ + "[Reference] concept_mappings -> 0 -> object_mappings -> 0 -> referent_mappings -> 0: " + "relationship 'Custmer.nr' names concept 'Custmer', which is not declared in the ontology", + "[Reference] concept_mappings -> 0 -> object_mappings -> 0 -> referent_mappings -> 1 " + "-> referent_mappings -> 0: relationship 'Order.nr' names concept 'Order' but this " + "mapping maps 'Customer'", + "[Reference] concept_mappings -> 0 -> object_mappings -> 0 -> referent_mappings -> 2: " + "relationship 'Order.nmbr' is not declared on concept 'Order' or a concept it extends", + ] + + +def test_mapping_nested_referents_need_a_role_to_resolve_on(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["object_mappings"] = [ + { + "referent_mappings": [ + { + "relationship": "active", + "referent_mappings": [{"relationship": "nr", "expression": "orders.order_id"}], + } + ] + } + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ + "[Reference] concept_mappings -> 0 -> object_mappings -> 0 -> referent_mappings -> 0: " + "relationship 'active' has no role for its referent_mappings to resolve on", + ] + + +def test_mapping_link_level_matches_relationship_arity(offline, mapping_schema, tmp_path): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["link_mappings"] = [ + { + "object_mapping": {"expression": "orders.order_id"}, + "relationship": "customer", + "children": [ + {"object_mapping": {"expression": "orders.order_id"}, "relationship": "active"} + ], + } + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ + "[Reference] concept_mappings -> 0 -> link_mappings -> 0: relationship 'customer' has " + "arity 2 but is named at level 1", + "[Reference] concept_mappings -> 0 -> link_mappings -> 0 -> children -> 0: relationship " + "'active' has arity 1 but is named at level 2", + ] + + @pytest.mark.parametrize( "mutate, expected", [ diff --git a/validation/validate.py b/validation/validate.py index f8a3ac33..2faa99ab 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -43,7 +43,10 @@ Mapping documents (--schema ontology/mapping.json) are cross-checked against the ontology and semantic model each iri points at: the reference names the document it resolves to, every concept the mapping names is declared there or built in, -and every dataset.field an expression names is a declared field of the model. +every relationship a referent or link mapping names is declared on its concept +or inherited through extends, with a role for nested referents to resolve on and +at the link level its arity calls for, and every dataset.field an expression +names is a declared field of the model. Usage: python validation/validate.py [--schema ] [--] [ ...] @@ -708,17 +711,61 @@ def resolve_document_reference( return document, errors +class OntologyRelationships: + """The relationships an ontology declares, looked up through extends. + + A name resolves on the concept or on a concept it extends, the way + validate_ontology resolves identify_by. + """ + + def __init__(self, components: list[dict]): + self._declared: dict[str, dict[str, dict]] = {} + self._supertypes: dict[str, list[str]] = {} + for component in components: + concept = component.get("concept") + if not isinstance(concept, str): + continue + self._declared.setdefault(concept, {}).update( + (r["name"], r) + for r in as_list(component.get("relationships")) + if isinstance(r, dict) and isinstance(r.get("name"), str) + ) + self._supertypes.setdefault(concept, []).extend( + s for s in as_list(component.get("extends")) if isinstance(s, str) + ) + + def ancestors(self, concept: str) -> list[str]: + """The concept followed by every concept it extends, each once.""" + found, pending = [], [concept] + while pending: + current = pending.pop() + if current in found: + continue + found.append(current) + pending.extend(self._supertypes.get(current, [])) + return found + + def find(self, concept: str, name: str) -> dict | None: + for current in self.ancestors(concept): + if name in self._declared.get(current, {}): + return self._declared[current][name] + return None + + def validate_mapping(data: dict, base: Path | None) -> list[str]: """Cross-check a mapping document against the two documents it references. ontology_ref and semantic_model_ref must name the documents they point at, every concept the mapping names must be declared in that ontology or be a - built-in concept, and every dataset.field an expression names must be a - field of that semantic model. A dataset that declares no fields list is not - checked, since the core spec does not require one. Relative iri are read - from base, the mapping document's own directory. A reference that cannot - be followed skips its half of the checks with a warning, see - resolve_document_reference. + built-in concept, every relationship a referent or link mapping names must + be declared on the concept whose mapping names it, directly or through + extends, with a role for nested referents to resolve on and at the link + level its arity calls for, and every dataset.field an expression names + must be a field of that semantic model. + A dataset that declares no fields list is not checked, since the core spec + does not require one. Relative iri are read from base, the mapping + document's own directory. A reference that cannot be followed skips its + half of the checks with a warning, see resolve_document_reference. """ if not isinstance(data, dict) or "concept_mappings" not in data: return [] @@ -734,11 +781,12 @@ def validate_mapping(data: dict, base: Path | None) -> list[str]: # None means that half of the checks is off: the document was not followed # or is not the kind of document the reference promises. concepts = None + relationships = None if ontology is not None: if isinstance(ontology.get("ontology"), list): - concepts = { - c.get("concept") for c in ontology["ontology"] if isinstance(c, dict) - } | BUILT_IN_CONCEPTS + components = [c for c in ontology["ontology"] if isinstance(c, dict)] + concepts = {c.get("concept") for c in components} | BUILT_IN_CONCEPTS + relationships = OntologyRelationships(components) else: errors.append( f"[Reference] ontology_ref iri '{ontology_ref.get('iri')}' does not hold an " @@ -787,6 +835,119 @@ def visit(node: object, path: str) -> None: visit(item, f"{path} -> {index}") visit(data["concept_mappings"], "concept_mappings") + if relationships is not None: + errors.extend( + mapping_relationship_errors(data["concept_mappings"], concepts, relationships) + ) + return errors + + +def mapping_relationship_errors( + concept_mappings: object, concepts: set, relationships: OntologyRelationships +) -> list[str]: + """Report the relationships a mapping names that its ontology does not declare. + + A referent mapping names a relationship of the concept being mapped, and its + nested referent mappings name relationships of the concept that plays the + last role of that relationship. A link mapping names a relationship of the + concept mapping's concept, at the level that matches the relationship's + arity. Its root maps that concept and a child maps the objects in the last + position of its relationship's links, and an object mapping that names a + concept itself maps that concept instead, at the root as well (ontology.md, + Object mappings and Link mappings). A relationship written as Concept.name + names the concept being mapped or one it extends, as in the spec examples. + """ + errors = [] + + def role_concept(relationship: dict | None) -> str | None: + roles = as_list(relationship.get("roles")) if relationship else [] + last = roles[-1] if roles else None + concept = last.get("concept") if isinstance(last, dict) else None + return concept if isinstance(concept, str) else None + + def resolve(concept: str | None, name: object, path: str) -> dict | None: + if concept is None or not isinstance(name, str): + return None + written = name + if "." in name: + prefix, name = name.split(".", 1) + if prefix not in concepts: + errors.append( + f"[Reference] {path}: relationship '{written}' names concept '{prefix}', " + f"which is not declared in the ontology" + ) + return None + if prefix not in relationships.ancestors(concept): + errors.append( + f"[Reference] {path}: relationship '{written}' names concept '{prefix}' but " + f"this mapping maps '{concept}'" + ) + return None + concept = prefix + relationship = relationships.find(concept, name) + if relationship is None: + errors.append( + f"[Reference] {path}: relationship '{written}' is not declared on concept " + f"'{concept}' or a concept it extends" + ) + return relationship + + def check_referents(items: object, concept: str | None, path: str) -> None: + for index, referent in enumerate(as_list(items)): + if not isinstance(referent, dict): + continue + here = f"{path} -> {index}" + relationship = resolve(concept, referent.get("relationship"), here) + nested = as_list(referent.get("referent_mappings")) + target = role_concept(relationship) + if relationship is not None and nested and target is None: + errors.append( + f"[Reference] {here}: relationship '{referent['relationship']}' has no role " + f"for its referent_mappings to resolve on" + ) + check_referents(nested, target, f"{here} -> referent_mappings") + + def check_object_mapping(node: object, concept: str | None, path: str) -> None: + if not isinstance(node, dict): + return + named = node.get("concept") + if isinstance(named, str): + # An undeclared concept is reported by the concept check alone. + if named not in concepts: + return + concept = named + check_referents(node.get("referent_mappings"), concept, f"{path} -> referent_mappings") + + def check_link_mapping(node: object, concept: str, path: str, level: int) -> None: + if not isinstance(node, dict): + return + relationship = resolve(concept, node.get("relationship"), path) + if relationship is not None: + arity = len(as_list(relationship.get("roles"))) + 1 + if arity != level: + errors.append( + f"[Reference] {path}: relationship '{node['relationship']}' has arity " + f"{arity} but is named at level {level}" + ) + object_concept = concept if level == 1 else role_concept(relationship) + check_object_mapping( + node.get("object_mapping"), object_concept, f"{path} -> object_mapping" + ) + for index, child in enumerate(as_list(node.get("children"))): + check_link_mapping(child, concept, f"{path} -> children -> {index}", level + 1) + + for index, concept_mapping in enumerate(as_list(concept_mappings)): + if not isinstance(concept_mapping, dict): + continue + concept = concept_mapping.get("concept") + # An undeclared concept is reported by the concept check and has nothing to resolve on. + if not isinstance(concept, str) or concept not in concepts: + continue + path = f"concept_mappings -> {index}" + for j, node in enumerate(as_list(concept_mapping.get("object_mappings"))): + check_object_mapping(node, concept, f"{path} -> object_mappings -> {j}") + for j, node in enumerate(as_list(concept_mapping.get("link_mappings"))): + check_link_mapping(node, concept, f"{path} -> link_mappings -> {j}", level=1) return errors From 48846b3bc052e2f961ce7dcfd3230b81b15b4b8e Mon Sep 17 00:00:00 2001 From: Kyoung Min Kim Date: Sun, 18 Oct 2026 17:36:14 +0900 Subject: [PATCH 3/3] fix(validation): parse mapping expressions and schema-check the documents a mapping references The dataset.field check read mapping expressions with a regular expression, which took any two-part name for a reference. A type name in CAST or ::, a lambda parameter and a schema-qualified function all came back as datasets the model does not declare. The expression is now parsed with sqlglot and the references are the two-part columns of the tree, so those shapes are left alone, a column inside a subquery is left to that query's scope, and quoted identifiers are resolved too. Mapping expressions declare no dialect, so one that does not parse as a SQL expression, or that sqlglot reads as a statement, skips the check with a warning, and so does every expression when sqlglot is not installed, as validate_sql already does. Identifiers are compared the way expression_language.md compares them: a regular identifier upper-cased, a quoted one as written, and the model's names as regular identifiers. ORDERS.order_id names dataset orders, and "orders".order_id does not. Datasets whose names differ only in case are one dataset to a regular identifier, so their fields are pooled. The referenced ontology and semantic model are validated against their own schemas right after they are loaded. A document that fails is dropped with one error that names the reference and the first schema error, instead of surfacing later as a TypeError from a set of unhashable names or as a misleading arity error for roles given as a mapping. The checks that read the documents assume the shapes the schemas guarantee and no longer guard against the others. A copy of the script without the schemas next to it skips that half with a warning. An iri that urlsplit rejects, and a path with an embedded NUL, are reported as reference errors rather than internal validator errors. The two Flight link mappings in examples/flights.mapping.yaml filter with "FLIGHT.id WHERE ( ... == TRUE )", which is not a SQL expression, so the example now validates with two warnings. Generated-by: Claude Code --- validation/tests/test_ontology.py | 264 ++++++++++++++++++++++++++++-- validation/validate.py | 227 +++++++++++++++++-------- 2 files changed, 410 insertions(+), 81 deletions(-) diff --git a/validation/tests/test_ontology.py b/validation/tests/test_ontology.py index 435ed039..d9c304b6 100644 --- a/validation/tests/test_ontology.py +++ b/validation/tests/test_ontology.py @@ -520,7 +520,13 @@ def _cross_check(tmp_path, mapping_schema, mapping) -> list[str]: def test_split_flights_mapping_cross_checks_against_its_documents(offline, mapping_schema): mapping = yaml.safe_load((_REPO / "examples/flights.mapping.yaml").read_text()) - assert _VALIDATE.validate_document(mapping, mapping_schema, _REPO / "examples") == [] + diagnostics = _VALIDATE.validate_document(mapping, mapping_schema, _REPO / "examples") + + # Two Flight link mappings filter with "FLIGHT.id WHERE ( ... == TRUE )", which is + # not a SQL expression, so their fields are skipped with a warning and nothing fails. + assert [type(d) for d in diagnostics] == [_VALIDATE.ValidationWarning] * 2 + assert all("expression does not parse as a SQL expression" in d for d in diagnostics) + assert all("concept_mappings -> 8 -> link_mappings -> " in d for d in diagnostics) def test_mapping_cross_checks_accept_resolving_references(offline, mapping_schema, tmp_path): @@ -597,9 +603,18 @@ def test_mapping_expressions_must_name_declared_fields(offline, mapping_schema, mapping["concept_mappings"][0]["object_mappings"] = [ {"expression": "orders.order_id"}, {"expression": "orders.order_nr + orders.order_nr"}, - {"expression": "ORDERS.order_id"}, + # Regular identifiers match without regard to case (expression_language.md). + {"expression": "ORDERS.Order_Id"}, + {"expression": "ORDERS.ORDER_NR"}, + # A quoted identifier matches as written, against the model's name upper-cased. + {"expression": '"ORDERS"."ORDER_ID"'}, + {"expression": '"orders".order_id'}, + {"expression": 'orders."order_id"'}, {"expression": "CONCAT(orders.order_id, '.', orders.customer_id)"}, {"expression": "CASE WHEN orders.order_id > 0 THEN 'a.b' END"}, + {"expression": "CAST(orders.order_id AS pg_catalog.int4)"}, + {"expression": "orders.order_id::pg_catalog.int4"}, + {"expression": "transform(orders.order_id, x -> x.y)"}, {"expression": "customers.anything"}, ] @@ -608,27 +623,151 @@ def test_mapping_expressions_must_name_declared_fields(offline, mapping_schema, assert errors == [ "[Reference] concept_mappings -> 0 -> object_mappings -> 1: expression names " "'orders.order_nr' but dataset 'orders' declares no field 'order_nr'", + "[Reference] concept_mappings -> 0 -> object_mappings -> 3: expression names " + "'ORDERS.ORDER_NR' but dataset 'orders' declares no field 'ORDER_NR'", + "[Reference] concept_mappings -> 0 -> object_mappings -> 5: expression names " + "'\"orders\".order_id' but the semantic model declares no dataset '\"orders\"'", + "[Reference] concept_mappings -> 0 -> object_mappings -> 6: expression names " + "'orders.\"order_id\"' but dataset 'orders' declares no field '\"order_id\"'", + ] + + +def test_mapping_datasets_that_differ_only_in_case_pool_their_fields( + offline, mapping_schema, tmp_path +): + model = _sales_model() + model["datasets"].append( + { + "name": "ORDERS", + "source": "sales.public.orders_v2", + "fields": [{"name": "status", "expression": {"dialects": [ + {"dialect": "ANSI_SQL", "expression": "status"} + ]}}], + } + ) + mapping = _write_mapping_set(tmp_path, model=model) + mapping["concept_mappings"][0]["object_mappings"] = [ + {"expression": "orders.order_id"}, + {"expression": "orders.status"}, + {"expression": "Orders.order_nr"}, + ] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [ "[Reference] concept_mappings -> 0 -> object_mappings -> 2: expression names " - "'ORDERS.order_id' but the semantic model declares no dataset 'ORDERS'", + "'Orders.order_nr' but dataset 'orders' declares no field 'order_nr'" ] +def test_mapping_dataset_without_a_fields_list_turns_off_the_check_for_its_name( + offline, mapping_schema, tmp_path +): + model = _sales_model() + model["datasets"].append({"name": "ORDERS", "source": "sales.public.orders_v2"}) + mapping = _write_mapping_set(tmp_path, model=model) + mapping["concept_mappings"][0]["object_mappings"] = [{"expression": "orders.order_nr"}] + + assert _cross_check(tmp_path, mapping_schema, mapping) == [] + + +def test_mapping_expression_that_is_not_sql_is_skipped_with_a_warning( + offline, mapping_schema, tmp_path +): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["object_mappings"] = [ + {"expression": "orders.payload:customer.id"}, + {"expression": "orders.order_nr"}, + ] + + diagnostics = _cross_check(tmp_path, mapping_schema, mapping) + + assert [type(d) for d in diagnostics] == [_VALIDATE.ValidationWarning, str] + assert diagnostics[0] == ( + "[Reference] Warning: concept_mappings -> 0 -> object_mappings -> 0: expression does " + "not parse as a SQL expression, so the fields it names are not checked" + ) + assert "declares no field 'order_nr'" in diagnostics[1] + + +def test_mapping_expressions_are_not_checked_without_sqlglot( + offline, mapping_schema, tmp_path, monkeypatch +): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["object_mappings"] = [{"expression": "orders.order_nr"}] + monkeypatch.setattr(_VALIDATE, "SQLGLOT_AVAILABLE", False) + + diagnostics = _cross_check(tmp_path, mapping_schema, mapping) + + assert diagnostics == [ + "[Reference] Warning: sqlglot is not installed, so the fields the mapping " + "expressions name are not checked. Install with: pip install sqlglot" + ] + assert isinstance(diagnostics[0], _VALIDATE.ValidationWarning) + + +def _reference(dataset: str, field: str, dataset_quoted: bool = False, field_quoted: bool = False): + """A FieldReference for a dataset.field written with the given quoting.""" + written = ( + f'"{dataset}"' if dataset_quoted else dataset, + f'"{field}"' if field_quoted else field, + ) + normalized = ( + dataset if dataset_quoted else dataset.upper(), + field if field_quoted else field.upper(), + ) + return _VALIDATE.FieldReference(written, normalized) + + +_ORDER_ID = _reference("orders", "order_id") + + @pytest.mark.parametrize( "expression, expected", [ - ("orders.order_id -- see orders.nope", [("orders", "order_id")]), - ("/* orders.nope */ orders.order_id", [("orders", "order_id")]), - ('"a.b" + orders.order_id', [("orders", "order_id")]), - ("`a.b` + orders.order_id", [("orders", "order_id")]), - ("'it''s a.b' || orders.order_id", [("orders", "order_id")]), + ("orders.order_id -- see orders.nope", [_ORDER_ID]), + ("/* orders.nope */ orders.order_id", [_ORDER_ID]), + ("'it''s a.b' || orders.order_id", [_ORDER_ID]), + ("'orders.nope' || orders.order_id", [_ORDER_ID]), + ("order_id", []), ("cat.sch.orders.order_id", []), - ("sch.fn(orders.order_id)", [("orders", "order_id")]), - ("orders.order_id + orders.order_id", [("orders", "order_id")]), - ("1.5 * orders.order_id", [("orders", "order_id")]), - ("orders.order_id > 0 AND orders.amount > 1.5", [("orders", "order_id"), ("orders", "amount")]), + ("orders.*", []), + ("COUNT(*)", []), + ("sch.fn(orders.order_id)", [_ORDER_ID]), + ("CAST(orders.order_id AS pg_catalog.int4)", [_ORDER_ID]), + ("orders.order_id::pg_catalog.int4", [_ORDER_ID]), + ("transform(orders.order_id, x -> x.y)", [_ORDER_ID]), + ("orders.order_id + orders.order_id", [_ORDER_ID]), + ("1.5 * orders.order_id", [_ORDER_ID]), + ( + "orders.order_id > 0 AND orders.amount > 1.5", + [_ORDER_ID, _reference("orders", "amount")], + ), + ('"Orders".order_id', [_reference("Orders", "order_id", True, False)]), + ('orders."Order Id"', [_reference("orders", "Order Id", False, True)]), + # The quoted form is kept as written and the normalized one unescaped. + ('"it""s".x', [_VALIDATE.FieldReference(('"it""s"', "x"), ('it"s', "X"))]), + # A column inside a subquery resolves in that query, not in the model, + # and a correlated reference to a dataset goes with it. + ("orders.order_id IN (SELECT o.id FROM other o)", [_ORDER_ID]), + ("(SELECT MAX(o.amount) FROM other o WHERE o.id = orders.order_id)", []), + # Not SQL expressions: a Snowflake path, a backslash escape, a backtick + # identifier, nothing at all, token soup, and nesting past the recursion limit. + ("orders.payload:customer.id", None), + ("'it\\'s a.b' || orders.order_id", None), + ("`a.b` + orders.order_id", None), + ("", None), + ("orders.order_id = {:}", None), + ("(" * 5000 + "orders.order_id" + ")" * 5000, None), + # A dataset named like a keyword reads as a statement, not an expression. + ("from.x", None), + ("update.x", None), + ("set.x", None), + ("pivot.x", None), + ("VALUES (1)", None), ], ) -def test_field_references_read_two_part_identifiers_only(expression, expected): +def test_field_references_are_the_two_part_columns_of_the_parsed_expression(expression, expected): assert _VALIDATE.field_references(expression) == expected @@ -942,11 +1081,61 @@ def test_mapping_reference_to_the_wrong_kind_of_document_is_reported_once( # One line for the document, not one per concept the mapping names. assert errors == [ - "[Reference] ontology_ref iri './sales.semantic_model.yaml' does not hold an ontology " - "document, so its concepts cannot be checked" + "[Reference] ontology_ref iri './sales.semantic_model.yaml' does not validate against " + "ontology/ontology.json (2 error(s), the first at (root): 'ontology' is a required " + "property), so its concepts are not checked" ] +@pytest.mark.parametrize( + "mutate, label, expected", + [ + ( + lambda o, m: o["ontology"][2].update(concept=["Order"]), + "ontology_ref", + "ontology -> 2 -> concept: ['Order'] is not of type 'string'", + ), + ( + lambda o, m: o["ontology"][2]["relationships"][1].update(roles={"concept": "Customer"}), + "ontology_ref", + "ontology -> 2 -> relationships -> 1 -> roles: {'concept': 'Customer'} is not of " + "type 'array'", + ), + ( + lambda o, m: m["datasets"][0].update(name=["orders"]), + "semantic_model_ref", + "datasets -> 0 -> name: ['orders'] is not of type 'string'", + ), + ( + lambda o, m: m["datasets"][0]["fields"][0].update(name={"a": 1}), + "semantic_model_ref", + "datasets -> 0 -> fields -> 0 -> name: {'a': 1} is not of type 'string'", + ), + ], +) +def test_mapping_referenced_document_that_fails_its_schema_is_one_error( + offline, mapping_schema, tmp_path, mutate, label, expected +): + ontology, model = _sales_ontology(), _sales_model() + mutate(ontology, model) + mapping = _write_mapping_set(tmp_path, ontology=ontology, model=model) + # Both halves have something to report. Only the intact half does. + mapping["concept_mappings"][0]["concept"] = "Custmer" + mapping["concept_mappings"][0]["object_mappings"] = [{"expression": "orders.order_nr"}] + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert len(errors) == 2 + iri = "./sales.ontology.yaml" if label == "ontology_ref" else "./sales.semantic_model.yaml" + half = "concepts" if label == "ontology_ref" else "fields" + assert errors[0].startswith(f"[Reference] {label} iri '{iri}' does not validate against ") + assert errors[0].endswith( + f"(1 error(s), the first at {expected}), so its {half} are not checked" + ) + other = "declares no field 'order_nr'" if label == "ontology_ref" else "concept 'Custmer'" + assert other in errors[1] + + @pytest.mark.parametrize( "content, expected", [ @@ -973,6 +1162,51 @@ def test_mapping_reference_that_does_not_read_as_a_document_is_an_error( assert expected in errors[0] +def test_schema_at_is_none_for_a_file_that_is_not_there_or_not_json(tmp_path): + (tmp_path / "broken.json").write_text("{") + + assert _VALIDATE._schema_at(tmp_path / "missing.json") is None + assert _VALIDATE._schema_at(tmp_path / "broken.json") is None + assert _VALIDATE._schema_at(_REPO / "ontology/ontology.json")["$id"].endswith("ontology.json") + + +def test_mapping_referenced_document_is_not_checked_without_its_schema( + offline, mapping_schema, tmp_path, monkeypatch +): + mapping = _write_mapping_set(tmp_path) + mapping["concept_mappings"][0]["concept"] = "Custmer" + mapping["concept_mappings"][0]["object_mappings"] = [{"expression": "orders.order_nr"}] + monkeypatch.setattr(_VALIDATE, "_schema_at", lambda path: None) + + diagnostics = _cross_check(tmp_path, mapping_schema, mapping) + + assert [type(d) for d in diagnostics] == [_VALIDATE.ValidationWarning] * 2 + assert diagnostics[0] == ( + "[Reference] Warning: ontology/ontology.json is not next to this script, so the " + "concepts of ontology_ref are not checked" + ) + assert diagnostics[1].startswith("[Reference] Warning: core-spec/ossie-schema.json is not") + + +@pytest.mark.parametrize( + "iri, expected", + [ + ("http://[oops/sales.ontology.yaml", "is not a valid URI reference: Invalid IPv6 URL"), + ("//[::1/sales.ontology.yaml", "is not a valid URI reference: Invalid IPv6 URL"), + ("./sales%00.ontology.yaml", "does not resolve: embedded null byte"), + ], +) +def test_mapping_iri_that_cannot_be_followed_is_an_error( + offline, mapping_schema, tmp_path, iri, expected +): + mapping = _write_mapping_set(tmp_path) + mapping["ontology_ref"]["iri"] = iri + + errors = _cross_check(tmp_path, mapping_schema, mapping) + + assert errors == [f"[Reference] ontology_ref iri '{iri}' {expected}"] + + def test_validate_file_resolves_mapping_references_from_the_file_location( offline, mapping_schema, tmp_path, capsys ): diff --git a/validation/validate.py b/validation/validate.py index 2faa99ab..cee9d832 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -46,7 +46,9 @@ every relationship a referent or link mapping names is declared on its concept or inherited through extends, with a role for nested referents to resolve on and at the link level its arity calls for, and every dataset.field an expression -names is a declared field of the model. +names is a declared field of the model, identifiers compared as +expression_language.md compares them. A referenced document is checked against +its own schema first. Usage: python validation/validate.py [--schema ] [--] [ ...] @@ -62,12 +64,13 @@ import json import os -import re import sys import traceback from collections import Counter from collections.abc import Hashable +from functools import lru_cache from pathlib import Path +from typing import NamedTuple from urllib.parse import unquote, urlsplit try: @@ -627,28 +630,66 @@ def inherited_relationships(concept: str) -> set[str]: return errors -_QUOTED_OR_COMMENT = re.compile( - r"'(?:[^']|'')*'" # string literal - r'|"(?:[^"]|"")*"' # quoted identifier - r"|`[^`]*`" # backtick identifier - r"|--[^\n]*" # line comment - r"|/\*.*?\*/", # block comment - re.DOTALL, -) -_FIELD_REFERENCE = re.compile(r"(? str: + return ".".join(self.written) -def field_references(expression: str) -> list[tuple[str, str]]: +def normalized_identifier(identifier) -> str: + """The normalized form of a parsed identifier (expression_language.md).""" + return identifier.name if identifier.quoted else identifier.name.upper() + + +def field_references(expression: str) -> list[FieldReference] | None: """The dataset.field pairs a mapping expression names, each once. Mapping expressions reach the semantic model through two-part identifiers - (ontology.md, Object mappings). String literals, quoted identifiers and - comments are blanked first, so a dot inside them is not a reference, and - neither is a longer dotted chain or a schema-qualified function call. - Quoted identifiers are therefore not resolved. + (ontology.md, Object mappings), so the expression is parsed and every + two-part column in it is a reference. A one-part column, a longer chain, + a type name, a function name and a lambda parameter are not, and a column + inside a subquery is left to that query's own scope, a correlated + reference to a dataset included. Mapping expressions declare no dialect, + so the expression is parsed with the sqlglot default, + and one that does not parse as a SQL expression comes back as None. That + includes a string sqlglot reads as a statement, which a dataset named + like a keyword, such as from.x, otherwise would be. Needs sqlglot. """ - text = _QUOTED_OR_COMMENT.sub(" ", expression) - return list(dict.fromkeys(_FIELD_REFERENCE.findall(text))) + try: + tree = sqlglot.parse_one(expression) + except Exception: + # ParseError and TokenError for input sqlglot rejects, RecursionError + # for deep nesting, and the parser raises AttributeError from inside + # on token soup such as `{:}`. Every one means it did not parse. + return None + exp = sqlglot.exp + statement = (exp.Command, exp.DDL, exp.DML, exp.Pivot, exp.Values) + query = isinstance(tree, exp.Query) and not isinstance(tree, exp.Subquery) + if isinstance(tree, statement) or query: + return None + references = [] + for column in tree.find_all(sqlglot.exp.Column): + parts = column.parts + if len(parts) != 2 or not all(isinstance(p, sqlglot.exp.Identifier) for p in parts): + continue + if column.find_ancestor(sqlglot.exp.Select) is not None: + continue + references.append(FieldReference( + written=(parts[0].sql(), parts[1].sql()), + normalized=(normalized_identifier(parts[0]), normalized_identifier(parts[1])), + )) + return list(dict.fromkeys(references)) def resolve_document_reference( @@ -672,7 +713,10 @@ def resolve_document_reference( f"[Reference] Warning: {label} '{name}' has no iri, so only a catalog can " f"resolve it: {skipped}" )] - parts = urlsplit(iri) + try: + parts = urlsplit(iri) + except ValueError as e: + return None, [f"[Reference] {label} iri '{iri}' is not a valid URI reference: {e}"] if parts.scheme or parts.netloc: return None, [ValidationWarning( f"[Reference] Warning: {label} iri '{iri}' is not a path reference, so it is " @@ -693,6 +737,9 @@ def resolve_document_reference( return None, [f"[Reference] {label} iri '{iri}' does not resolve: {e.strerror or e}"] except UnicodeDecodeError: return None, [f"[Reference] {label} iri '{iri}' is not valid UTF-8 text"] + except ValueError as e: + # A path with an embedded NUL, from %00 in the iri, is rejected before it is opened. + return None, [f"[Reference] {label} iri '{iri}' does not resolve: {e}"] try: document = load_yaml_named(text, str(path)) except yaml.YAMLError as e: @@ -711,6 +758,44 @@ def resolve_document_reference( return document, errors +@lru_cache(maxsize=None) +def _schema_at(path: Path) -> dict | None: + """The schema in a file, or None where the script runs without one.""" + try: + return json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + + +def schema_checked_document( + document: dict | None, schema_file: str, label: str, iri: object, half: str +) -> tuple[dict | None, list[str]]: + """Validate a referenced document against the schema its reference promises. + + The checks that read the document assume the shapes that schema + guarantees, so a document that fails it is dropped with one error that + names the reference, the schema and the first schema error. The schema + is read from this checkout, like the ones validate_schema resolves, and + a copy of the script without it skips that half with a warning. + """ + if document is None: + return None, [] + schema = _schema_at(_REPO_ROOT / schema_file) + if schema is None: + return None, [ValidationWarning( + f"[Reference] Warning: {schema_file} is not next to this script, so the {half} " + f"of {label} are not checked" + )] + schema_errors = validate_schema(document, schema) + if not schema_errors: + return document, [] + first = schema_errors[0].removeprefix("[Schema] ") + return None, [ + f"[Reference] {label} iri '{iri}' does not validate against {schema_file} " + f"({len(schema_errors)} error(s), the first at {first}), so its {half} are not checked" + ] + + class OntologyRelationships: """The relationships an ontology declares, looked up through extends. @@ -722,17 +807,11 @@ def __init__(self, components: list[dict]): self._declared: dict[str, dict[str, dict]] = {} self._supertypes: dict[str, list[str]] = {} for component in components: - concept = component.get("concept") - if not isinstance(concept, str): - continue + concept = component["concept"] self._declared.setdefault(concept, {}).update( - (r["name"], r) - for r in as_list(component.get("relationships")) - if isinstance(r, dict) and isinstance(r.get("name"), str) - ) - self._supertypes.setdefault(concept, []).extend( - s for s in as_list(component.get("extends")) if isinstance(s, str) + (r["name"], r) for r in component.get("relationships", []) ) + self._supertypes.setdefault(concept, []).extend(component.get("extends", [])) def ancestors(self, concept: str) -> list[str]: """The concept followed by every concept it extends, each once.""" @@ -761,11 +840,16 @@ def validate_mapping(data: dict, base: Path | None) -> list[str]: be declared on the concept whose mapping names it, directly or through extends, with a role for nested referents to resolve on and at the link level its arity calls for, and every dataset.field an expression names - must be a field of that semantic model. + must be a field of that semantic model, comparing identifiers the way + expression_language.md does: regular ones without regard to case, quoted + ones as written, and the model's names as regular identifiers. A dataset that declares no fields list is not checked, since the core spec - does not require one. Relative iri are read from base, the mapping - document's own directory. A reference that cannot be followed skips its - half of the checks with a warning, see resolve_document_reference. + does not require one, and an expression that does not parse as a SQL + expression is skipped with a warning, as is every one when sqlglot is missing. + Relative iri are read from base, the mapping document's own directory. A + reference that cannot be followed, or whose document fails its schema, + skips its half of the checks, see resolve_document_reference and + schema_checked_document. """ if not isinstance(data, dict) or "concept_mappings" not in data: return [] @@ -775,40 +859,43 @@ def validate_mapping(data: dict, base: Path | None) -> list[str]: model_ref = data.get("semantic_model_ref") or {} ontology, diagnostics = resolve_document_reference(ontology_ref, base, "ontology_ref") errors.extend(diagnostics) + ontology, diagnostics = schema_checked_document( + ontology, "ontology/ontology.json", "ontology_ref", ontology_ref.get("iri"), "concepts" + ) + errors.extend(diagnostics) model, diagnostics = resolve_document_reference(model_ref, base, "semantic_model_ref") errors.extend(diagnostics) + model, diagnostics = schema_checked_document( + model, "core-spec/ossie-schema.json", "semantic_model_ref", model_ref.get("iri"), "fields" + ) + errors.extend(diagnostics) # None means that half of the checks is off: the document was not followed # or is not the kind of document the reference promises. concepts = None relationships = None if ontology is not None: - if isinstance(ontology.get("ontology"), list): - components = [c for c in ontology["ontology"] if isinstance(c, dict)] - concepts = {c.get("concept") for c in components} | BUILT_IN_CONCEPTS - relationships = OntologyRelationships(components) - else: - errors.append( - f"[Reference] ontology_ref iri '{ontology_ref.get('iri')}' does not hold an " - f"ontology document, so its concepts cannot be checked" - ) - fields_by_dataset = None - if model is not None: - if isinstance(model.get("datasets"), list): - fields_by_dataset = { - d.get("name"): ( - {f.get("name") for f in d["fields"] if isinstance(f, dict)} - if isinstance(d.get("fields"), list) - else None - ) - for d in model["datasets"] - if isinstance(d, dict) - } - else: - errors.append( - f"[Reference] semantic_model_ref iri '{model_ref.get('iri')}' does not hold a " - f"semantic model document, so its fields cannot be checked" - ) + concepts = {c["concept"] for c in ontology["ontology"]} | BUILT_IN_CONCEPTS + relationships = OntologyRelationships(ontology["ontology"]) + # Keyed by the normalized dataset name: the name as first declared and its + # normalized field names, or None once a dataset declares no fields list. + # Datasets whose names differ only in case are one dataset to a regular + # identifier, so their fields are pooled. + datasets = None + if model is not None and not SQLGLOT_AVAILABLE: + errors.append(ValidationWarning( + "[Reference] Warning: sqlglot is not installed, so the fields the mapping " + "expressions name are not checked. Install with: pip install sqlglot" + )) + elif model is not None: + datasets = {} + for dataset in model["datasets"]: + name, fields = datasets.get(dataset["name"].upper(), (dataset["name"], set())) + if fields is not None and "fields" in dataset: + fields = fields | {f["name"].upper() for f in dataset["fields"]} + else: + fields = None + datasets[dataset["name"].upper()] = (name, fields) def visit(node: object, path: str) -> None: if isinstance(node, dict): @@ -816,17 +903,25 @@ def visit(node: object, path: str) -> None: if concepts is not None and isinstance(concept, str) and concept not in concepts: errors.append(f"[Reference] {path}: concept '{concept}' is not declared in the ontology") expression = node.get("expression") - if fields_by_dataset is not None and isinstance(expression, str): - for dataset, field in field_references(expression): - if dataset not in fields_by_dataset: + if datasets is not None and isinstance(expression, str): + references = field_references(expression) + if references is None: + errors.append(ValidationWarning( + f"[Reference] Warning: {path}: expression does not parse as a SQL " + f"expression, so the fields it names are not checked" + )) + for reference in references or []: + dataset, field = reference.normalized + declared = datasets.get(dataset) + if declared is None: errors.append( - f"[Reference] {path}: expression names '{dataset}.{field}' but " - f"the semantic model declares no dataset '{dataset}'" + f"[Reference] {path}: expression names '{reference.text}' but " + f"the semantic model declares no dataset '{reference.written[0]}'" ) - elif fields_by_dataset[dataset] is not None and field not in fields_by_dataset[dataset]: + elif declared[1] is not None and field not in declared[1]: errors.append( - f"[Reference] {path}: expression names '{dataset}.{field}' but " - f"dataset '{dataset}' declares no field '{field}'" + f"[Reference] {path}: expression names '{reference.text}' but " + f"dataset '{declared[0]}' declares no field '{reference.written[1]}'" ) for key, value in node.items(): visit(value, f"{path} -> {key}")