diff --git a/CHANGELOG.md b/CHANGELOG.md index 758fa407..a94fd882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`export_yaml` raised a raw `AttributeError` on list input, silently wrote empty exports for unrecognized dict keys, and graph payloads were reconciled differently by every exporter** (#958, closes #956, #952, #953) by @pravit-amp, reviewed by @Sameer6305 + - Graph payloads circulate under two vocabularies, `entities`/`relationships` and `nodes`/`edges`, and each exporter reconciled them locally with a different idiom — `LPGExporter` in particular dropped every entity whenever `nodes` was present but empty, the exact shape `JSONExporter` emits. A new `normalize_graph_payload()` in `utils/helpers.py` centralizes that decision once, adopted by `LPGExporter`, `ArangoAQLExporter`, `Neo4jCSVExporter`, and both YAML exporters; `ContextGraph.to_dict()` now round-trips through YAML correctly as a result + - `export_yaml(records, path)` on a bare list previously failed with `AttributeError` from inside the exporter; it and the other YAML methods now reject non-mapping input with an actionable `ProcessingError` naming the expected keys, since these formats distinguish entities/relationships/triplets and guessing which one a list represents would mislabel the records + - `export_yaml({"data": [...]}, path)` previously wrote a structurally valid file with every collection empty, no exception, no warning, and the progress log reporting a completed export. `export_semantic_network`, `export_for_pipeline`, and `export_ontology_schema` now raise `ValidationError` when the payload shares no recognized key with what the method reads, or resolves to nothing while an unread key still holds records — an empty mapping is still accepted, since a genuinely empty graph has no records to lose + - **Breaking**: the two cases above, plus a bare list, now raise instead of returning cleanly with data silently dropped or a raw `AttributeError` from exporter internals. Migration: pass records under a recognized key (`{"entities": [...]}` / `{"nodes": [...]}` for `semantic_network`, `{"classes": [...]}` for `schema`) + - **Fixed during review** (Qodo): progress tracking could report a completed export before the output directory existed or the file was written; `export()` now creates the directory and serializes before starting tracking, so a rejected export leaves nothing behind + - **Fixed during review** (@Sameer6305, round 1): `normalize_graph_payload()`'s collection resolver treated any truthy value as a collection — `{"entities": "abc"}` silently became three single-character records, `{"entities": 42}` leaked a raw `TypeError` from inside `list()`. Collection values are now validated before conversion, rejecting strings/bytes/mappings/non-iterable scalars by name. Separately, `Neo4jCSVExporter._normalize_graph` called the shared resolver with `require_recognized=False`, so it alone kept accepting an unrecognized mapping as a silent empty export; the opt-out (introduced earlier in this same PR, with no other caller) was removed + - **Fixed during review** (@Sameer6305, round 2): `YAMLSchemaExporter`'s usable-schema check could treat scalar schema metadata (`version`, `uri`, `title`, `description`) as evidence records had been exported, letting records under an unread key drop silently; and `_is_record()` accepted modules and class/type objects through the generic `__dict__` path, which would have reached exporter internals instead of failing at the boundary. Both closed, with regression coverage + - **Fixed during final maintainer review** (before merge): four more gaps in the shared boundary that the earlier rounds didn't reach + - `LPGExporter`/`ArangoAQLExporter` called `normalize_graph_payload()` with no type guard, so non-mapping input raised `ValidationError` from inside the resolver — while YAML and `Neo4jCSVExporter` raised `ProcessingError` for the identical mistake, per this PR's own stated contract. The `_require_mapping()` guard that already existed in `yaml_exporter.py` is now shared from `utils/helpers.py` and used by all three + - `Neo4jCSVExporter._normalize_graph` checked `isinstance(graph, dict)`, so a non-dict `Mapping` (`MappingProxyType`, `ChainMap`) fell through to the object-attribute branch and was rejected, even though the identical payload exported fine via `LPGExporter`/`ArangoAQLExporter`/YAML. Now checks `isinstance(graph, Mapping)` + - `normalize_graph_payload()` accepts dataclass and attribute-bearing object records (`Neo4jCSVExporter._record_to_dict` reads them), but `LPGExporter`/`ArangoAQLExporter` call `.get(...)` directly on resolved entities — an object-shaped record passed validation only to crash with a raw `AttributeError` once used, the exact failure this PR's boundary exists to prevent. Records are now converted to plain dicts at the boundary (`_coerce_records` → new `_record_to_dict`), so every consumer gets a uniform shape regardless of which reading the caller used + - Two non-empty spellings of the same collection (e.g. `entities` and `nodes`) holding identical records in a different order were rejected as conflicting, since the check used plain list equality; a caller round-tripping through a dict-keyed cache or a set has no reason to preserve order. Comparison is now an order-independent multiset of each record's canonical JSON form + - New regression coverage in `tests/utils/test_normalize_graph_payload.py`: exception-type parity for non-mapping input across `export_lpg`/`export_arango`/`export_neo4j_csv`, dataclass-record conversion verified end-to-end through the same three exporters, `Neo4jCSVExporter` accepting a `MappingProxyType` payload, and reordered-alias equality (plus a duplicate-count case confirming the multiset check still catches real conflicts); 4 existing tests updated to assert the corrected dict-conversion behavior instead of the previous object passthrough + - `pytest tests/export tests/utils tests/context tests/test_export_module.py tests/test_export_methods_wrapper.py tests/test_notebooks_simulation.py`: 718 passed, 4 skipped (up from 641 passed, 62 subtests at PR submission); `black`/`isort`/`flake8 --max-line-length=88` clean on every line this PR touches; `python -m build`: succeeds + - **`ContextGraph.add_edge` had no dedupe — identical edges were stored repeatedly under one shared edge ID, and re-ingest doubled the edge set** (#926, closes #922) by @pravit-amp - `_add_internal_edge` appended to `self.edges`, `edge_type_index`, and `_adjacency` unconditionally, with no check for an edge already present. Edge identity is content-derived (`_resolve_edge_identity` builds `edge_id` from `source_id`/`target_id`/`edge_type`/`weight`/`metadata`/`valid_from`/`valid_until`), so two identical `add_edge` calls produced two edge objects sharing one `edge_id` — the graph already considered them the same edge, it just kept both copies. `self.nodes` already deduped by ID; edges did not, so `stats()["edge_count"]` inflated, `density()` could exceed its mathematical maximum of `1.0`, and a refresh/restore job calling `build_from_entities_and_relationships()` (or reloading a saved graph) doubled the edge set on every cycle - Added an `edge_id -> ContextEdge` index (`_edge_index`), mirroring how `self.nodes` dedupes by node ID. `_add_internal_edge` now returns `False` when the `edge_id` already exists, checked before touching `edges`/`edge_type_index`/`_adjacency` and before firing the mutation callback, so a repeat `add_edge` is a silent no-op with no phantom `ADD_EDGE` audit event diff --git a/docs/reference/export.md b/docs/reference/export.md index 49df715e..eef19e94 100644 --- a/docs/reference/export.md +++ b/docs/reference/export.md @@ -203,6 +203,13 @@ export_lpg(graph, "import.cypher", method="cypher") exporter = SemanticNetworkYAMLExporter() exporter.export(graph, "graph.yaml") ``` + + The YAML exporters read `entities`/`relationships`/`triplets` (with + `nodes`/`edges` accepted as aliases, so `ContextGraph.to_dict()` exports + directly). A non-empty mapping supplying none of them raises + `ValidationError` rather than writing a file with every collection empty, + as does one whose collection value is not a list of records + (`{"entities": "abc"}`). **LPGExporter** writes Cypher `CREATE` statements for Neo4j and Memgraph: @@ -236,6 +243,12 @@ export_lpg(graph, "import.cypher", method="cypher") Both exporters write to a file and return `None`. + `LPGExporter`, `ArangoAQLExporter`, and `Neo4jCSVExporter` resolve mapping + payloads on the same terms as the YAML exporters above, so an unrecognized + or malformed mapping is rejected instead of exported as an empty graph. + `Neo4jCSVExporter` still reads graph *objects* off their + `nodes`/`entities` and `edges`/`relationships` attributes. + **`ArangoAQLExporter.export()` and `LPGExporter.export()` write to a file and return `None`.** They do not return the AQL/Cypher string. Write to a file and read it back if you need the string. diff --git a/semantica/export/arango_aql_exporter.py b/semantica/export/arango_aql_exporter.py index 69140481..d5f9a571 100644 --- a/semantica/export/arango_aql_exporter.py +++ b/semantica/export/arango_aql_exporter.py @@ -28,7 +28,7 @@ import json from pathlib import Path from typing import Any, Dict, List, Optional, Union -from ..utils.helpers import ensure_directory +from ..utils.helpers import _require_mapping, ensure_directory, normalize_graph_payload from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker @@ -204,17 +204,19 @@ class ArangoAQLExporter: self._generate_collection_creation(vertex_collection, edge_collection) ) - # Extract entities and relationships - entities = knowledge_graph.get("entities", []) - relationships = knowledge_graph.get("relationships", []) - nodes = knowledge_graph.get("nodes", entities) - edges = knowledge_graph.get("edges", relationships) + # A non-mapping payload cannot reach normalize_graph_payload(): it + # raises ValidationError for that case, which would leave this + # exporter alone in raising a different exception type than the YAML + # and Neo4j exporters raise for the identical mistake. + _require_mapping( + knowledge_graph, ("entities", "relationships", "nodes", "edges") + ) - # Use nodes/edges if entities/relationships are empty - if not entities and nodes: - entities = nodes - if not relationships and edges: - relationships = edges + # Accept either vocabulary; resolution is centralized so every + # exporter agrees on what a given payload means. + normalized = normalize_graph_payload(knowledge_graph) + entities = normalized["entities"] + relationships = normalized["relationships"] # Generate vertex INSERT statements vertex_statements = self._generate_vertex_inserts(entities, vertex_collection) diff --git a/semantica/export/export_usage.md b/semantica/export/export_usage.md index 140c5a01..7c8881e2 100644 --- a/semantica/export/export_usage.md +++ b/semantica/export/export_usage.md @@ -297,6 +297,57 @@ export_yaml(semantic_network, "network.yaml", method="semantic_network") export_yaml(schema, "schema.yaml", method="schema") ``` +### Accepted Input + +Both YAML exporters read their payload by key, so the input must be a mapping; +anything else raises `ProcessingError`. A bare list is rejected rather than +wrapped, since these formats distinguish entities from relationships from +triplets and guessing which one a list holds would mislabel the records. + +Each exporter then reads a fixed set of keys, and raises `ValidationError` on a +non-empty mapping that supplies none of them — such a payload would otherwise +serialize to a valid file with every collection empty. Naming a recognized key +is not enough on its own: `{"entities": [], "data": [...]}` also raises, since +nothing resolves while the records sit under a key the exporter never reads. + +| Method | Recognized keys | +| :--- | :--- | +| `"semantic_network"` | `entities` (alias `nodes`), `relationships` (alias `edges`), `triplets` | +| `"schema"` | `classes`, `properties`, `namespaces`, `uri`, `title`, `description`, `version` | + +`metadata` is carried through on both, but does not by itself make a payload +recognized — an `export_json` envelope (`{"data": [...], "count": N, +"metadata": {...}}`) carries one and is rejected. + +```python +# ContextGraph.to_dict() exports directly via the nodes/edges aliases +export_yaml(context_graph.to_dict(), "graph.yaml") + +# A bare list has no unambiguous meaning here +export_yaml(records, "out.yaml") # ProcessingError + +# An export_json payload is refused rather than written out empty +export_yaml({"data": records}, "out.yaml") # ValidationError + +# ...and so is one that names a recognized key but leaves it empty +export_yaml({"entities": [], "data": records}, "out.yaml") # ValidationError +``` + +The value under a recognized key must be a collection of records — a list or +tuple of mappings or objects. A string, a bare mapping, or a scalar raises +`ValidationError` naming the key, rather than being iterated into +character-sized "records" or surfacing as a `TypeError` from inside the +exporter. `None` is read as an absent collection, the same as `[]`. + +```python +export_yaml({"entities": "abc"}, "out.yaml") # ValidationError +export_yaml({"entities": 42}, "out.yaml") # ValidationError +export_yaml({"nodes": {"id": "n1"}}, "out.yaml") # ValidationError — wrap it in a list +``` + +An empty mapping is still accepted: an empty graph is a legitimate export and +has no records to lose. + ## OWL Export ### OWL/XML Format @@ -479,6 +530,23 @@ Pass `validate=True` to run a post-export integrity check before returning: export_neo4j_csv(kg, "neo4j_import/", validate=True) ``` +#### Accepted Input + +Mapping payloads are read on the same terms as the YAML exporters (see [Accepted +Input](#accepted-input) above): `entities`/`relationships`, with `nodes`/`edges` +accepted as aliases. A non-empty mapping that supplies neither — or that supplies +a malformed collection value — raises `ValidationError` rather than writing +header-only CSVs indistinguishable from a genuinely exported empty graph. The +payload is normalized before any file is opened, so a rejected export writes +nothing. + +Graph *objects* are unaffected: they are still read off `nodes`/`entities` and +`edges`/`relationships` attributes. + +```python +export_neo4j_csv({"data": [{"id": "e1"}]}, "neo4j_import/") # ValidationError +``` + #### Importing into Neo4j Once the CSV files are generated, they can be imported into a new Neo4j database using the `neo4j-admin database import` command: diff --git a/semantica/export/lpg_exporter.py b/semantica/export/lpg_exporter.py index 40f688a5..06ac4554 100644 --- a/semantica/export/lpg_exporter.py +++ b/semantica/export/lpg_exporter.py @@ -24,7 +24,7 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Union from ..utils.exceptions import ProcessingError, ValidationError -from ..utils.helpers import ensure_directory +from ..utils.helpers import _require_mapping, ensure_directory, normalize_graph_payload from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker @@ -154,15 +154,26 @@ class LPGExporter: """ queries = [] - # Generate indexes if requested - if self.include_indexes: - queries.extend(self._generate_indexes(knowledge_graph)) + # A non-mapping payload cannot reach normalize_graph_payload(): it + # raises ValidationError for that case, which would leave this + # exporter alone in raising a different exception type than the YAML + # and Neo4j exporters raise for the identical mistake. + _require_mapping( + knowledge_graph, ("entities", "relationships", "nodes", "edges") + ) - # Extract entities and relationships - entities = knowledge_graph.get("entities", []) - relationships = knowledge_graph.get("relationships", []) - nodes = knowledge_graph.get("nodes", entities) - edges = knowledge_graph.get("edges", relationships) + # Accept either vocabulary. Reading 'nodes' with 'entities' as the + # default dropped every entity when 'nodes' was present but empty -- + # the shape JSONExporter emits -- so resolution is centralized. + normalized = normalize_graph_payload(knowledge_graph) + nodes = normalized["entities"] + edges = normalized["relationships"] + + # Generate indexes if requested. Fed the normalized entities so index + # generation sees the same records as node generation; reading + # 'entities' directly here skipped indexes for nodes/edges payloads. + if self.include_indexes: + queries.extend(self._generate_indexes(nodes)) # Generate node creation queries node_queries = self._generate_node_queries(nodes) @@ -174,13 +185,18 @@ class LPGExporter: return queries - def _generate_indexes(self, knowledge_graph: Dict[str, Any]) -> List[str]: - """Generate Cypher index and constraint creation queries.""" + def _generate_indexes(self, entities: List[Dict[str, Any]]) -> List[str]: + """Generate Cypher index and constraint creation queries. + + Args: + entities: Entity records, already resolved from whichever + vocabulary the caller supplied. + """ indexes = [] # Get unique entity types for labels entity_types = set() - for entity in knowledge_graph.get("entities", []): + for entity in entities: entity_type = entity.get("type") or entity.get("entity_type") if entity_type: entity_types.add(entity_type) diff --git a/semantica/export/methods.py b/semantica/export/methods.py index 6cec22c4..d2af2581 100644 --- a/semantica/export/methods.py +++ b/semantica/export/methods.py @@ -494,7 +494,7 @@ def export_graph( def export_yaml( - data: Union[Dict[str, Any], List[Dict[str, Any]]], + data: Dict[str, Any], file_path: Union[str, Path], method: str = "semantic_network", **kwargs, @@ -504,14 +504,32 @@ def export_yaml( This is a user-friendly wrapper that exports data to YAML format. + Unlike :func:`export_json` and :func:`export_csv`, which treat a list as + opaque records, both YAML methods are keyed formats: they distinguish + entities from relationships from triplets (and classes from properties + for ``method="schema"``). A bare list is therefore rejected rather than + guessed at, since inferring which collection it represents would silently + mislabel the records. + Args: - data: Data to export (semantic network, entities, relationships) + data: Data to export, as a mapping. For ``method="semantic_network"``, + keyed by 'entities'/'relationships'/'triplets'; for + ``method="schema"``, by 'classes'/'properties'. file_path: Output YAML file path method: Export method (default: "semantic_network") - "semantic_network": Semantic network YAML export - "schema": Schema YAML export **kwargs: Additional options passed to YAML exporters + Raises: + ProcessingError: if ``data`` is not a mapping, or if ``method`` is not + a known YAML export method. + ValidationError: if ``data`` is a mapping whose keys the selected + exporter does not read -- an ``export_json`` envelope + (``{"data": [...], "count": N, "metadata": {...}}``) is the + common case. Such a payload used to be written out as a valid + YAML file with every collection empty. + Examples: >>> from semantica.export.methods import export_yaml >>> export_yaml(semantic_network, "network.yaml", method="semantic_network") diff --git a/semantica/export/neo4j_csv_exporter.py b/semantica/export/neo4j_csv_exporter.py index 7a3461d1..8dbbd31b 100644 --- a/semantica/export/neo4j_csv_exporter.py +++ b/semantica/export/neo4j_csv_exporter.py @@ -32,12 +32,13 @@ from __future__ import annotations import csv import hashlib import json +from collections.abc import Mapping from dataclasses import asdict, is_dataclass from pathlib import Path from typing import Any, Dict, Iterable, List, Optional, Sequence, Union from ..utils.exceptions import ProcessingError, ValidationError -from ..utils.helpers import ensure_directory +from ..utils.helpers import ensure_directory, normalize_graph_payload from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker @@ -173,6 +174,19 @@ class Neo4jCSVExporter: Returns: Mapping with ``"nodes"`` and ``"relationships"`` output paths. + + Raises: + ValidationError: if a mapping payload carries no recognized graph + key, resolves to nothing while an unread key still holds + records, or holds something other than records under one -- + see + :func:`~semantica.utils.helpers.normalize_graph_payload`. + Each would otherwise be written out as header-only CSVs + indistinguishable from a genuinely empty graph. The payload is + normalized before any file is opened, so a rejected export + writes nothing. + ProcessingError: if a non-mapping payload exposes none of the + graph attributes. """ output_dir = Path(output_dir) ensure_directory(output_dir) @@ -494,9 +508,19 @@ class Neo4jCSVExporter: return prepared def _normalize_graph(self, graph: Any) -> Dict[str, List[Dict[str, Any]]]: - if isinstance(graph, dict): - nodes = graph.get("nodes") or graph.get("entities") or [] - relationships = graph.get("edges") or graph.get("relationships") or [] + if isinstance(graph, Mapping): + # Mapping payloads go through the shared resolver on its default + # terms, so this backend cannot drift from the others: an + # unrecognized mapping raises here rather than writing header-only + # CSVs that read as a successful export of an empty graph. Checked + # against Mapping rather than dict, so a non-dict Mapping (a + # MappingProxyType, a ChainMap) takes this path too, instead of + # falling through to the attribute branch below and being rejected + # as an unrecognized object -- the LPG, Arango, and YAML exporters + # already accept such payloads via the same resolver. + resolved = normalize_graph_payload(graph) + nodes = resolved["entities"] + relationships = resolved["relationships"] else: nodes = getattr(graph, "nodes", None) if nodes is None: diff --git a/semantica/export/yaml_exporter.py b/semantica/export/yaml_exporter.py index 0249ec08..ecbd55fa 100644 --- a/semantica/export/yaml_exporter.py +++ b/semantica/export/yaml_exporter.py @@ -21,15 +21,83 @@ Author: Semantica Contributors License: MIT """ +from collections.abc import Mapping from datetime import datetime from pathlib import Path from typing import Any, Dict, List, Optional, Union -from ..utils.exceptions import ProcessingError, ValidationError -from ..utils.helpers import ensure_directory +from ..utils.exceptions import ValidationError +from ..utils.helpers import ( + _require_mapping, + _require_nothing_dropped, + _require_recognized_keys, + ensure_directory, + normalize_graph_payload, +) from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker +# Keys YAMLSchemaExporter.export_ontology_schema reads. Graph payloads use the +# recognized set owned by normalize_graph_payload() instead; schemas are a +# separate vocabulary with no aliasing, so the set lives here. +_SCHEMA_KEYS = ( + "classes", + "properties", + "namespaces", + "uri", + "title", + "description", + "version", +) + + +def _require_usable_schema(ontology: Mapping) -> None: + """Reject a schema mapping this exporter cannot read. + + Two ways an ontology mapping produces an empty file: it shares no key with + the recognized set at all, or it names a recognized key that is empty + while the real records sit under a key this exporter does not read + (``{"classes": [], "nodes": [...]}``). Both are refused, using the same + checks the graph payloads go through, so the two vocabularies cannot drift + apart in what they consider a silent-empty export. + + An empty mapping is allowed through: it carries nothing that could be + lost, and an empty export is a legitimate result. + + Note the deliberate split in exception types, which the codebase already + makes: a wrong *type* cannot be exported at all and raises + ProcessingError, matching ``Neo4jCSVExporter._normalize_graph``; a mapping + whose *contents* are unusable raises ValidationError, matching + ``normalize_graph_payload``. + + Args: + ontology: Mapping already checked by :func:`_require_mapping`. + + Raises: + ValidationError: if the mapping shares no key with ``_SCHEMA_KEYS``, + or resolves to nothing while an unread key still holds records. + """ + _require_recognized_keys(ontology, _SCHEMA_KEYS, what="Ontology schema") + # Only non-empty list/tuple values from recognized schema keys count as + # evidence that records survived export. Scalar metadata fields such as + # 'uri', 'title', 'description', and 'version' are truthy strings, but + # their presence does not mean the caller's record collections were + # exported -- passing them as ``resolved`` would let any scalar value + # short-circuit the dropped-records check and silently discard a list + # under an unread key alongside e.g. {"version": "1.0", "nodes": [...]}. + resolved = [ + v + for key in _SCHEMA_KEYS + for v in (ontology.get(key),) + if isinstance(v, (list, tuple)) and v + ] + _require_nothing_dropped( + ontology, + _SCHEMA_KEYS, + resolved, + what="Ontology schema", + ) + class SemanticNetworkYAMLExporter: """ @@ -90,15 +158,39 @@ class SemanticNetworkYAMLExporter: Args: semantic_network: Semantic network dictionary containing: - - entities: List of entity dictionaries + - entities: List of entity dictionaries (alias: 'nodes') - relationships: List of relationship dictionaries + (alias: 'edges') - triplets: List of triplet dictionaries (optional) - metadata: Metadata dictionary (optional) + + Key resolution is delegated to + :func:`~semantica.utils.helpers.normalize_graph_payload`, so + ``ContextGraph.to_dict()`` output ('nodes'/'edges') exports + directly. **options: Additional export options (unused) Returns: String containing YAML representation of semantic network + Raises: + ProcessingError: if ``semantic_network`` is not a mapping. A bare + list of records cannot be exported here because this format + distinguishes entities, relationships, and triplets, and + guessing which one a list represents would silently mislabel + it. + ValidationError: if the mapping carries both spellings of a + collection with different contents; if it is non-empty and + shares no key with the recognized set; or if it resolves to + nothing while an unread key still holds records + (``{"entities": [], "data": [...]}``). Each previously + serialized to a file with every collection empty while the log + reported success. An empty mapping is still accepted -- it has + no records to lose. Note that 'metadata' alone is not a + recognized key: an ``export_json`` envelope carries one, and + accepting it would readmit the silent-empty export it is the + most likely source of. + Example: >>> network = { ... "entities": [...], @@ -107,6 +199,8 @@ class SemanticNetworkYAMLExporter: ... } >>> yaml_str = exporter.export_semantic_network(network) """ + _require_mapping(semantic_network, ("entities", "relationships", "triplets")) + # Track YAML export tracking_id = self.progress_tracker.start_tracking( file=None, @@ -119,15 +213,14 @@ class SemanticNetworkYAMLExporter: self.progress_tracker.update_tracking( tracking_id, message="Preparing YAML data..." ) + records = normalize_graph_payload(semantic_network) yaml_data = { "metadata": { "exported_at": datetime.now().isoformat(), "version": "1.0", **semantic_network.get("metadata", {}), }, - "entities": semantic_network.get("entities", []), - "relationships": semantic_network.get("relationships", []), - "triplets": semantic_network.get("triplets", []), + **records, } self.progress_tracker.update_tracking( @@ -140,7 +233,7 @@ class SemanticNetworkYAMLExporter: self.progress_tracker.stop_tracking( tracking_id, status="completed", - message="Exported semantic network to YAML", + message="Serialized semantic network to YAML", ) return result @@ -160,16 +253,46 @@ class SemanticNetworkYAMLExporter: data: Data to export file_path: Output file path **options: Additional options + + Raises: + ProcessingError: if ``data`` is not a mapping. + ValidationError: on the mappings :meth:`export_semantic_network` + rejects. Serialization runs before the output directory is + created, so a rejected export leaves nothing behind. + OSError: if the file cannot be written. The write is tracked + separately from serialization, so no progress entry reports a + completed export until the bytes are on disk. """ file_path = Path(file_path) - ensure_directory(file_path.parent) - yaml_content = self.export_semantic_network(data, **options) - with open(file_path, "w", encoding="utf-8") as f: - f.write(yaml_content) + # Serialization reports its own completion, but it says nothing about + # the file: without this second span, a failing write would leave the + # tracker showing a completed export and no output. + tracking_id = self.progress_tracker.start_tracking( + file=str(file_path), + module="export", + submodule="SemanticNetworkYAMLExporter", + message=f"Writing YAML to {file_path}", + ) - self.logger.info(f"Exported YAML to: {file_path}") + try: + ensure_directory(file_path.parent) + with open(file_path, "w", encoding="utf-8") as f: + f.write(yaml_content) + + self.logger.info(f"Exported YAML to: {file_path}") + self.progress_tracker.stop_tracking( + tracking_id, + status="completed", + message=f"Exported YAML to: {file_path}", + ) + + except Exception as e: + self.progress_tracker.stop_tracking( + tracking_id, status="failed", message=str(e) + ) + raise def export_entities( self, entities: List[Dict[str, Any]], include_metadata: bool = True, **options @@ -263,18 +386,34 @@ class SemanticNetworkYAMLExporter: • Structure for definition generation • Include extraction metadata • Return pipeline-ready YAML + + Args: + extracted_data: Semantic network mapping, read through + :func:`~semantica.utils.helpers.normalize_graph_payload` on + the same terms as :meth:`export_semantic_network`. + pipeline_stage: Stage number recorded in the output. + **options: Additional export options (unused) + + Returns: + Pipeline-ready YAML string. + + Raises: + ProcessingError: if ``extracted_data`` is not a mapping. + ValidationError: on the same mappings as + :meth:`export_semantic_network` -- this method built its + nested semantic network from the same defaulted lookups and + so had the same silent-empty failure. """ + _require_mapping(extracted_data, ("entities", "relationships", "triplets")) + + semantic_network = normalize_graph_payload(extracted_data) yaml_data = { "pipeline_stage": pipeline_stage, "metadata": { "extracted_at": datetime.now().isoformat(), **extracted_data.get("metadata", {}), }, - "semantic_network": { - "entities": extracted_data.get("entities", []), - "relationships": extracted_data.get("relationships", []), - "triplets": extracted_data.get("triplets", []), - }, + "semantic_network": semantic_network, } return self.yaml.dump(yaml_data, default_flow_style=False, sort_keys=False) @@ -308,7 +447,29 @@ class YAMLSchemaExporter: • Include hierarchies and constraints • Structure for easy editing • Return YAML schema + + Args: + ontology: Ontology mapping keyed by any of 'classes', + 'properties', 'namespaces', 'uri', 'title', 'description', + 'version'. + **options: Additional export options (unused) + + Returns: + YAML schema string. + + Raises: + ProcessingError: if ``ontology`` is not a mapping. + ValidationError: if ``ontology`` is a non-empty mapping sharing + no key with the recognized set, or resolves to nothing while + an unread key still holds records + (``{"classes": [], "nodes": [...]}``) -- each previously + produced a file with empty 'classes', 'properties' and + 'namespaces' and no indication anything was dropped. An empty + mapping is still accepted. """ + _require_mapping(ontology, ("classes", "properties")) + _require_usable_schema(ontology) + yaml_data = { "ontology": { "uri": ontology.get("uri", ""), diff --git a/semantica/utils/__init__.py b/semantica/utils/__init__.py index bd0e7e2d..002a139d 100644 --- a/semantica/utils/__init__.py +++ b/semantica/utils/__init__.py @@ -80,6 +80,7 @@ from .helpers import ( hash_data, merge_dicts, normalize_entities, + normalize_graph_payload, parse_timestamp, read_json_file, retry_on_error, @@ -183,6 +184,7 @@ __all__ = [ "format_data", "clean_text", "normalize_entities", + "normalize_graph_payload", "hash_data", "safe_filename", "ensure_directory", diff --git a/semantica/utils/helpers.py b/semantica/utils/helpers.py index 21f6cfc2..7462f6db 100644 --- a/semantica/utils/helpers.py +++ b/semantica/utils/helpers.py @@ -63,9 +63,16 @@ import importlib import json import os import re +import types +from collections import Counter +from collections.abc import Iterable as IterableABC +from collections.abc import Mapping +from dataclasses import asdict, is_dataclass from datetime import datetime, timezone from pathlib import Path -from typing import Any, Dict, List, Optional, Tuple, Type, Union +from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple, Type, Union + +from .exceptions import ProcessingError, ValidationError def format_data(data: Any, format_type: str = "json") -> str: @@ -584,3 +591,367 @@ def classify_path_distance(hop_count: int) -> str: if hop_count <= 6: return "mid-range" return "distant" + + +# Graph payloads circulate under two vocabularies: 'entities'/'relationships' +# (kg builders, most exporters) and 'nodes'/'edges' (ContextGraph.to_dict, +# Neo4jCSVExporter, the Explorer routes). Consumers each reconciled them +# locally, with at least three competing idioms, so the same payload could be +# exported, silently dropped, or rejected depending on which consumer read it. +# This is the single place that decision is made. +_ENTITY_KEYS = ("entities", "nodes") +_RELATIONSHIP_KEYS = ("relationships", "edges") +_TRIPLET_KEYS = ("triplets",) + +# Keys that legitimately travel alongside the collections without being +# records themselves, so their presence is never evidence that records were +# dropped: ContextGraph.to_dict() carries 'statistics', JSON envelopes carry +# 'metadata' and 'count'. +_CONTEXT_KEYS = ("metadata", "statistics", "count") + + +def _require_recognized_keys( + payload: Mapping, recognized_keys: Sequence[str], *, what: str +) -> None: + """Reject a mapping that shares no key with the recognized set. + + A consumer that reads a fixed set of keys turns an unrecognized mapping + into an empty result that looks like a legitimate one. An empty mapping is + allowed through -- it carries nothing that could be lost. + + Args: + payload: Mapping to check. + recognized_keys: Keys the consumer reads. + what: Noun for the error message, e.g. ``"Graph payload"``. + + Raises: + ValidationError: if ``payload`` is non-empty and shares no key with + ``recognized_keys``. + """ + if not payload or any(key in payload for key in recognized_keys): + return + + supplied = ", ".join(f"'{key}'" for key in sorted(map(str, payload))) + expected = ", ".join(f"'{key}'" for key in recognized_keys) + raise ValidationError( + f"{what} has no recognized key. Supplied: {supplied}. " + f"Expected at least one of: {expected}." + ) + + +def _require_nothing_dropped( + payload: Mapping, + recognized_keys: Sequence[str], + resolved: Iterable[Any], + *, + what: str, +) -> None: + """Reject a mapping that resolved to nothing while still holding records. + + Checking that a recognized key is *present* is not enough: + ``{"entities": [], "data": [...]}`` clears that bar and still resolves to + empty, dropping every record under 'data'. Presence answers "did the + caller use our vocabulary"; this answers the question that actually + matters, "did anything the caller supplied survive". + + Only non-empty lists count as evidence of dropped records. A payload can + carry scalars and dicts that are not collections -- ContextGraph.to_dict() + always includes 'statistics' -- and an empty graph must stay exportable. + + Args: + payload: Mapping to check. + recognized_keys: Keys the consumer reads. + resolved: The collections the consumer resolved from ``payload``. + what: Noun for the error message, e.g. ``"Graph payload"``. + + Raises: + ValidationError: if nothing resolved and an unread key holds a + non-empty list. + """ + if any(resolved): + return + + dropped = sorted( + str(key) + for key, value in payload.items() + if key not in recognized_keys + and key not in _CONTEXT_KEYS + and isinstance(value, (list, tuple)) + and value + ) + if not dropped: + return + + named = ", ".join(f"'{key}'" for key in dropped) + expected = ", ".join(f"'{key}'" for key in recognized_keys) + raise ValidationError( + f"{what} resolved to nothing, but {named} still holds records. " + f"Exporting it would drop them silently. Supply the records under " + f"one of: {expected}." + ) + + +def _is_record(value: Any) -> bool: + """Report whether a value can stand in for a graph record. + + Consumers read records either as mappings (``entity.get("type")`` in the + LPG and Arango exporters) or as objects with attributes + (``Neo4jCSVExporter._record_to_dict`` accepts dataclasses and anything + carrying a ``__dict__``). Both are legitimate, so both are accepted here; + strings, numbers, and nested sequences are not records under either + reading. + + Modules and class/type objects are excluded even though they carry + ``__dict__``: they are not graph records under any supported reading, and + passing them through the boundary would produce ``AttributeError`` inside + exporters rather than a ``ValidationError`` at the boundary where the + problem is visible. + """ + return isinstance(value, Mapping) or is_dataclass(value) or ( + hasattr(value, "__dict__") + and not isinstance(value, (types.ModuleType, type)) + ) + + +def _record_to_dict(record: Any) -> Dict[str, Any]: + """Convert an accepted record to a plain dict. + + :func:`_is_record` accepts mappings, dataclasses, and objects carrying + ``__dict__`` as legitimate record shapes, but consumers of + :func:`normalize_graph_payload` -- YAML serialization, ``entity.get(...)`` + in the LPG and Arango exporters -- read records as dicts. Converting here, + at the boundary, means every exporter gets the same shape regardless of + which reading the caller used; previously only ``Neo4jCSVExporter`` + converted object-shaped records locally, so a dataclass record passed + validation for the other exporters only to crash with a raw + ``AttributeError`` once used. + """ + if isinstance(record, Mapping): + return dict(record) + if is_dataclass(record): + return asdict(record) + return { + key: value for key, value in vars(record).items() if not key.startswith("_") + } + + +def _coerce_records(key: str, value: Any) -> List[Any]: + """Validate one collection value and materialize it as a list of records. + + This runs before any truthiness or ``list()`` call, because both mislead + on malformed input: ``list("abc")`` quietly turns a string into three + single-character "records", and ``list(42)`` raises a bare ``TypeError`` + from deep inside the exporter that named the exporter rather than the + offending payload key. Neither reaches the caller as an actionable + message, so the shapes that produce them are rejected by name instead. + + ``None`` is deliberately not rejected: JSON round-trips an absent + collection to null, and treating that as "no records under this key" is + the same answer an explicit ``[]`` gets. It is not silent data loss -- + a null collection alongside records under an unread key is still caught + by :func:`_require_nothing_dropped`. + + Args: + key: Payload key the value came from, for the error message. + value: The raw value stored under ``key``. + + Returns: + The records as a new list, so the result never aliases the input. + + Raises: + ValidationError: if ``value`` is a string, bytes, a mapping, or any + non-iterable scalar; or if any element is not a record. + """ + if value is None: + return [] + + if isinstance(value, (str, bytes, bytearray)): + raise ValidationError( + f"Graph payload key '{key}' holds a {type(value).__name__}, not a " + f"collection of records. Iterating it would yield characters, not " + f"records. Supply a list of records." + ) + + if isinstance(value, Mapping): + raise ValidationError( + f"Graph payload key '{key}' holds a mapping, not a collection of " + f"records. If it is a single record, wrap it in a list; if it is " + f"keyed by ID, supply its values as a list." + ) + + if not isinstance(value, IterableABC): + raise ValidationError( + f"Graph payload key '{key}' holds a " + f"{type(value).__name__}, not a collection of records. Supply a " + f"list of records." + ) + + records = list(value) + for index, record in enumerate(records): + if not _is_record(record): + raise ValidationError( + f"Graph payload key '{key}' holds a " + f"{type(record).__name__} at index {index}, not a record. " + f"Records must be mappings or objects with attributes." + ) + return [_record_to_dict(record) for record in records] + + +def _canonical_record_multiset(records: List[Dict[str, Any]]) -> "Counter[str]": + """Represent records as an order-independent multiset for equality checks. + + Two spellings of the same collection (``entities`` and ``nodes``) can + legitimately list identical records in a different order -- a caller + round-tripping through a dict-keyed cache or a set has no reason to + preserve list order. Comparing with plain list equality would treat that + as a conflict and reject a payload that carries no real data loss, so + records are compared as a multiset of their canonical JSON form instead. + """ + return Counter( + json.dumps(record, sort_keys=True, default=str) for record in records + ) + + +def _resolve_collection( + payload: Dict[str, Any], keys: Tuple[str, ...] +) -> List[Dict[str, Any]]: + """Pick one collection from a payload that may use either vocabulary. + + Both spellings may legitimately be present: ``JSONExporter`` writes + 'entities' and 'nodes' side by side, so a round-trip of its output carries + both, one of them empty. Where only one holds records, that one wins. + + Two non-empty, unequal spellings are a different matter -- there is no + basis for preferring either, and picking one would silently discard the + other -- so that is refused rather than guessed at. + + Every spelling present is validated, not just the one that wins: a + malformed 'nodes' alongside a well-formed 'entities' is a payload the + caller should hear about, and validating only the winner would let it + through on the strength of the other key. + + Args: + payload: Mapping to read from. + keys: Accepted spellings, most canonical first. + + Returns: + The resolved collection, or an empty list if no spelling is present. + + Raises: + ValidationError: if a spelling holds something other than a collection + of records; or if two spellings are both present, both non-empty, + and hold different records, order ignored. + """ + present = { + key: _coerce_records(key, payload[key]) for key in keys if key in payload + } + populated = {key: value for key, value in present.items() if value} + + if len(populated) > 1: + values = list(populated.values()) + canonical = [_canonical_record_multiset(value) for value in values] + if any(entry != canonical[0] for entry in canonical[1:]): + named = " and ".join(f"'{key}'" for key in populated) + raise ValidationError( + f"Graph payload carries {named} with different contents; " + f"cannot determine which to export. Supply one, or make them " + f"identical." + ) + + for key in keys: + value = present.get(key) + if value: + # Already a fresh list from _coerce_records, so the result cannot + # alias the caller's collection. + return value + + # Every spelling present is empty (or none is): an explicit empty + # collection is a legitimate answer, distinct from "unrecognized". + return [] + + +def _require_mapping(data: Any, expected_keys: Sequence[str]) -> None: + """Reject non-mapping export input with an actionable error. + + Shared by every consumer of :func:`normalize_graph_payload` so that a + wrong *type* fails the same way everywhere. Handed a sequence (or any + other non-mapping), every downstream key lookup would fail with a bare + ``AttributeError: 'list' object has no attribute 'get'``, which tells the + caller nothing about the shape expected -- and ``normalize_graph_payload`` + itself raises ``ValidationError`` for this case, which would leave + exporters that skip this guard raising a different exception type than + the ones that call it, for the identical mistake. + + A list is rejected rather than wrapped: these formats distinguish + entities from relationships from triplets (or nodes/edges), so inferring + which one a bare list represents would silently mislabel the records. + + Args: + data: Candidate export payload. + expected_keys: Key names the caller reads, named in the error so the + caller learns the expected shape. + + Raises: + ProcessingError: if ``data`` is not a mapping. + """ + if not isinstance(data, Mapping): + keys = "/".join(f"'{key}'" for key in expected_keys) + raise ProcessingError( + f"Cannot export object of type '{type(data).__name__}': " + f"expected a dict with {keys}." + ) + + +def normalize_graph_payload( + payload: Dict[str, Any], +) -> Dict[str, List[Dict[str, Any]]]: + """Reduce a graph payload to one canonical vocabulary. + + Accepts either 'entities'/'relationships' or 'nodes'/'edges' (or a mix) + and returns the canonical spelling, so consumers read one shape instead of + reimplementing the reconciliation. + + This is the validation boundary for graph payloads: it either returns + collections of records or raises. Nothing that reaches an exporter through + it needs re-checking, and nothing malformed passes through it as a + valid-looking empty graph. + + Args: + payload: Graph payload mapping. + + Returns: + ``{"entities": [...], "relationships": [...], "triplets": [...]}``. + + Raises: + ValidationError: if ``payload`` is not a mapping; if a recognized key + holds something other than a collection of records; if two + spellings of the same collection are both non-empty and differ; if + a non-empty mapping contains no recognized key; or if it resolves + to nothing while an unread key still holds records. The last two + would otherwise hand the caller a valid-looking result with their + records silently dropped. + + Example: + >>> normalize_graph_payload({"nodes": [{"id": "n1"}], "edges": []}) + {'entities': [{'id': 'n1'}], 'relationships': [], 'triplets': []} + """ + if not isinstance(payload, Mapping): + raise ValidationError( + f"Cannot normalize graph payload of type " + f"'{type(payload).__name__}': expected a mapping." + ) + + recognized = _ENTITY_KEYS + _RELATIONSHIP_KEYS + _TRIPLET_KEYS + _require_recognized_keys(payload, recognized, what="Graph payload") + + resolved = { + "entities": _resolve_collection(payload, _ENTITY_KEYS), + "relationships": _resolve_collection(payload, _RELATIONSHIP_KEYS), + "triplets": _resolve_collection(payload, _TRIPLET_KEYS), + } + + _require_nothing_dropped( + payload, recognized, resolved.values(), what="Graph payload" + ) + + return resolved diff --git a/tests/export/test_neo4j_csv_exporter.py b/tests/export/test_neo4j_csv_exporter.py index 529f7bd4..e7f4d0c9 100644 --- a/tests/export/test_neo4j_csv_exporter.py +++ b/tests/export/test_neo4j_csv_exporter.py @@ -301,3 +301,60 @@ def test_nested_properties_are_json_serialized(tmp_path): by_id = {row[0]: row for row in rows[1:]} assert by_id["node1"][2] == '{"k":"v"}' assert by_id["node1"][3] == "[1,2,3]" + + +def test_unrecognized_mapping_is_refused_rather_than_exported_empty(tmp_path): + """The Neo4j path reads mappings on the shared normalizer's default terms. + + An ``export_json`` envelope names no graph key, so it resolves to nothing. + Written out, that is a pair of header-only CSVs indistinguishable from a + genuinely empty graph -- the silent-empty export the shared contract + exists to prevent. + """ + exporter = Neo4jCSVExporter() + + with pytest.raises(ValidationError) as excinfo: + exporter.export({"data": [{"id": "e1"}]}, tmp_path) + + message = str(excinfo.value) + assert "data" in message + assert "entities" in message + + assert not (tmp_path / "nodes.csv").exists() + assert not (tmp_path / "relationships.csv").exists() + + +def test_records_under_an_unread_key_are_not_dropped_silently(tmp_path): + """Naming a recognized key is not enough if nothing resolves from it.""" + exporter = Neo4jCSVExporter() + + with pytest.raises(ValidationError): + exporter.export({"nodes": [], "data": [{"id": "e1"}]}, tmp_path) + + assert not (tmp_path / "nodes.csv").exists() + + +def test_malformed_collection_value_is_refused(tmp_path): + """``list("abc")`` would otherwise export one node per character.""" + exporter = Neo4jCSVExporter() + + for value in ("abc", 42, {"id": "n1"}): + with pytest.raises(ValidationError) as excinfo: + exporter.export({"nodes": value}, tmp_path) + assert "nodes" in str(excinfo.value) + + assert not (tmp_path / "nodes.csv").exists() + + +def test_graph_objects_still_use_the_attribute_path(tmp_path): + """Only mappings changed; objects are not mappings and are unaffected.""" + + class Graph: + def __init__(self): + self.nodes = [{"id": "e1", "type": "Person", "name": "Acme"}] + self.edges = [] + + exporter = Neo4jCSVExporter() + exporter.export(Graph(), tmp_path) + + assert "Acme" in (tmp_path / "nodes.csv").read_text(encoding="utf-8") diff --git a/tests/export/test_yaml_exporter_input_validation.py b/tests/export/test_yaml_exporter_input_validation.py new file mode 100644 index 00000000..13a375d7 --- /dev/null +++ b/tests/export/test_yaml_exporter_input_validation.py @@ -0,0 +1,181 @@ +"""Regression tests for YAML export input validation (issue #952). + +``export_yaml`` declared ``Union[Dict[str, Any], List[Dict[str, Any]]]`` but +both YAML exporters read their payload by key, so a list reached +``semantic_network.get(...)`` and surfaced as a bare +``AttributeError: 'list' object has no attribute 'get'`` from inside the +exporter — an error that names neither the offending argument nor the shape +expected. + +A list is rejected rather than wrapped. These formats distinguish entities +from relationships from triplets, so inferring which collection a bare list +represents would silently mislabel the records; and wrapping it under an +unrecognised key would write a structurally valid file with every collection +empty, trading a loud failure for silent data loss. + +Both directions are pinned: non-mappings raise ``ProcessingError`` with an +actionable message, and every mapping that worked before still exports. +""" + +import os +import shutil +import tempfile +import unittest +from collections import OrderedDict, defaultdict + +import yaml + +from semantica.export.methods import export_yaml +from semantica.export.yaml_exporter import ( + SemanticNetworkYAMLExporter, + YAMLSchemaExporter, +) +from semantica.utils.exceptions import ProcessingError + +# Non-mapping payloads that must be rejected. A list of dicts is the shape +# from #952; the rest guard the same path against other sequence/scalar types. +NON_MAPPINGS = { + "list_of_dicts": [{"id": "1", "name": "Acme"}], + "empty_list": [], + "tuple_of_dicts": ({"id": "1"},), + "list_of_scalars": ["a", "b"], + "string": "entities", + "bytes": b"entities", + "int": 42, + "none": None, + "set": {"a"}, +} + +# Both YAML methods, with a minimal valid payload and the key names the +# corresponding error message must mention. +METHODS = { + "semantic_network": { + "valid": { + "entities": [{"id": "1", "name": "Acme"}], + "relationships": [], + "triplets": [], + }, + "expected_key": "entities", + "top_level_key": "entities", + }, + "schema": { + "valid": {"classes": [{"name": "Thing"}], "properties": []}, + "expected_key": "classes", + "top_level_key": "classes", + }, +} + + +class TestExportYamlRejectsNonMappings(unittest.TestCase): + """Non-mapping input fails loudly, through the public wrapper.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir, ignore_errors=True) + + def _path(self, name="out.yaml"): + return os.path.join(self.tmpdir, name) + + def test_fixture_tables_are_populated(self): + """Guard against a vacuous suite. + + Every test below iterates a table; emptying or renaming one would let + those loops pass without asserting anything. + """ + self.assertGreaterEqual(len(NON_MAPPINGS), 9) + self.assertEqual(set(METHODS), {"semantic_network", "schema"}) + + def test_non_mapping_raises_processing_error(self): + for method in METHODS: + for label, payload in NON_MAPPINGS.items(): + with self.subTest(method=method, case=label): + with self.assertRaises(ProcessingError): + export_yaml(payload, self._path(), method=method) + + def test_error_names_the_offending_type_and_expected_keys(self): + """The message must be actionable, not just the right exception type.""" + for method, spec in METHODS.items(): + with self.subTest(method=method): + with self.assertRaises(ProcessingError) as ctx: + export_yaml([{"id": "1"}], self._path(), method=method) + message = str(ctx.exception) + self.assertIn("list", message) + self.assertIn(spec["expected_key"], message) + + def test_no_file_is_written_when_input_is_rejected(self): + """A rejected export must not leave a partial or empty artefact.""" + for method in METHODS: + with self.subTest(method=method): + path = self._path(f"{method}_rejected.yaml") + with self.assertRaises(ProcessingError): + export_yaml([{"id": "1"}], path, method=method) + self.assertFalse(os.path.exists(path)) + + def test_exporter_classes_reject_non_mappings_directly(self): + """Validation lives in the exporters, not only the convenience wrapper. + + Callers using the classes directly get the same contract. + """ + for label, payload in NON_MAPPINGS.items(): + with self.subTest(exporter="SemanticNetworkYAMLExporter", case=label): + with self.assertRaises(ProcessingError): + SemanticNetworkYAMLExporter().export_semantic_network(payload) + with self.subTest(exporter="YAMLSchemaExporter", case=label): + with self.assertRaises(ProcessingError): + YAMLSchemaExporter().export_ontology_schema(payload) + + +class TestExportYamlStillAcceptsMappings(unittest.TestCase): + """Everything that exported before must still export.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir, ignore_errors=True) + + def _path(self, name="out.yaml"): + return os.path.join(self.tmpdir, name) + + def _load(self, path): + with open(path, encoding="utf-8") as handle: + return yaml.safe_load(handle) + + def test_valid_mapping_exports_for_each_method(self): + for method, spec in METHODS.items(): + with self.subTest(method=method): + path = self._path(f"{method}.yaml") + export_yaml(spec["valid"], path, method=method) + self.assertTrue(os.path.exists(path)) + loaded = self._load(path) + self.assertIn(spec["top_level_key"], loaded) + + def test_semantic_network_records_survive_the_round_trip(self): + path = self._path("network.yaml") + export_yaml(METHODS["semantic_network"]["valid"], path) + loaded = self._load(path) + self.assertEqual(loaded["entities"], [{"id": "1", "name": "Acme"}]) + + def test_empty_mapping_is_still_accepted(self): + """An empty dict is a mapping; rejecting it would be a behaviour change.""" + for method in METHODS: + with self.subTest(method=method): + path = self._path(f"{method}_empty.yaml") + export_yaml({}, path, method=method) + self.assertTrue(os.path.exists(path)) + + def test_mapping_subclasses_are_accepted(self): + """Validation is by Mapping, not dict, so these must keep working.""" + valid = METHODS["semantic_network"]["valid"] + subclasses = { + "OrderedDict": OrderedDict(valid), + "defaultdict": defaultdict(list, valid), + } + for label, payload in subclasses.items(): + with self.subTest(case=label): + path = self._path(f"{label}.yaml") + export_yaml(payload, path) + loaded = self._load(path) + self.assertEqual(loaded["entities"], valid["entities"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/export/test_yaml_exporter_key_recognition.py b/tests/export/test_yaml_exporter_key_recognition.py new file mode 100644 index 00000000..d7dc2839 --- /dev/null +++ b/tests/export/test_yaml_exporter_key_recognition.py @@ -0,0 +1,434 @@ +"""Tests for YAML export key recognition (issue #953). + +``SemanticNetworkYAMLExporter`` built its output from ``.get(key, [])`` +lookups, so a mapping keyed by anything it did not read -- an ``export_json`` +envelope, a typo'd 'entitys', ``ContextGraph.to_dict()``'s 'nodes'/'edges' -- +serialized to a structurally valid file with every collection empty. Nothing +signalled the loss: no exception, no warning, and the progress log reported a +completed export. ``YAMLSchemaExporter`` had the same defect over a different +key set. + +The exporters are run for real rather than mocked, and the written files are +parsed back, since the behaviour under test is what actually lands on disk. +""" + +from pathlib import Path + +import pytest +import yaml + +from semantica.context.context_graph import ContextGraph +from semantica.export.methods import export_json, export_yaml +from semantica.export.yaml_exporter import ( + SemanticNetworkYAMLExporter, + YAMLSchemaExporter, +) +from semantica.utils.exceptions import ProcessingError, ValidationError + +ENTITIES = [{"id": "e1", "name": "Acme"}, {"id": "e2", "name": "Beta"}] +RELATIONSHIPS = [{"id": "r1", "source": "e1", "target": "e2", "type": "PARTNER"}] +TRIPLETS = [{"subject": "e1", "predicate": "partner_of", "object": "e2"}] + + +def _load(path): + with open(path, "r", encoding="utf-8") as handle: + return yaml.safe_load(handle) + + +class TestSemanticNetworkKeyRecognition: + """An unrecognized mapping is refused instead of silently emptied.""" + + def test_export_json_envelope_is_rejected(self, tmp_path): + """The realistic trigger: re-exporting an export_json payload. + + ``export_json`` wraps records as ``{"data": [...], "count": N, + "metadata": {...}}``. Feeding that straight to ``export_yaml`` used to + write a file with every record gone. Note the envelope's 'metadata' + key is deliberately not enough to make the payload recognized -- + treating it as sufficient would readmit exactly this case. + """ + json_path = tmp_path / "records.json" + export_json(ENTITIES, json_path) + envelope = yaml.safe_load(json_path.read_text(encoding="utf-8")) + assert "data" in envelope and "metadata" in envelope + + yaml_path = tmp_path / "records.yaml" + with pytest.raises(ValidationError) as excinfo: + export_yaml(envelope, yaml_path) + + message = str(excinfo.value) + assert "'data'" in message, "error should name the supplied keys" + assert "'entities'" in message, "error should name the expected keys" + assert not yaml_path.exists(), "a rejected export must write nothing" + + @pytest.mark.parametrize( + "payload", + [ + {"records": ENTITIES}, + {"entitys": ENTITIES}, + {"data": ENTITIES}, + {"metadata": {"source": "test"}}, + ], + ids=["records", "typo", "data", "metadata-only"], + ) + def test_unrecognized_mappings_are_rejected(self, payload): + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError): + exporter.export_semantic_network(payload) + + @pytest.mark.parametrize( + "payload", + [ + {"entities": [], "data": ENTITIES}, + {"nodes": [], "edges": [], "records": ENTITIES}, + {"triplets": [], "data": ENTITIES, "metadata": {"source": "test"}}, + ], + ids=["entities-empty", "nodes-edges-empty", "triplets-empty"], + ) + def test_recognized_but_empty_with_records_elsewhere_is_rejected(self, payload): + """Presence of a recognized key is not proof the records survived. + + ``{"entities": [], "data": [...]}`` clears a presence-only check and + still resolves to empty, dropping everything under 'data' -- the same + silent-empty export by a narrower route. + """ + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError) as excinfo: + exporter.export_semantic_network(payload) + + message = str(excinfo.value) + assert "holds records" in message + assert "'entities'" in message, "error should name where records belong" + + def test_empty_graph_with_non_record_keys_still_exports(self, tmp_path): + """The rejection must key on dropped *records*, not on unread keys. + + ``ContextGraph.to_dict()`` always carries a populated 'statistics' + dict, so an empty graph would be refused if any unread key counted. + """ + graph = ContextGraph() + path = tmp_path / "empty_graph.yaml" + export_yaml(graph.to_dict(), path) + + written = _load(path) + assert written["entities"] == [] + assert written["relationships"] == [] + + def test_empty_mapping_still_exports(self, tmp_path): + """An empty graph is legitimate and carries nothing that could be lost.""" + path = tmp_path / "empty.yaml" + export_yaml({}, path) + + written = _load(path) + assert written["entities"] == [] + assert written["relationships"] == [] + assert written["triplets"] == [] + + def test_recognized_keys_still_export(self, tmp_path): + path = tmp_path / "network.yaml" + export_yaml( + { + "entities": ENTITIES, + "relationships": RELATIONSHIPS, + "triplets": TRIPLETS, + "metadata": {"source": "test"}, + }, + path, + ) + + written = _load(path) + assert written["entities"] == ENTITIES + assert written["relationships"] == RELATIONSHIPS + assert written["triplets"] == TRIPLETS + assert written["metadata"]["source"] == "test" + + def test_nodes_edges_alias_exports_records(self, tmp_path): + path = tmp_path / "aliased.yaml" + export_yaml({"nodes": ENTITIES, "edges": RELATIONSHIPS}, path) + + written = _load(path) + assert written["entities"] == ENTITIES + assert written["relationships"] == RELATIONSHIPS + + def test_context_graph_to_dict_round_trips(self, tmp_path): + """The most direct path from this library's own graph type to YAML. + + Built from a real ``ContextGraph`` rather than a hand-written + 'nodes'/'edges' dict, so the test breaks if ``to_dict()`` changes + vocabulary. + """ + graph = ContextGraph() + graph.add_node("n1", node_type="Person", content="Alice") + graph.add_node("n2", node_type="Org", content="Acme") + graph.add_edge("n1", "n2", "WORKS_FOR") + + path = tmp_path / "context.yaml" + export_yaml(graph.to_dict(), path) + + written = _load(path) + assert len(written["entities"]) == 2 + assert len(written["relationships"]) == 1 + + def test_conflicting_spellings_are_refused(self): + """Two populated spellings of one collection: no basis to pick either.""" + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError): + exporter.export_semantic_network( + {"entities": ENTITIES, "nodes": [{"id": "other"}]} + ) + + def test_non_mapping_raises_processing_error(self): + """A wrong type is a different failure from a wrong-keyed mapping. + + ProcessingError says the object cannot be exported at all; + ValidationError says the mapping's contents are unusable. Pinned here + so the two do not quietly converge. + """ + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ProcessingError): + exporter.export_semantic_network(ENTITIES) + + def test_rejected_export_creates_no_output_directory(self, tmp_path): + """Validation runs before the output directory is created.""" + target = tmp_path / "nested" / "out.yaml" + exporter = SemanticNetworkYAMLExporter() + + with pytest.raises(ValidationError): + exporter.export({"data": ENTITIES}, target) + + assert not target.parent.exists() + + +class TestPipelineExportKeyRecognition: + """export_for_pipeline read the same defaulted lookups, so it had the bug too.""" + + def test_unrecognized_mapping_is_rejected(self): + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError): + exporter.export_for_pipeline({"data": ENTITIES}) + + def test_non_mapping_raises_processing_error(self): + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ProcessingError): + exporter.export_for_pipeline(ENTITIES) + + def test_aliases_resolve_into_the_semantic_network(self): + exporter = SemanticNetworkYAMLExporter() + written = yaml.safe_load( + exporter.export_for_pipeline({"nodes": ENTITIES, "edges": RELATIONSHIPS}) + ) + + assert written["semantic_network"]["entities"] == ENTITIES + assert written["semantic_network"]["relationships"] == RELATIONSHIPS + + def test_metadata_is_preserved(self): + exporter = SemanticNetworkYAMLExporter() + written = yaml.safe_load( + exporter.export_for_pipeline( + {"entities": ENTITIES, "metadata": {"source": "test"}} + ) + ) + + assert written["metadata"]["source"] == "test" + assert written["semantic_network"]["entities"] == ENTITIES + + +class TestSchemaKeyRecognition: + """method="schema" emitted empty classes/properties/namespaces the same way.""" + + def test_unrecognized_mapping_is_rejected(self, tmp_path): + path = tmp_path / "schema.yaml" + with pytest.raises(ValidationError) as excinfo: + export_yaml({"nodes": [{"id": "1"}]}, path, method="schema") + + message = str(excinfo.value) + assert "'nodes'" in message + assert "'classes'" in message + assert not path.exists() + + def test_non_mapping_raises_processing_error(self): + exporter = YAMLSchemaExporter() + with pytest.raises(ProcessingError): + exporter.export_ontology_schema([{"id": "1"}]) + + def test_recognized_but_empty_with_records_elsewhere_is_rejected(self): + """The schema path had the same presence-only hole.""" + exporter = YAMLSchemaExporter() + with pytest.raises(ValidationError) as excinfo: + exporter.export_ontology_schema({"classes": [], "nodes": [{"id": "1"}]}) + + assert "holds records" in str(excinfo.value) + + def test_ontology_metadata_without_records_still_exports(self): + """A schema described only by its identity is not a dropped export.""" + exporter = YAMLSchemaExporter() + written = yaml.safe_load( + exporter.export_ontology_schema( + {"uri": "http://example.org/o", "classes": []} + ) + ) + + assert written["ontology"]["uri"] == "http://example.org/o" + assert written["classes"] == [] + + def test_empty_mapping_still_exports(self, tmp_path): + path = tmp_path / "schema.yaml" + export_yaml({}, path, method="schema") + + written = _load(path) + assert written["classes"] == [] + assert written["properties"] == [] + assert written["namespaces"] == {} + + @pytest.mark.parametrize( + "payload", + [ + {"classes": ["Person"], "properties": ["WORKS_FOR"]}, + {"namespaces": {"ex": "http://example.org/"}}, + {"uri": "http://example.org/ontology"}, + ], + ids=["classes-properties", "namespaces-only", "uri-only"], + ) + def test_recognized_keys_still_export(self, payload, tmp_path): + path = tmp_path / "schema.yaml" + export_yaml(payload, path, method="schema") + + written = _load(path) + assert written["classes"] == payload.get("classes", []) + assert written["properties"] == payload.get("properties", []) + assert written["ontology"]["uri"] == payload.get("uri", "") + + # ── Fix regression: scalar recognized keys must not short-circuit the ── + # ── dropped-records check (version, uri, title, description). ────────── + + @pytest.mark.parametrize( + "scalar_key, scalar_value", + [ + ("version", "1.0"), + ("uri", "http://example.org/ontology"), + ("title", "My Ontology"), + ("description", "A test ontology"), + ], + ids=["version", "uri", "title", "description"], + ) + def test_scalar_recognized_key_does_not_excuse_records_under_unread_key( + self, scalar_key, scalar_value + ): + """A truthy scalar such as version='1.0' must not silence the dropped- + records check. Before the fix, any truthy value from _SCHEMA_KEYS + would make _require_nothing_dropped believe something resolved and + return early, silently discarding a list under an unread key. + """ + exporter = YAMLSchemaExporter() + with pytest.raises(ValidationError) as excinfo: + exporter.export_ontology_schema( + {scalar_key: scalar_value, "nodes": [{"id": "c1"}]} + ) + assert "holds records" in str(excinfo.value), str(excinfo.value) + + def test_valid_classes_with_scalar_metadata_is_accepted(self): + """classes/properties populated alongside version/uri must still work.""" + exporter = YAMLSchemaExporter() + written = yaml.safe_load( + exporter.export_ontology_schema( + { + "classes": [{"id": "Person"}], + "properties": [{"id": "name"}], + "version": "2.0", + "uri": "http://example.org/o", + } + ) + ) + assert written["classes"] == [{"id": "Person"}] + assert written["properties"] == [{"id": "name"}] + assert written["ontology"]["version"] == "2.0" + assert written["ontology"]["uri"] == "http://example.org/o" + + +class TestFailureIsObservable: + """The complaint in #953 was that the logs affirmatively reported success.""" + + def test_no_success_is_logged_for_a_rejected_export(self, tmp_path, caplog): + path = tmp_path / "out.yaml" + + with caplog.at_level("DEBUG"): + with pytest.raises(ValidationError): + export_yaml({"data": ENTITIES}, path) + + assert "Exported YAML to" not in caplog.text + assert any( + record.levelname in ("WARNING", "ERROR", "CRITICAL") + for record in caplog.records + ), "a rejected export should leave something at warning or above" + + +class _RecordingTracker: + """Records the exporter's own progress calls, which are what is under test.""" + + def __init__(self): + self.stopped = [] + self._next_id = 0 + + def start_tracking(self, **kwargs): + self._next_id += 1 + return str(self._next_id) + + def update_tracking(self, tracking_id, **kwargs): + pass + + def stop_tracking(self, tracking_id, status=None, message=None): + self.stopped.append((status, message)) + + +class TestProgressReflectsTheWrite: + """Serialization completing is not the same as the file landing on disk.""" + + def test_failed_write_is_not_reported_as_completed(self, tmp_path): + """A write failure after serialization must not leave a clean tracker. + + The path's parent is an existing *file*, so directory creation fails + after `export_semantic_network` has already reported its own + completion. + """ + blocker = tmp_path / "blocker" + blocker.write_text("not a directory", encoding="utf-8") + target = blocker / "nested" / "out.yaml" + + exporter = SemanticNetworkYAMLExporter() + tracker = _RecordingTracker() + exporter.progress_tracker = tracker + + with pytest.raises(OSError): + exporter.export({"entities": ENTITIES}, target) + + assert not target.exists() + statuses = [status for status, _ in tracker.stopped] + assert "failed" in statuses, f"write failure went unreported: {tracker.stopped}" + assert not any( + status == "completed" and "Exported YAML" in (message or "") + for status, message in tracker.stopped + ), "no span may claim a completed export when nothing was written" + + def test_successful_write_is_reported_as_completed(self, tmp_path): + target = tmp_path / "out.yaml" + exporter = SemanticNetworkYAMLExporter() + tracker = _RecordingTracker() + exporter.progress_tracker = tracker + + exporter.export({"entities": ENTITIES}, target) + + assert target.exists() + assert all(status == "completed" for status, _ in tracker.stopped) + assert any( + "Exported YAML" in (message or "") for _, message in tracker.stopped + ), "the write should report its own completion, not just serialization" + + +class TestUnaffectedExporters: + """export_json's own behaviour is untouched -- only the YAML path changed.""" + + def test_export_json_still_accepts_a_bare_list(self, tmp_path): + path = tmp_path / "records.json" + export_json(ENTITIES, path) + + assert Path(path).exists() diff --git a/tests/utils/test_normalize_graph_payload.py b/tests/utils/test_normalize_graph_payload.py new file mode 100644 index 00000000..02b53219 --- /dev/null +++ b/tests/utils/test_normalize_graph_payload.py @@ -0,0 +1,487 @@ +"""Tests for the shared graph-payload normalizer (issue #956). + +Graph payloads circulate under two vocabularies -- 'entities'/'relationships' +and 'nodes'/'edges' -- and consumers each reconciled them locally with at +least three competing idioms. The same payload could therefore be exported, +silently dropped, or rejected depending on which consumer read it: +``export_lpg`` dropped every entity when 'nodes' was present but empty, which +is precisely the shape ``JSONExporter`` emits. + +The end-to-end assertions run the real exporters rather than mocking them, +since the behaviour under test is that the exporters now agree. +""" + +import os +import shutil +import tempfile +import unittest +from dataclasses import dataclass + +from semantica.export import methods as export_methods +from semantica.utils import normalize_graph_payload +from semantica.utils.exceptions import ValidationError + +ENTITY = {"id": "e1", "name": "Acme"} +RELATIONSHIP = {"id": "r1", "source": "e1", "target": "e2"} + + +class TestVocabularyResolution(unittest.TestCase): + def test_canonical_keys_pass_through(self): + result = normalize_graph_payload( + {"entities": [ENTITY], "relationships": [RELATIONSHIP]} + ) + self.assertEqual(result["entities"], [ENTITY]) + self.assertEqual(result["relationships"], [RELATIONSHIP]) + self.assertEqual(result["triplets"], []) + + def test_aliases_are_mapped_to_canonical_keys(self): + result = normalize_graph_payload({"nodes": [ENTITY], "edges": [RELATIONSHIP]}) + self.assertEqual(result["entities"], [ENTITY]) + self.assertEqual(result["relationships"], [RELATIONSHIP]) + + def test_empty_alias_does_not_mask_a_populated_canonical_key(self): + """The JSONExporter round-trip shape, and the #956 data-loss case.""" + result = normalize_graph_payload( + {"entities": [ENTITY], "nodes": [], "relationships": [], "edges": []} + ) + self.assertEqual(result["entities"], [ENTITY]) + + def test_empty_canonical_key_does_not_mask_a_populated_alias(self): + result = normalize_graph_payload({"entities": [], "nodes": [ENTITY]}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_identical_spellings_are_accepted(self): + result = normalize_graph_payload({"entities": [ENTITY], "nodes": [ENTITY]}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_conflicting_spellings_are_refused(self): + """No basis to prefer either, and picking one would lose the other.""" + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload( + {"entities": [ENTITY], "nodes": [{"id": "different"}]} + ) + message = str(ctx.exception) + self.assertIn("entities", message) + self.assertIn("nodes", message) + + def test_reordered_identical_spellings_are_accepted(self): + """Same records, different order, is not a conflict. + + A caller round-tripping through a dict-keyed cache or a set has no + reason to preserve list order; comparing spellings with plain list + equality rejected this as if the records differed. + """ + other = {"id": "e2", "name": "Beta"} + result = normalize_graph_payload( + {"entities": [ENTITY, other], "nodes": [other, ENTITY]} + ) + self.assertCountEqual(result["entities"], [ENTITY, other]) + + def test_reordered_spellings_with_duplicate_records_still_conflict(self): + """Multiset comparison must still catch a real count mismatch.""" + with self.assertRaises(ValidationError): + normalize_graph_payload({"entities": [ENTITY, ENTITY], "nodes": [ENTITY]}) + + def test_triplets_are_carried_through(self): + result = normalize_graph_payload({"triplets": [{"s": "a", "p": "b", "o": "c"}]}) + self.assertEqual(result["triplets"], [{"s": "a", "p": "b", "o": "c"}]) + + def test_missing_collections_default_to_empty_lists(self): + result = normalize_graph_payload({"entities": [ENTITY]}) + self.assertEqual(result["relationships"], []) + self.assertEqual(result["triplets"], []) + + def test_result_does_not_alias_the_input_collections(self): + payload = {"entities": [ENTITY]} + result = normalize_graph_payload(payload) + result["entities"].append({"id": "e2"}) + self.assertEqual(len(payload["entities"]), 1) + + +class TestUnrecognizedInput(unittest.TestCase): + def test_unrecognized_keys_raise_by_default(self): + for payload in ({"data": [ENTITY]}, {"records": [ENTITY]}, {"foo": "bar"}): + with self.subTest(payload=payload): + with self.assertRaises(ValidationError): + normalize_graph_payload(payload) + + def test_error_names_supplied_and_expected_keys(self): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"data": [ENTITY]}) + message = str(ctx.exception) + self.assertIn("data", message) + self.assertIn("entities", message) + self.assertIn("nodes", message) + + def test_empty_mapping_is_accepted(self): + """An empty graph is legitimate and carries nothing that could be lost.""" + result = normalize_graph_payload({}) + self.assertEqual(result, {"entities": [], "relationships": [], "triplets": []}) + + def test_non_mapping_input_raises(self): + for payload in ([ENTITY], (ENTITY,), "entities", 42, None): + with self.subTest(payload=repr(payload)): + with self.assertRaises(ValidationError): + normalize_graph_payload(payload) + + +class TestExportersAgree(unittest.TestCase): + """The divergence from #956, run against the real exporters.""" + + # export_csv is excluded: it writes entities/relationships/nodes/edges to + # four separate files by design, so it is not resolving two spellings of + # one collection and is out of scope for this change. + EXPORTERS = ("export_json", "export_arango", "export_neo4j_csv", "export_lpg") + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir, ignore_errors=True) + + def _export_and_read(self, name, payload): + outdir = os.path.join(self.tmpdir, name) + os.makedirs(outdir, exist_ok=True) + getattr(export_methods, name)(payload, os.path.join(outdir, "out")) + blob = "" + for root, _, files in os.walk(outdir): + for filename in files: + with open(os.path.join(root, filename), errors="ignore") as handle: + blob += handle.read() + return blob + + def test_exporter_list_is_populated(self): + """Guard against a vacuous suite if the list is emptied.""" + self.assertGreaterEqual(len(self.EXPORTERS), 4) + + def test_every_exporter_keeps_records_when_an_alias_is_empty(self): + payload = { + "entities": [ENTITY], + "nodes": [], + "relationships": [], + "edges": [], + } + for name in self.EXPORTERS: + with self.subTest(exporter=name): + self.assertIn( + "Acme", + self._export_and_read(name, payload), + f"{name} dropped the entity when 'nodes' was present but empty", + ) + + def test_every_exporter_accepts_the_alias_vocabulary(self): + payload = {"nodes": [ENTITY], "edges": []} + for name in self.EXPORTERS: + with self.subTest(exporter=name): + self.assertIn( + "Acme", + self._export_and_read(name, payload), + f"{name} dropped the entity supplied as 'nodes'", + ) + + def test_every_exporter_raises_processing_error_for_non_mapping_input(self): + """A wrong-type payload is rejected the same way everywhere. + + export_yaml and export_neo4j_csv raised ProcessingError for a bare + list; export_lpg and export_arango called normalize_graph_payload() + directly with no type guard, so they alone raised ValidationError + (from inside the resolver) for the identical mistake. + """ + from semantica.utils.exceptions import ProcessingError + + for name in ("export_arango", "export_neo4j_csv", "export_lpg"): + with self.subTest(exporter=name): + outdir = os.path.join(self.tmpdir, name + "_bad_type") + os.makedirs(outdir, exist_ok=True) + with self.assertRaises(ProcessingError): + getattr(export_methods, name)([ENTITY], os.path.join(outdir, "out")) + + def test_every_exporter_converts_object_shaped_records(self): + """A dataclass record must not merely pass validation. + + normalize_graph_payload() accepts dataclass/attribute-bearing + records (Neo4jCSVExporter reads them off attributes), but + export_lpg and export_arango read records with ``.get(...)``. A + record that passed validation unconverted crashed with a raw + AttributeError once used -- the exact failure the boundary exists + to prevent. + """ + + @dataclass + class Node: + id: str + name: str + + payload = {"entities": [Node(id="e1", name="Acme")], "relationships": []} + for name in ("export_arango", "export_neo4j_csv", "export_lpg"): + with self.subTest(exporter=name): + self.assertIn("Acme", self._export_and_read(name, payload)) + + def test_neo4j_accepts_non_dict_mappings(self): + """Neo4jCSVExporter's mapping path must not be narrower than the rest. + + _normalize_graph checked isinstance(graph, dict), so a non-dict + Mapping (a MappingProxyType, a ChainMap) fell into the + object-attribute branch and was rejected as an unrecognized object, + even though the identical payload exports fine via LPG/Arango/YAML. + """ + import types + + payload = types.MappingProxyType({"entities": [ENTITY], "relationships": []}) + self.assertIn("Acme", self._export_and_read("export_neo4j_csv", payload)) + + +class TestRecordsCannotBeDroppedSilently(unittest.TestCase): + """Presence of a recognized key is not proof the records survived. + + ``{"entities": [], "data": [...]}`` clears a presence-only check and still + resolves to empty, so the records under 'data' would be dropped with no + signal -- the same failure the recognition check exists to prevent. + """ + + def test_empty_recognized_key_does_not_excuse_records_elsewhere(self): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [], "data": [ENTITY]}) + + message = str(ctx.exception) + self.assertIn("'data'", message) + self.assertIn("holds records", message) + + def test_check_applies_to_every_recognized_spelling(self): + for key in ("entities", "nodes", "relationships", "edges", "triplets"): + with self.subTest(key=key): + with self.assertRaises(ValidationError): + normalize_graph_payload({key: [], "records": [ENTITY]}) + + def test_non_record_keys_are_not_mistaken_for_dropped_records(self): + """ContextGraph.to_dict() always carries 'statistics'. + + An empty graph must stay exportable, so only a non-empty list counts + as evidence that records were dropped. + """ + result = normalize_graph_payload( + {"nodes": [], "edges": [], "statistics": {"node_count": 0}} + ) + + self.assertEqual(result["entities"], []) + self.assertEqual(result["relationships"], []) + + def test_records_alongside_a_populated_collection_are_not_refused(self): + """Something resolved, so the export is not silently empty.""" + result = normalize_graph_payload({"entities": [ENTITY], "statistics": {"n": 1}}) + + self.assertEqual(result["entities"], [ENTITY]) + + +class TestCollectionValuesAreValidated(unittest.TestCase): + """A recognized key is not proof its value is a collection of records. + + Resolving on truthiness alone let ``{"entities": "abc"}`` through as three + single-character "records" and let ``{"entities": 42}`` surface as a raw + ``TypeError`` from ``list()`` inside an exporter, naming the exporter + rather than the payload key at fault. Both are rejected here, at the + boundary that owns the question. + """ + + COLLECTION_KEYS = ("entities", "nodes", "relationships", "edges", "triplets") + + # Every public export path that reads its payload through the normalizer. + # export_json is excluded: it treats the payload as opaque records rather + # than resolving graph collections, so it never calls the normalizer. + NORMALIZING_EXPORTERS = ( + "export_arango", + "export_neo4j_csv", + "export_lpg", + "export_yaml", + ) + + def test_string_value_is_not_treated_as_a_collection(self): + for key in self.COLLECTION_KEYS: + with self.subTest(key=key): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({key: "abc"}) + message = str(ctx.exception) + self.assertIn(f"'{key}'", message) + self.assertIn("str", message) + + def test_bytes_value_is_not_treated_as_a_collection(self): + for value in (b"abc", bytearray(b"abc")): + with self.subTest(value=repr(value)): + with self.assertRaises(ValidationError): + normalize_graph_payload({"entities": value}) + + def test_scalar_value_raises_validation_error_not_type_error(self): + for key in self.COLLECTION_KEYS: + for value in (42, 3.5, True, object()): + with self.subTest(key=key, value=repr(value)): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({key: value}) + self.assertIn(f"'{key}'", str(ctx.exception)) + + def test_mapping_value_is_not_treated_as_a_collection(self): + """``{"nodes": {"id": "n1"}}`` -- a single record, or an ID index.""" + for payload in ( + {"nodes": {"id": "n1"}}, + {"entities": {"e1": ENTITY}}, + {"edges": {"id": "r1"}}, + ): + with self.subTest(payload=payload): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload(payload) + self.assertIn("mapping", str(ctx.exception)) + + def test_non_record_elements_are_rejected(self): + for value in (["Acme"], [ENTITY, "Acme"], [42], [None], [[ENTITY]]): + with self.subTest(value=repr(value)): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": value}) + self.assertIn("'entities'", str(ctx.exception)) + + def test_error_names_the_offending_index(self): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [ENTITY, ENTITY, "Acme"]}) + self.assertIn("index 2", str(ctx.exception)) + + def test_object_records_are_accepted(self): + """Attribute-bearing objects are accepted and converted to dicts. + + LPGExporter and ArangoAQLExporter read records with ``.get(...)``, so + an object record that merely passed validation unconverted would + still crash with AttributeError once used; the boundary converts it. + """ + + class Node: + def __init__(self): + self.id = "e1" + self.name = "Acme" + + node = Node() + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "e1", "name": "Acme"}]) + + def test_dataclass_records_are_accepted(self): + @dataclass + class Node: + id: str + + node = Node(id="e1") + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "e1"}]) + + def test_tuple_collections_are_accepted_and_materialized(self): + result = normalize_graph_payload({"entities": (ENTITY,)}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_none_is_read_as_an_absent_collection(self): + """JSON round-trips an absent collection to null.""" + result = normalize_graph_payload( + {"entities": None, "relationships": [RELATIONSHIP]} + ) + self.assertEqual(result["entities"], []) + self.assertEqual(result["relationships"], [RELATIONSHIP]) + + def test_null_collection_still_cannot_hide_dropped_records(self): + with self.assertRaises(ValidationError): + normalize_graph_payload({"entities": None, "data": [ENTITY]}) + + def test_every_spelling_is_validated_not_just_the_winner(self): + """A malformed alias is a defect even when the canonical key resolves.""" + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [ENTITY], "nodes": "abc"}) + self.assertIn("'nodes'", str(ctx.exception)) + + def test_malformed_value_reaches_no_exporter(self): + """The end-to-end half: no exporter sees a TypeError from list().""" + tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmpdir, ignore_errors=True) + + for name in self.NORMALIZING_EXPORTERS: + for value in ("abc", 42, {"id": "n1"}): + with self.subTest(exporter=name, value=repr(value)): + outdir = os.path.join(tmpdir, f"{name}_{type(value).__name__}") + os.makedirs(outdir, exist_ok=True) + with self.assertRaises(ValidationError): + getattr(export_methods, name)( + {"entities": value}, os.path.join(outdir, "out") + ) + + +class TestIsRecordBoundary(unittest.TestCase): + """_is_record gates the validation boundary introduced by this PR. + + Modules and class/type objects carry ``__dict__`` but are not graph + records. Passing them through previously produced ``AttributeError`` + inside exporters rather than a ``ValidationError`` at the boundary. + """ + + def test_python_module_in_entities_raises_validation_error(self): + """import math; {"entities": [math]} must be rejected at the boundary.""" + import math + + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [math]}) + self.assertIn("'entities'", str(ctx.exception)) + + def test_class_object_in_entities_raises_validation_error(self): + """A class (type object) is not a graph record.""" + + class MyNode: + pass + + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [MyNode]}) + self.assertIn("'entities'", str(ctx.exception)) + + def test_user_defined_instance_with_attributes_is_accepted(self): + """Attribute-bearing instances are the legitimate use-case, converted + to a dict so every exporter -- not just Neo4jCSVExporter -- can read + it with ``.get(...)``.""" + + class Node: + def __init__(self): + self.id = "n1" + self.name = "Alice" + + node = Node() + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "n1", "name": "Alice"}]) + + def test_dataclass_instance_is_accepted(self): + """Dataclasses are a common record type used by Neo4jCSVExporter, + converted to a dict at the boundary so LPGExporter and + ArangoAQLExporter can read it too.""" + node = dataclass_node() + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "dc1"}]) + + def test_mapping_record_is_accepted(self): + """Plain dicts are the canonical record shape.""" + result = normalize_graph_payload({"entities": [ENTITY]}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_module_rejected_through_normalizing_exporter(self): + """End-to-end: a module element must not reach an exporter's internals.""" + import math + + tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmpdir, ignore_errors=True) + + for name in ("export_arango", "export_neo4j_csv", "export_lpg"): + with self.subTest(exporter=name): + outdir = os.path.join(tmpdir, name) + os.makedirs(outdir, exist_ok=True) + with self.assertRaises(ValidationError): + getattr(export_methods, name)( + {"entities": [math]}, os.path.join(outdir, "out") + ) + + +@dataclass +class _DataclassNode: + id: str + + +def dataclass_node(): + return _DataclassNode(id="dc1") + + +if __name__ == "__main__": + unittest.main()