diff --git a/docs/extending/components.md b/docs/extending/components.md index 6498139f..c1067a3d 100644 --- a/docs/extending/components.md +++ b/docs/extending/components.md @@ -80,9 +80,9 @@ destinations a source writes to, a job's targets, an upstream asset. | `kind` | The component kind, or kinds, the relation may point at. | | `key` | The keys it narrows to: an exact key, `source.asset`, `*.asset`, a list, or `""` for any key of those kinds. | | `many` | Whether it binds several components at once. | -| `optional` | Whether it may stay unbound. Says nothing about data. | +| `optional` | Whether it may stay unbound, or be emptied once bound. Says nothing about data, and nothing about deletion. | | `default` | A zero-argument factory producing the value an unbound relation resolves to. | -| `on_delete` | What deleting the target does to the referrer: `block` (default, for consumption relations) or `detach` (for orchestration pointers such as a job's targets or a hook's watches). | +| `on_delete` | What deleting a bound target does, and the only field that decides it: `block` (the default) refuses the deletion while this relation holds the target, `detach` lets it through and drops the binding. Orchestration pointers (a job's `targets`, a hook's `watches`) and inferred optional upstreams declare `detach`; consumption relations keep `block`, an optional one included. | Two attributes are derived, not declared: `name` is stamped from the attribute the relation is declared under, and `target` is the class the `il.Relation(cls)` shorthand was written with, diff --git a/docs/guide/dependencies.md b/docs/guide/dependencies.md index a4a416d9..e420e57f 100644 --- a/docs/guide/dependencies.md +++ b/docs/guide/dependencies.md @@ -30,7 +30,10 @@ class Shop(il.Source): The parameter name is the relation name and the bare asset key it expects, so `orders: il.Upstream` declares `il.Relation("asset", "orders")`, a sibling of the same source -instance. A `None` default makes the relation **optional**, meaning its wiring may be absent: +instance. A `None` default makes the relation **optional**, meaning its wiring may be absent, and +declares `on_delete="detach"` with it: a parameter that tolerates a missing leg has no claim on +the upstream, so deleting that upstream is allowed and simply drops the wiring, where a required +upstream refuses the deletion while it is bound. ```py @il.asset(partitioning=il.TimePartitionConfig(column="date")) diff --git a/docs/guide/resources.md b/docs/guide/resources.md index 10b80093..0aff52f9 100644 --- a/docs/guide/resources.md +++ b/docs/guide/resources.md @@ -79,6 +79,9 @@ class WarehouseDestination(il.Destination): `il.Relation(ReportingConfig)` is shorthand for `il.Relation(kind="config", key="reporting_config")` with the class kept as the relation's `target`, which is what makes a fallback possible. `optional=True` allows the relation to stay unbound; `default=` is a zero-argument factory. +Neither says anything about deletion: `on_delete` alone decides that, and a resource relation +keeps its `block` default whether it is optional or not, so a bound connection, config or +destination cannot be deleted while a component consumes it. ## Fallbacks diff --git a/packages/interloper-agent/src/interloper_agent/agent.py b/packages/interloper-agent/src/interloper_agent/agent.py index 71305a6c..137fa4d8 100644 --- a/packages/interloper-agent/src/interloper_agent/agent.py +++ b/packages/interloper-agent/src/interloper_agent/agent.py @@ -117,9 +117,10 @@ def _catalog_tools() -> list: description=( "The organisation's collection of component instances: lists their sources, connections, and " "destinations, checks connection health, sets up new connections via the app's secure form " - "(never collecting credentials in chat), creates sources conversationally — resolving " - "provider-backed options like the account to use through an existing connection — and edits " - "existing components (rename, config changes, a source's enabled assets)." + "(never collecting credentials in chat), creates sources conversationally (resolving " + "provider-backed options like the account to use through an existing connection), edits " + "existing components (rename, config changes, a source's enabled assets), and binds or " + "unbinds a component's relations by name (a source's connection, a job's targets)." ), instruction=with_current_time(COLLECTION_INSTRUCTION), tools=[ @@ -131,6 +132,8 @@ def _catalog_tools() -> list: collection.create_source, collection.create_sources, collection.update_component, + collection.bind_relation, + collection.unbind_relation, collection.create_job, interaction.request_user_selection, interaction.request_confirmation, diff --git a/packages/interloper-agent/src/interloper_agent/prompts.py b/packages/interloper-agent/src/interloper_agent/prompts.py index 661abe10..a3d6a467 100644 --- a/packages/interloper-agent/src/interloper_agent/prompts.py +++ b/packages/interloper-agent/src/interloper_agent/prompts.py @@ -110,7 +110,7 @@ 6. Recap with request_confirmation (what and how many; accounts, assets, connection, destination — "None" when none), then create_sources on confirm. Report per-account failures and any unresolved cross-source - requirements (those are wired in the app). + requirements, and offer to bind them once the user names the upstream. 7. Offer a schedule: recap the job (name, cadence in words, targets) and create_job on confirm. @@ -124,6 +124,14 @@ just the delta. Connection credentials are never edited in chat — offer a rename, or point the user to the app (or a fresh connection via the secure form) for credential changes. + +Bind what a component points at with bind_relation, and detach it with +unbind_relation: both take the component's id, the relation name its class +declares (list_components names the component, the catalog specialist the +names it declares), and the target's id. A single-valued name repoints, so +it needs no unbind first; a non-optional one cannot be emptied, only +repointed. Recap the component, the name, and old → new, and act only on +confirmation. """ + PRESENTATION CATALOG_CONSULT_INSTRUCTION = """\ diff --git a/packages/interloper-agent/src/interloper_agent/tools/collection.py b/packages/interloper-agent/src/interloper_agent/tools/collection.py index 4e2c17d9..09b73088 100644 --- a/packages/interloper-agent/src/interloper_agent/tools/collection.py +++ b/packages/interloper-agent/src/interloper_agent/tools/collection.py @@ -67,8 +67,8 @@ def update_component( the stored config is kept; pass null to reset a field to its default. Connection configs hold credentials and are never edited here: the user changes those in the app (renaming a connection is fine). Rebinding - relations (a source's connection, a job's targets) also happens in the - app, not through config. + relations (a source's connection, a job's targets) is not config either: + use bind_relation and unbind_relation. Args: component_id: UUID of the component, from list_components. @@ -144,6 +144,41 @@ def update_component( return {"status": "error", "error": str(e)} +# -- Relations (generic over kinds) ----------------------------------------------- + + +def bind_relation( + component_id: str, + name: str, + dst_id: str, + tool_context: ToolContext | None = None, +) -> dict[str, Any]: + # Thin ADK wrapper: the implementation (and LLM-facing docstring, adopted + # below) lives in the shared toolkit, which the MCP server deliberately + # does not register, since it writes. + return toolkit_collection.bind_relation(toolkit_ctx(tool_context), component_id, name, dst_id).model_dump( + mode="json" + ) + + +bind_relation.__doc__ = toolkit_collection.bind_relation.__doc__ + + +def unbind_relation( + component_id: str, + name: str, + dst_id: str, + tool_context: ToolContext | None = None, +) -> dict[str, Any]: + # Thin ADK wrapper: see bind_relation above. + return toolkit_collection.unbind_relation(toolkit_ctx(tool_context), component_id, name, dst_id).model_dump( + mode="json" + ) + + +unbind_relation.__doc__ = toolkit_collection.unbind_relation.__doc__ + + # -- Connection operations (kind-specific by nature) ------------------------------ @@ -501,46 +536,87 @@ def _source_relations( source_key: str, connection_id: str | None, destination_ids: list[str] | None, -) -> tuple[dict[str, list[tuple[UUID, str]]] | None, dict[str, Any] | None]: - """Bind the connection into the definition's resource slot, plus destinations. +) -> tuple[dict[str, list[UUID]] | None, dict[str, Any] | None]: + """Bind the connection into the definition's named relation, plus destinations. + + Finds the relations the definition declares whose ``kind`` includes + ``"connection"``. A given connection binds under whichever one it fits + (an empty ``key`` accepts any connection, otherwise the connection's key + must appear in the relation's key list); every non-optional connection + relation left unbound afterwards is an error naming the relation and the + key it expects. Destinations bind under the fixed ``"destinations"`` name + once each one is confirmed to belong to the organisation. + + Args: + store: The database store, for looking up the connection and + destination rows. + org_id: Organisation UUID the connection and every destination must + belong to. + defn: The source's catalog definition, carrying its declared + ``relations`` (name to relation dict: ``kind``, ``key``, ``many``, + ``optional``, ``on_delete``, ``name``). + source_key: The source definition's catalog key, named in error + messages. + connection_id: UUID of the connection to bind, or ``None`` to leave + every connection relation unbound. + destination_ids: UUIDs of the destinations to attach, or ``None``. Returns: - ``(relations, None)`` or ``(None, error_response)``. + ``(relations, None)``, ``relations`` mapping relation name to the + UUIDs bound under it (ready for ``ComponentStore.create``'s + ``relations`` argument), or ``(None, error_response)``. """ - slots = (((defn.get("relations") or {}).get("resource") or {}).get("slots")) or {} + relations_defn = defn.get("relations") or {} + connection_relations = { + name: relation + for name, relation in relations_defn.items() + if "connection" in (relation["kind"] if isinstance(relation["kind"], list) else [relation["kind"]]) + } + connection = None if connection_id is not None: connection = store.components.get(UUID(connection_id), kind="connection") if connection.org_id != org_id: return None, {"status": "error", "error": f"Connection '{connection_id}' not found"} - resource_bindings: list[tuple[UUID, str]] = [] - for slot_name, spec in slots.items(): - expected = spec.get("key") - if connection is not None and (not expected or connection.key == expected): - resource_bindings.append((connection.id, slot_name)) - connection = None - elif spec.get("required"): + + bindings: dict[str, list[UUID]] = {} + if connection is not None: + name = next( + ( + relation_name + for relation_name, relation in connection_relations.items() + if not relation.get("key") + or connection.key in ([relation["key"]] if isinstance(relation["key"], str) else relation["key"]) + ), + None, + ) + if name is None: + return None, { + "status": "error", + "error": f"Connection '{connection.key}' does not fit any relation of '{source_key}'", + } + bindings[name] = [connection.id] + + for name, relation in connection_relations.items(): + if not relation.get("optional") and name not in bindings: + expected = relation.get("key") or "connection" return None, { "status": "error", "error": ( - f"'{source_key}' requires a '{expected or 'connection'}' in slot '{slot_name}' — " + f"'{source_key}' requires a '{expected}' as '{name}'; " "pick one from the collection or set one up first" ), } - if connection is not None: - return None, { - "status": "error", - "error": f"Connection '{connection.key}' does not fit any slot of '{source_key}'", - } - destination_bindings: list[tuple[UUID, str]] = [] + destination_bindings: list[UUID] = [] for dest_id in destination_ids or []: dest = store.components.get(UUID(dest_id), kind="destination") if dest.org_id != org_id: return None, {"status": "error", "error": f"Destination '{dest_id}' not found"} - destination_bindings.append((dest.id, "")) + destination_bindings.append(dest.id) + bindings["destinations"] = destination_bindings - return {"resource": resource_bindings, "destination": destination_bindings}, None + return bindings, None def _unresolved_requirements(defn: dict[str, Any], row: Any) -> list[str]: @@ -576,8 +652,8 @@ def create_source( name: Display name — default to the label of the chosen account / discriminator option. config: Values for the definition's config schema (e.g. account_id). - connection_id: UUID of the connection to bind — required when the - definition declares a required connection slot. + connection_id: UUID of the connection to bind; required when the + definition declares a required connection relation. asset_keys: Child asset keys to enable; omit to enable all. destination_ids: Destination UUIDs to attach (optional). """ @@ -619,8 +695,8 @@ def create_source( "key": row.key, "name": row.name, "asset_count": len(row.children), - "connection_bound": bool(relations["resource"]), - "destination_count": len(relations["destination"]), + "connection_bound": connection_id is not None, + "destination_count": len(relations["destinations"]), }, "unresolved_requirements": _unresolved_requirements(defn, row), } @@ -765,12 +841,12 @@ def create_job( org_id = get_org_id(tool_context) store = get_store() - targets: list[tuple[UUID, str]] = [] + targets: list[UUID] = [] for source_id in target_source_ids: source = store.components.get(UUID(source_id), kind="source") if source.org_id != org_id: return {"status": "error", "error": f"Source '{source_id}' not found"} - targets.append((source.id, "")) + targets.append(source.id) if not targets: return {"status": "error", "error": "target_source_ids must name at least one source"} @@ -787,7 +863,7 @@ def create_job( "lookback": lookback, "offset": offset, }, - relations={"target": targets}, + relations={"targets": targets}, ) except (ConfigError, CatalogKeyError) as e: return {"status": "error", "error": str(e)} diff --git a/packages/interloper-agent/tests/test_agent.py b/packages/interloper-agent/tests/test_agent.py index f866aef5..aa1eefce 100644 --- a/packages/interloper-agent/tests/test_agent.py +++ b/packages/interloper-agent/tests/test_agent.py @@ -3,11 +3,26 @@ import datetime from typing import cast +from google.adk.agents.base_agent import BaseAgent from google.adk.agents.readonly_context import ReadonlyContext from interloper_agent import agent as agent_module +def _tool_names(agent: BaseAgent) -> set[str]: + """The names an agent's registered tools carry. + + Args: + agent: The agent whose tool list is read. + + Returns: + One name per registered tool: a plain function's ``__name__``, or a + tool object's own ``name`` (an ``AgentTool``, say). + """ + tools = getattr(agent, "tools", []) + return {getattr(tool, "__name__", None) or getattr(tool, "name", "") for tool in tools} + + def test_with_current_time_appends_now(): provider = agent_module.with_current_time("BASE") text = provider(cast(ReadonlyContext, None)) @@ -32,3 +47,9 @@ def test_all_agents_carry_the_current_time(): text = instruction(cast(ReadonlyContext, None)) assert isinstance(text, str) assert "Current date and time:" in text + + +def test_the_collection_agent_registers_the_relation_write_tools(): + # The toolkit's only write functions: exposed here, never on the + # deliberately read-only MCP server. + assert {"bind_relation", "unbind_relation"} <= _tool_names(agent_module.collection_agent) diff --git a/packages/interloper-agent/tests/tools/test_collection.py b/packages/interloper-agent/tests/tools/test_collection.py index 77474101..4a90c2ab 100644 --- a/packages/interloper-agent/tests/tools/test_collection.py +++ b/packages/interloper-agent/tests/tools/test_collection.py @@ -153,3 +153,94 @@ def test_update_component_requires_a_change(store: FakeStore, ctx: ToolContext): result = collection.update_component(str(store.component.id), tool_context=ctx) assert result["status"] == "error" assert store.update_kwargs is None + + +# -- _source_relations -------------------------------------------------------- + + +class _RelationsComponentStore: + """Serves fixed connection/destination rows by id for _source_relations tests.""" + + def __init__(self, rows: dict[Any, Any]): + """Bind the fake facet to the rows it serves. + + Args: + rows: Maps component UUID to the row ``get`` returns for it. + """ + self._rows = rows + + def get(self, component_id: Any, *, kind: str | None = None) -> Any: + return self._rows[component_id] + + +class _RelationsStore: + """Presents the ``components`` facet ``_source_relations`` reaches for.""" + + def __init__(self, rows: dict[Any, Any]): + """Bind the fake store to the rows its components facet serves. + + Args: + rows: Maps component UUID to the row ``components.get`` returns. + """ + self.components = _RelationsComponentStore(rows) + + +def _relation_row(**overrides: Any) -> Any: + defaults: dict[str, Any] = {"id": uuid4(), "org_id": ORG_ID, "key": "facebook_ads_connection"} + return SimpleNamespace(**{**defaults, **overrides}) + + +def test_source_relations_binds_connection_by_name(): + connection = _relation_row(key="facebook_ads_connection") + destination = _relation_row(key="bigquery") + store = _RelationsStore({connection.id: connection, destination.id: destination}) + defn = { + "relations": { + "connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": False}, + "destinations": {"kind": "destination", "many": True, "optional": True}, + } + } + relations, error = collection._source_relations( + store, ORG_ID, defn, "facebook_ads", str(connection.id), [str(destination.id)] + ) + assert error is None + assert relations == {"connection": [connection.id], "destinations": [destination.id]} + + +def test_source_relations_rejects_connection_with_mismatched_key(): + connection = _relation_row(key="bing_ads_connection") + store = _RelationsStore({connection.id: connection}) + defn = {"relations": {"connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": False}}} + relations, error = collection._source_relations(store, ORG_ID, defn, "facebook_ads", str(connection.id), None) + assert relations is None + assert error is not None + assert "does not fit any relation of 'facebook_ads'" in error["error"] + + +def test_source_relations_requires_missing_connection(): + store = _RelationsStore({}) + defn = {"relations": {"connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": False}}} + relations, error = collection._source_relations(store, ORG_ID, defn, "facebook_ads", None, None) + assert relations is None + assert error is not None + assert "requires a 'facebook_ads_connection' as 'connection'" in error["error"] + + +def test_source_relations_rejects_a_connection_a_source_cannot_hold(): + connection = _relation_row() + store = _RelationsStore({connection.id: connection}) + defn = {"relations": {"destinations": {"kind": "destination", "many": True, "optional": True}}} + relations, error = collection._source_relations( + store, ORG_ID, defn, "static_source", str(connection.id), None + ) + assert relations is None + assert error is not None + assert "does not fit any relation of 'static_source'" in error["error"] + + +def test_source_relations_allows_optional_connection_unbound(): + store = _RelationsStore({}) + defn = {"relations": {"connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": True}}} + relations, error = collection._source_relations(store, ORG_ID, defn, "facebook_ads", None, None) + assert error is None + assert relations == {"destinations": []} diff --git a/packages/interloper-api/src/interloper_api/routes/catalog.py b/packages/interloper-api/src/interloper_api/routes/catalog.py index dc95e5f0..97e9f335 100644 --- a/packages/interloper-api/src/interloper_api/routes/catalog.py +++ b/packages/interloper-api/src/interloper_api/routes/catalog.py @@ -30,8 +30,8 @@ def list_resource_kinds(catalog: Catalog = Depends(get_catalog)) -> list[str]: """Return distinct resource kinds from the catalog. A resource kind is any registered kind anchored under ``Resource`` - (currently ``connection`` and ``config``) — the kinds usable as - slot bindings on other components. + (currently ``connection`` and ``config``) - the kinds usable as + relation bindings on other components. Args: catalog: Injected catalog. diff --git a/packages/interloper-api/src/interloper_api/routes/components.py b/packages/interloper-api/src/interloper_api/routes/components.py index fdb0b4ff..98a73c5d 100644 --- a/packages/interloper-api/src/interloper_api/routes/components.py +++ b/packages/interloper-api/src/interloper_api/routes/components.py @@ -62,30 +62,32 @@ class RelationEntry(BaseModel): """One relation binding in a create/update request.""" dst_id: UUID - slot: str = "" class RelationCreateRequest(RelationEntry): """Request body for adding one relation.""" - type: str + name: str class RelationRef(BaseModel): """One relation binding in a component response.""" dst_id: UUID - slot: str = "" dst_kind: str class RelationResponse(BaseModel): - """An org-wide relation row (graph edges, dependency lists).""" + """An org-wide relation row (graph edges, dependency lists). + + ``src_kind`` rides along so the app's graph and upstream views can filter + asset-to-asset rows without a second lookup. + """ src_id: UUID + name: str dst_id: UUID - type: str - slot: str + src_kind: str dst_kind: str @@ -95,8 +97,8 @@ class ComponentCreateRequest(BaseModel): ``encrypted`` applies to secret kinds only: None encrypts whenever an encryption key is configured, an explicit bool forces it on or off. ``children`` applies to source kinds only and names the child asset keys - to enable (None enables all of them). Every relation type listed in - ``relations`` is replaced wholesale, so an empty list clears that type. + to enable (None enables all of them). Every relation name listed in + ``relations`` is replaced wholesale, so an empty list clears that name. """ kind: str @@ -228,35 +230,35 @@ class PartitionRowCountsResponse(BaseModel): def _relations_of(row: Component) -> dict[str, list[RelationRef]]: - """Group a component's outgoing relations by relation type. + """Group a component's outgoing relations by name. Args: row: The component row, with its ``out_relations`` eager-loaded. Returns: - A ``{type: [bindings]}`` map of the row's outgoing relations. + A ``{name: [bindings]}`` map of the row's outgoing relations. """ grouped: dict[str, list[RelationRef]] = {} for relation in row.out_relations: - grouped.setdefault(relation.type, []).append( - RelationRef(dst_id=relation.dst_id, slot=relation.slot, dst_kind=relation.dst_kind) + grouped.setdefault(relation.name, []).append( + RelationRef(dst_id=relation.dst_id, dst_kind=relation.dst_kind) ) return grouped -def _bindings(relations: dict[str, list[RelationEntry]] | None) -> dict[str, list[tuple[UUID, str]]] | None: - """Flatten a request's relation entries into the tuples the store takes. +def _bindings(relations: dict[str, list[RelationEntry]] | None) -> dict[str, list[UUID]] | None: + """Flatten a request's relation entries into the ids the store takes. Args: - relations: The request's ``{type: [entries]}`` map, or None to leave - every relation type untouched. + relations: The request's ``{name: [entries]}`` map, or None to leave + every relation name untouched. Returns: - A ``{type: [(dst_id, slot)]}`` map, or None when *relations* is None. + A ``{name: [dst_id, ...]}`` map, or None when *relations* is None. """ if relations is None: return None - return {type_: [(entry.dst_id, entry.slot) for entry in entries] for type_, entries in relations.items()} + return {name: [entry.dst_id for entry in entries] for name, entries in relations.items()} # -- Component endpoints ------------------------------------------------------- @@ -286,15 +288,19 @@ def list_components( @router.get("/relations") def list_relations( - type: str | None = None, + name: str | None = None, + src_kind: str | None = None, + dst_kind: str | None = None, user: Profile = Depends(require_viewer), org_id: UUID = Depends(get_org_id), store: Store = Depends(get_store), ) -> list[RelationResponse]: - """List the organisation's component relations, optionally by type. + """List the organisation's component relations, optionally filtered. Args: - type: The relation type to keep; None lists every type. + name: The relation name to keep; None lists every name. + src_kind: The source kind to keep; None lists every kind. + dst_kind: The destination kind to keep; None lists every kind. user: The authenticated user. org_id: The active organisation UUID. store: The Store instance. @@ -305,12 +311,12 @@ def list_relations( return [ RelationResponse( src_id=relation.src_id, + name=relation.name, dst_id=relation.dst_id, - type=relation.type, - slot=relation.slot, + src_kind=relation.src_kind, dst_kind=relation.dst_kind, ) - for relation in store.relations.list_all(org_id, type=type) + for relation in store.relations.list_all(org_id, name=name, src_kind=src_kind, dst_kind=dst_kind) ] @@ -464,7 +470,7 @@ def add_relation( Args: component_id: The source component's UUID. - body: The relation to add: its type, target ``dst_id`` and slot. + body: The relation to add: its name and target ``dst_id``. user: The authenticated user. store: The Store instance. @@ -482,45 +488,45 @@ def add_relation( if destination_row.org_id != source.org_id: raise HTTPException(status_code=404, detail=f"Component {body.dst_id} not found") try: - relation = store.relations.add(component_id, type=body.type, dst_id=body.dst_id, slot=body.slot) + relation = store.relations.add(component_id, name=body.name, dst_id=body.dst_id) except ConfigError as e: raise HTTPException(status_code=400, detail=str(e)) except NotFoundError as e: raise HTTPException(status_code=404, detail=str(e)) return RelationResponse( src_id=relation.src_id, + name=relation.name, dst_id=relation.dst_id, - type=relation.type, - slot=relation.slot, + src_kind=relation.src_kind, dst_kind=relation.dst_kind, ) -@router.delete("/{component_id}/relations/{type}/{dst_id}", status_code=204) +@router.delete("/{component_id}/relations/{name}/{dst_id}", status_code=204) def remove_relation( component_id: UUID, - type: str, + name: str, dst_id: UUID, user: Profile = Depends(get_current_user), store: Store = Depends(get_store), ) -> None: - """Remove a component's relations of one type toward one destination. + """Remove a component's relation of one name toward one destination. - Refused (400) for required dependency slots — repoint them instead. + Refused (400) for required dependency names - repoint them instead. Args: component_id: The source component's UUID. - type: The relation type to remove. + name: The relation name to remove. dst_id: The target component's UUID. user: The authenticated user. store: The Store instance. Raises: - HTTPException: 400 for a required dependency slot. + HTTPException: 400 for a required dependency name. """ load_authorized(store.components.get, component_id, user, store, label="Component", minimum="editor") try: - store.relations.remove(component_id, type=type, dst_id=dst_id) + store.relations.remove(component_id, name=name, dst_id=dst_id) except ConfigError as e: raise HTTPException(status_code=400, detail=str(e)) @@ -608,8 +614,8 @@ def handle_error(error: Exception, context: str) -> None: class ResolveRequest(BaseModel): """A request to resolve one provider-backed FetchField's options. - ``deps`` carries the credentials the form already holds, keyed by resource - slot (e.g. ``{"connection": {"access_token": ...}}``). + ``deps`` carries the credentials the form already holds, keyed by relation + name (e.g. ``{"connection": {"access_token": ...}}``). """ component_key: str @@ -626,18 +632,18 @@ async def resolve_fetch_field( """Resolve the options for a ``FetchField(provider=...)`` field. One endpoint resolves any field declared with - ``FetchField(provider=".")`` — there are no hand-written + ``FetchField(provider=".")`` - there are no hand-written per-provider routes. The component definition comes from the catalog - (authoritative — the provider reference comes from the server's schema, - never the client), the resource in ```` is instantiated from the - credentials the form already holds, and the ``@fetch_field_provider`` - method ```` is called on it. That marker is the allowlist: only - methods opted in that way may be invoked, so the browser cannot call - arbitrary attributes. + (authoritative - the provider reference comes from the server's schema, + never the client), the resource named by the relation is instantiated + from the credentials the form already holds, and the + ``@fetch_field_provider`` method ```` is called on it. That + marker is the allowlist: only methods opted in that way may be invoked, + so the browser cannot call arbitrary attributes. Args: - body: The component key, the field name, and the per-slot credentials - the form currently holds. + body: The component key, the field name, and the per-relation + credentials the form currently holds. catalog: The Catalog instance. _user: The authenticated user (viewer gate). @@ -646,8 +652,9 @@ async def resolve_fetch_field( Raises: HTTPException: 404 for an unknown component key, 400 when the field is - not a provider-backed FetchField or names an unknown resource - slot, 403 when the target method is not a fetch provider. + not a provider-backed FetchField or names an unknown or + undeclared relation, 403 when the target method is not a fetch + provider. """ defn = catalog.get(body.component_key) if defn is None: @@ -660,16 +667,20 @@ async def resolve_fetch_field( status_code=400, detail=f"Field '{body.field}' on '{body.component_key}' is not a provider-backed FetchField", ) - slot, _, method = str(provider).partition(".") + name, _, method = str(provider).partition(".") component_cls = import_from_path(defn.path) - resource_cls = getattr(component_cls, "resource_types", {}).get(slot) + relation = component_cls.relations.get(name) + resource_cls = relation.target if relation else None if resource_cls is None: - raise HTTPException(status_code=400, detail=f"Resource slot '{slot}' not found on '{body.component_key}'") + raise HTTPException( + status_code=400, + detail=f"Relation '{name}' not found on '{body.component_key}' or not declared from a component class", + ) - # Only pass through fields the resource actually declares — the form may + # Only pass through fields the resource actually declares - the form may # carry extra markers (e.g. an internal id) that the model would reject. - raw = body.deps.get(slot, {}) + raw = body.deps.get(name, {}) creds = {k: v for k, v in raw.items() if k in resource_cls.model_fields} resource = resource_cls(**creds) diff --git a/packages/interloper-api/tests/routes/test_components.py b/packages/interloper-api/tests/routes/test_components.py index 06f0c26a..df072bbc 100644 --- a/packages/interloper-api/tests/routes/test_components.py +++ b/packages/interloper-api/tests/routes/test_components.py @@ -447,19 +447,31 @@ def _load(self, component_id: UUID) -> Any: raise self.load_error return self.loaded - def _list_relations(self, org_id: UUID, type: str | None = None) -> list[Any]: - return [r for r in self.relation_rows if type is None or r.type == type] + def _list_relations( + self, + org_id: UUID, + name: str | None = None, + src_kind: str | None = None, + dst_kind: str | None = None, + ) -> list[Any]: + return [ + r + for r in self.relation_rows + if (name is None or r.name == name) + and (src_kind is None or r.src_kind == src_kind) + and (dst_kind is None or r.dst_kind == dst_kind) + ] - def _add_relation(self, src_id: UUID, *, type: str, dst_id: UUID, slot: str) -> Any: + def _add_relation(self, src_id: UUID, *, name: str, dst_id: UUID) -> Any: if self.error: raise self.error - self.added_relations.append({"src_id": src_id, "type": type, "dst_id": dst_id, "slot": slot}) - return SimpleNamespace(src_id=src_id, dst_id=dst_id, type=type, slot=slot, dst_kind="connection") + self.added_relations.append({"src_id": src_id, "name": name, "dst_id": dst_id}) + return SimpleNamespace(src_id=src_id, name=name, dst_id=dst_id, src_kind="source", dst_kind="connection") - def _remove_relation(self, src_id: UUID, *, type: str, dst_id: UUID) -> None: + def _remove_relation(self, src_id: UUID, *, name: str, dst_id: UUID) -> None: if self.error: raise self.error - self.removed_relations.append({"src_id": src_id, "type": type, "dst_id": dst_id}) + self.removed_relations.append({"src_id": src_id, "name": name, "dst_id": dst_id}) @pytest.fixture @@ -514,13 +526,13 @@ def test_no_components_is_an_empty_list(self, crud_client: TestClient) -> None: class TestListRelations: - """``GET /components/relations`` — optionally narrowed by type.""" + """``GET /components/relations``, optionally narrowed by name and kinds.""" def test_lists_every_relation(self, crud_client: TestClient, crud_store: CrudStore) -> None: source_id, destination_id = uuid4(), uuid4() crud_store.relation_rows = [ SimpleNamespace( - src_id=source_id, dst_id=destination_id, type="resource", slot="connection", dst_kind="connection" + src_id=source_id, name="connection", dst_id=destination_id, src_kind="source", dst_kind="connection" ) ] @@ -529,22 +541,40 @@ def test_lists_every_relation(self, crud_client: TestClient, crud_store: CrudSto assert response.json() == [ { "src_id": str(source_id), + "name": "connection", "dst_id": str(destination_id), - "type": "resource", - "slot": "connection", + "src_kind": "source", "dst_kind": "connection", } ] - def test_the_type_filter_narrows_the_result(self, crud_client: TestClient, crud_store: CrudStore) -> None: + def test_the_name_filter_narrows_the_result(self, crud_client: TestClient, crud_store: CrudStore) -> None: crud_store.relation_rows = [ - SimpleNamespace(src_id=uuid4(), dst_id=uuid4(), type="resource", slot="a", dst_kind="connection"), - SimpleNamespace(src_id=uuid4(), dst_id=uuid4(), type="destination", slot="b", dst_kind="destination"), + SimpleNamespace( + src_id=uuid4(), name="connection", dst_id=uuid4(), src_kind="source", dst_kind="connection" + ), + SimpleNamespace( + src_id=uuid4(), name="destinations", dst_id=uuid4(), src_kind="source", dst_kind="destination" + ), + ] + + response = crud_client.get("/components/relations?name=destinations") + + assert [r["name"] for r in response.json()] == ["destinations"] + + def test_list_relations_filters_by_kind(self, crud_client: TestClient, crud_store: CrudStore) -> None: + crud_store.relation_rows = [ + SimpleNamespace(src_id=uuid4(), name="upstreams", dst_id=uuid4(), src_kind="asset", dst_kind="asset"), + SimpleNamespace( + src_id=uuid4(), name="connection", dst_id=uuid4(), src_kind="source", dst_kind="connection" + ), ] - response = crud_client.get("/components/relations?type=destination") + response = crud_client.get("/components/relations", params={"src_kind": "asset", "dst_kind": "asset"}) - assert [r["type"] for r in response.json()] == ["destination"] + rows = response.json() + assert all(r["dst_kind"] == "asset" for r in rows) + assert [r["name"] for r in rows] == ["upstreams"] class TestCreateComponent: @@ -569,12 +599,12 @@ def test_relations_are_flattened_into_store_bindings( body = { "kind": "source", "key": "fb", - "relations": {"resource": [{"dst_id": str(destination_id), "slot": "connection"}]}, + "relations": {"connection": [{"dst_id": str(destination_id)}]}, } crud_client.post("/components/", json=body) - assert crud_store.created[0]["relations"] == {"resource": [(destination_id, "connection")]} + assert crud_store.created[0]["relations"] == {"connection": [destination_id]} def test_omitted_relations_stay_none(self, crud_client: TestClient, crud_store: CrudStore) -> None: # None means "leave every relation type untouched", which is not the @@ -710,14 +740,27 @@ class TestAddRelation: def test_adds_the_relation(self, crud_client: TestClient, crud_store: CrudStore) -> None: source_id, destination_id = uuid4(), uuid4() - body = {"type": "resource", "dst_id": str(destination_id), "slot": "connection"} + body = {"name": "connection", "dst_id": str(destination_id)} response = crud_client.post(f"/components/{source_id}/relations", json=body) assert response.status_code == 201 assert response.json()["dst_kind"] == "connection" assert crud_store.added_relations == [ - {"src_id": source_id, "type": "resource", "dst_id": destination_id, "slot": "connection"} + {"src_id": source_id, "name": "connection", "dst_id": destination_id} + ] + + def test_add_relation_by_name(self, crud_client: TestClient, crud_store: CrudStore) -> None: + source_id, destination_id = uuid4(), uuid4() + + response = crud_client.post( + f"/components/{source_id}/relations", json={"name": "destinations", "dst_id": str(destination_id)} + ) + + assert response.status_code == 201 + assert response.json()["name"] == "destinations" + assert crud_store.added_relations == [ + {"src_id": source_id, "name": "destinations", "dst_id": destination_id} ] def test_a_target_in_another_org_is_a_404( @@ -735,7 +778,7 @@ def get(component_id: UUID) -> Any: response = crud_client.post( f"/components/{source_id}/relations", - json={"type": "resource", "dst_id": str(destination_id), "slot": "connection"}, + json={"name": "connection", "dst_id": str(destination_id)}, ) assert response.status_code == 404 @@ -752,30 +795,40 @@ def test_store_errors_map_to_statuses( response = crud_client.post( f"/components/{uuid4()}/relations", - json={"type": "resource", "dst_id": str(uuid4()), "slot": "connection"}, + json={"name": "connection", "dst_id": str(uuid4())}, ) assert response.status_code == expected class TestRemoveRelation: - """``DELETE /components/{id}/relations/{type}/{dst_id}``.""" + """``DELETE /components/{id}/relations/{name}/{dst_id}``.""" def test_removes_the_relation(self, crud_client: TestClient, crud_store: CrudStore) -> None: source_id, destination_id = uuid4(), uuid4() - response = crud_client.delete(f"/components/{source_id}/relations/resource/{destination_id}") + response = crud_client.delete(f"/components/{source_id}/relations/connection/{destination_id}") assert response.status_code == 204 assert crud_store.removed_relations == [ - {"src_id": source_id, "type": "resource", "dst_id": destination_id} + {"src_id": source_id, "name": "connection", "dst_id": destination_id} ] - def test_a_required_slot_is_refused(self, crud_client: TestClient, crud_store: CrudStore) -> None: - # Required dependency slots are repointed, never emptied. - crud_store.error = ConfigError("slot 'connection' is required") + def test_remove_relation_route_uses_name(self, crud_client: TestClient, crud_store: CrudStore) -> None: + source_id, destination_id = uuid4(), uuid4() + + response = crud_client.delete(f"/components/{source_id}/relations/destinations/{destination_id}") - response = crud_client.delete(f"/components/{uuid4()}/relations/resource/{uuid4()}") + assert response.status_code == 204 + assert crud_store.removed_relations == [ + {"src_id": source_id, "name": "destinations", "dst_id": destination_id} + ] + + def test_a_required_name_is_refused(self, crud_client: TestClient, crud_store: CrudStore) -> None: + # Required dependency names are repointed, never emptied. + crud_store.error = ConfigError("'connection' is required") + + response = crud_client.delete(f"/components/{uuid4()}/relations/connection/{uuid4()}") assert response.status_code == 400 @@ -884,35 +937,58 @@ def broken() -> dict[str, int]: class TestRelationGrouping: """``_relations_of`` and ``_bindings`` — the two shape converters.""" - def test_outgoing_relations_group_by_type(self) -> None: + def test_outgoing_relations_group_by_name(self) -> None: first, second = uuid4(), uuid4() row = _row( relations=[ - SimpleNamespace(type="resource", dst_id=first, slot="connection", dst_kind="connection"), - SimpleNamespace(type="resource", dst_id=second, slot="other", dst_kind="connection"), - SimpleNamespace(type="destination", dst_id=first, slot="", dst_kind="destination"), + SimpleNamespace(name="connection", dst_id=first, dst_kind="connection"), + SimpleNamespace(name="connection", dst_id=second, dst_kind="connection"), + SimpleNamespace(name="destinations", dst_id=first, dst_kind="destination"), ] ) grouped = components_module._relations_of(row) - assert set(grouped) == {"resource", "destination"} - assert [ref.dst_id for ref in grouped["resource"]] == [first, second] + assert set(grouped) == {"connection", "destinations"} + assert [ref.dst_id for ref in grouped["connection"]] == [first, second] def test_no_relations_is_an_empty_map(self) -> None: assert components_module._relations_of(_row()) == {} - def test_bindings_flatten_entries_to_tuples(self) -> None: + def test_bindings_flatten_entries_to_ids(self) -> None: destination_id = uuid4() - entries = {"resource": [components_module.RelationEntry(dst_id=destination_id, slot="connection")]} + entries = {"connection": [components_module.RelationEntry(dst_id=destination_id)]} - assert components_module._bindings(entries) == {"resource": [(destination_id, "connection")]} + assert components_module._bindings(entries) == {"connection": [destination_id]} def test_bindings_of_none_stay_none(self) -> None: - # None means "leave every relation type untouched". + # None means "leave every relation name untouched". assert components_module._bindings(None) is None +class TestComponentResponseRelations: + """``ComponentResponse.relations``, keyed by name, dst_kind carried along.""" + + def test_component_response_relations_keyed_by_name(self) -> None: + connection_id, destination_id = uuid4(), uuid4() + row = _row( + relations=[ + SimpleNamespace(name="connection", dst_id=connection_id, dst_kind="connection"), + SimpleNamespace(name="destinations", dst_id=destination_id, dst_kind="destination"), + ] + ) + store = cast(Store, SimpleNamespace( + components=SimpleNamespace(status=lambda row, parent_key=None: ComponentStatus.OK) + )) + + response = components_module.ComponentResponse.from_row(row, store, include_config=False) + + assert set(response.relations) == {"connection", "destinations"} + assert response.relations["connection"] == [ + components_module.RelationRef(dst_id=connection_id, dst_kind="connection") + ] + + class TestHandleError: """``handle_error`` maps a provider failure to a status, never a traceback.""" @@ -1022,13 +1098,13 @@ def test_anything_else_is_a_generic_error(self) -> None: class TestResolveEdgeCases: """``POST /components/resolve`` — the guards between the field and the provider.""" - def test_an_unknown_resource_slot_is_a_400( + def test_an_unknown_relation_name_is_a_400( self, source_catalog: il.Catalog, monkeypatch: pytest.MonkeyPatch ) -> None: - # The FetchField names a slot the component does not declare. + # The FetchField names a relation the component does not declare. from interloper_assets.facebook_ads.source import FacebookAds - monkeypatch.setattr(FacebookAds, "resource_types", {}) + monkeypatch.setattr(FacebookAds, "relations", {}) response = _client(source_catalog).post( "/components/resolve", @@ -1036,7 +1112,8 @@ def test_an_unknown_resource_slot_is_a_400( ) assert response.status_code == 400 - assert "Resource slot" in response.json()["detail"] + assert "Relation 'connection' not found" in response.json()["detail"] + assert "not declared from a component class" in response.json()["detail"] def test_a_provider_failure_is_mapped_not_raised( self, source_catalog: il.Catalog, mock_graph @@ -1054,7 +1131,7 @@ def handler(request: httpx.Request) -> httpx.Response: assert response.status_code == 500 assert response.json()["detail"].startswith("Failed resolving facebook_ads.account_id") - def test_a_slot_that_is_not_a_fetch_provider_is_a_403( + def test_a_relation_that_is_not_a_fetch_provider_is_a_403( self, source_catalog: il.Catalog, monkeypatch: pytest.MonkeyPatch ) -> None: # Validated at catalog build, so this is a defensive guard. diff --git a/packages/interloper-core/src/interloper/asset/base.py b/packages/interloper-core/src/interloper/asset/base.py index 202ee9f5..c9969c08 100644 --- a/packages/interloper-core/src/interloper/asset/base.py +++ b/packages/interloper-core/src/interloper/asset/base.py @@ -208,6 +208,14 @@ def _data_hints(cls) -> dict[str, Any]: def _infer_relation(cls, name: str, hint: Any, *, optional: bool) -> Relation: """The relation one ``data()`` parameter declares. + An optional upstream is inferred ``on_delete="detach"``: the parameter + already tolerates the leg being absent, so nothing is owed to it and + deleting the upstream must not be refused on its behalf. A parameter + naming a component class keeps the default ``block`` whether it is + optional or not: this ruling covers asset upstreams only, and a + connection or config a read consumes stays undeletable while it is + bound. + Args: name: The parameter name, which is the relation's name and, for an upstream, the bare asset key it expects. @@ -227,10 +235,11 @@ def _infer_relation(cls, name: str, hint: Any, *, optional: bool) -> Relation: """ target, admits_none = unwrap_optional(hint, {}) optional = optional or admits_none + on_delete = "detach" if optional else "block" if target is Upstream: - return Relation("asset", name, optional=optional) + return Relation("asset", name, optional=optional, on_delete=on_delete) if get_origin(target) is list and get_args(target) == (Upstream,): - return Relation("asset", name, many=True, optional=optional) + return Relation("asset", name, many=True, optional=optional, on_delete=on_delete) if isinstance(target, type) and issubclass(target, Component) and target.kind: return Relation(target, optional=optional) if isinstance(hint, str): @@ -337,7 +346,8 @@ def __call__( Two exceptions: ``normalizer``, whose sentinel default lets an explicit ``None`` clear the configured normalizer, and a name in **relations, where ``None`` clears the binding, so only leaving the name out - entirely leaves it as is. + entirely leaves it as is. Clearing a non-optional relation is refused + with a ``ConfigError``, since it cannot be left empty. The copy carries this asset's own bindings and parent, so a copy made to flip one field (the non-materializable parents of a mini-DAG, say) diff --git a/packages/interloper-core/src/interloper/asset/upstream.py b/packages/interloper-core/src/interloper/asset/upstream.py index 357a25ba..6cf3ad72 100644 --- a/packages/interloper-core/src/interloper/asset/upstream.py +++ b/packages/interloper-core/src/interloper/asset/upstream.py @@ -1,4 +1,4 @@ -"""The leg object handed to ``data()`` for many-valued upstream slots.""" +"""The leg object handed to ``data()`` for many-valued upstream relations.""" from __future__ import annotations @@ -11,7 +11,7 @@ @dataclass(frozen=True) class Upstream: - """One leg of a many-valued slot as handed to ``data()``. + """One leg of a many-valued relation as handed to ``data()``. Attributes: asset: The upstream asset the data was read from; its ``source``, diff --git a/packages/interloper-core/src/interloper/catalog/base.py b/packages/interloper-core/src/interloper/catalog/base.py index 159468d8..ff0cb71c 100644 --- a/packages/interloper-core/src/interloper/catalog/base.py +++ b/packages/interloper-core/src/interloper/catalog/base.py @@ -62,7 +62,7 @@ def get(self, key: str, default: Any = None, *, parent_key: str | None = None) - :attr:`SourceDefinition.assets`. Pass *parent_key* to resolve such an asset the way its owner declares it: the concrete :class:`AssetDefinition`, carrying the composite import path, the - partitioning and the dependency slots the flat key cannot name. A + partitioning and the relations the flat key cannot name. A parent that does not resolve, or does not declare the key, falls back to the flat lookup. @@ -96,11 +96,11 @@ def vocabulary(self, kind: str, key: str, *, parent_key: str | None = None) -> d kind: The row's component kind. key: The row's catalog key. parent_key: Key of the owning source, for a source-owned asset, - whose declaration carries the dependency slots. Defaults to + whose declaration carries the asset's relations. Defaults to ``None``, a flat lookup. Returns: - Relation type → definition. + The declared relations keyed by name. """ definition = self.get(key, parent_key=parent_key) if definition is not None and definition.kind == kind: diff --git a/packages/interloper-core/src/interloper/component/base.py b/packages/interloper-core/src/interloper/component/base.py index dda230e3..67ad0bac 100644 --- a/packages/interloper-core/src/interloper/component/base.py +++ b/packages/interloper-core/src/interloper/component/base.py @@ -346,6 +346,11 @@ def bind(self, name: str, *targets: Component) -> None: single-valued one replaces what it holds, so repointing it is a second :meth:`bind` and needs no :meth:`unbind` first. + A target whose kind or key the relation does not accept, this + component itself, and more than one target at once on a single-valued + relation are all refused with a ``ConfigError`` from + :meth:`_replace_binding`, which every write path goes through. + Args: name: The relation name as declared on the class. *targets: The components to bind. Binding nothing is a no-op, so a @@ -683,11 +688,14 @@ def _discriminator_fields(cls) -> list[str]: def _check_targets(self, name: str, relation: Relation, targets: tuple[Component, ...]) -> None: """Check that *targets* are legal for one of this component's relations. - Every target must be one the relation :meth:`~Relation.accepts`, and a - single-valued relation may not receive more than one target at once. - Performs no mutation, so :meth:`_replace_binding` can call it before - touching ``_bound`` and a rejected replacement leaves the existing - binding untouched. + Every target must be one the relation :meth:`~Relation.accepts` and + must be another component: a relation whose declared keys match the + declaring component's own key (a wildcard, or an asset named after + the key it reads) would otherwise let it fill itself. A single-valued + relation may not receive more than one target at once. Performs no + mutation, so :meth:`_replace_binding` can call it before touching + ``_bound`` and a rejected replacement leaves the existing binding + untouched. Args: name: The relation name as declared on the class. @@ -695,11 +703,14 @@ def _check_targets(self, name: str, relation: Relation, targets: tuple[Component targets: The candidate components to check. Raises: - ConfigError: If a target's kind or key is not one the relation - accepts, or if a single-valued relation is given more than one target. + ConfigError: If a target is this component itself, if a target's + kind or key is not one the relation accepts, or if a + single-valued relation is given more than one target. """ owner = self.identity for target in targets: + if target is self: + raise ConfigError(f"{type(self).__name__}.{name} cannot point at the component itself") if not relation.accepts(target.kind, target.identity, owner=owner): raise ConfigError( f"{type(self).__name__}.{name} does not accept {target.kind} '{target.qualified_key}' " diff --git a/packages/interloper-core/src/interloper/component/relation.py b/packages/interloper-core/src/interloper/component/relation.py index 61aa34c9..14cde5cc 100644 --- a/packages/interloper-core/src/interloper/component/relation.py +++ b/packages/interloper-core/src/interloper/component/relation.py @@ -137,9 +137,12 @@ class Relation(BaseModel): several component kinds, ``key`` (when non-empty) narrows to specific keys within those kinds, matched through :meth:`ComponentIdentity.satisfies`. ``many`` marks a relation that binds several components at once; - ``optional`` marks one that may stay unbound. ``default`` and - ``has_fallback`` together describe a relation that can be resolved - without an explicit binding. + ``optional`` marks one that may stay unbound or be left empty, and says + nothing else: ``on_delete`` alone decides what deleting a bound target + does, ``block`` refusing the deletion while this relation holds the + target and ``detach`` letting it through and dropping the binding. + ``default`` and ``has_fallback`` together describe a relation that can be + resolved without an explicit binding. A relation is also its own descriptor: :meth:`Component._collect` installs the stamped copy under the relation's name, so the class attribute reads diff --git a/packages/interloper-core/src/interloper/resource/__init__.py b/packages/interloper-core/src/interloper/resource/__init__.py index ef08c8f5..8bb75615 100644 --- a/packages/interloper-core/src/interloper/resource/__init__.py +++ b/packages/interloper-core/src/interloper/resource/__init__.py @@ -1,4 +1,4 @@ -"""Resources: injectable dependencies, their typed slots, and their field helpers.""" +"""Resources: injectable dependencies, their typed relations, and their field helpers.""" from interloper.resource.base import Resource, ResourceDefinition from interloper.resource.fields import ( diff --git a/packages/interloper-core/src/interloper/source/base.py b/packages/interloper-core/src/interloper/source/base.py index 3b9aecd3..45d1b30a 100644 --- a/packages/interloper-core/src/interloper/source/base.py +++ b/packages/interloper-core/src/interloper/source/base.py @@ -396,6 +396,8 @@ def __call__( name in **relations follows a different rule: passing it at all changes it, since ``None`` there clears the binding rather than leaving it alone; only leaving the name out entirely leaves it as is. + Clearing a non-optional relation is refused with a ``ConfigError``, + since it cannot be left empty. Args: dataset: Replacement dataset. Assets that inherited the source's diff --git a/packages/interloper-core/tests/asset/test_base.py b/packages/interloper-core/tests/asset/test_base.py index 971de281..6141bf1c 100644 --- a/packages/interloper-core/tests/asset/test_base.py +++ b/packages/interloper-core/tests/asset/test_base.py @@ -317,6 +317,39 @@ def data(self, context: il.ExecutionContext, config: Cfg | None = None) -> Any: assert A.relations["config"].optional is True + def test_an_optional_resource_still_blocks_its_deletion(self): + class A(il.Asset): + def data(self, context: il.ExecutionContext, config: Cfg | None = None) -> Any: # pragma: no cover + return [] + + assert A.relations["config"].on_delete == "block" + + def test_an_optional_upstream_detaches_on_delete(self): + class A(il.Asset): + def data(self, context: il.ExecutionContext, x: il.Upstream | None = None) -> Any: # pragma: no cover + return [] + + relation = A.relations["x"] + assert (relation.optional, relation.on_delete) == (True, "detach") + + def test_a_required_upstream_blocks_on_delete(self): + class A(il.Asset): + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> Any: # pragma: no cover + return [] + + relation = A.relations["orders"] + assert (relation.optional, relation.on_delete) == (False, "block") + + def test_an_optional_list_upstream_detaches_on_delete(self): + class A(il.Asset): + def data( + self, context: il.ExecutionContext, legs: list[il.Upstream] | None = None + ) -> Any: # pragma: no cover + return [] + + relation = A.relations["legs"] + assert (relation.many, relation.optional, relation.on_delete) == (True, True, "detach") + def test_optional_annotation_makes_optional(self): # Written as a string on purpose: a lazily-evaluated annotation must # resolve the same way as a real class. diff --git a/packages/interloper-core/tests/component/test_base.py b/packages/interloper-core/tests/component/test_base.py index 292c0c26..e2b22b1e 100644 --- a/packages/interloper-core/tests/component/test_base.py +++ b/packages/interloper-core/tests/component/test_base.py @@ -274,6 +274,12 @@ class Gadget(il.Source): store: NeedyDest = il.Relation(NeedyDest) +class Peer(il.Source): + """Source whose ``peers`` relation declares no key, so it would accept itself.""" + + peers: list[il.Source] = il.Relation("source", many=True, optional=True) + + class TestCollect: def test_annotation_becomes_relation(self): assert Widget.relations["connection"].kind == "connection" @@ -353,6 +359,15 @@ def test_wrong_kind_rejected(self): with pytest.raises(ConfigError, match="connection"): Widget(connection=Cfg()) # ty: ignore[invalid-argument-type] + def test_a_relation_refuses_the_component_itself(self): + peer = Peer() + other = Peer() + + with pytest.raises(ConfigError, match="cannot point at the component itself"): + peer.bind("peers", peer) + peer.bind("peers", other) + assert peer.peers == [other] + def test_single_relation_rejects_two_targets_at_once(self): widget = Widget(connection=Conn(api_secret="s")) with pytest.raises(ConfigError, match="single"): diff --git a/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py new file mode 100644 index 00000000..138355c8 --- /dev/null +++ b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py @@ -0,0 +1,94 @@ +"""Key component relations by name. + +A relation row used to carry a ``type`` (the vocabulary entry) and a ``slot`` +(empty for list-shaped types). The framework now declares one named +``Relation`` per link, so a row is ``(src_id, name, dst_id)``: ``name`` is the +slot for slotted rows and the plural field name for the others. Whether a +relation is single-valued is a class rule the store enforces, so the partial +unique index on resource slots goes. + +The types the old vocabulary ever wrote (``resource``, ``dependency``/ +``upstream``, and the three ``_PLURAL`` maps: ``destination``, ``target``, +``watch``) are backfilled into ``name`` below. The final ``DELETE`` is +defensive only: it drops a row left with a NULL ``name`` (a ``type`` outside +that vocabulary) or an empty one (a ``resource`` row whose ``slot`` was +never filled, which the store had no path to write). Production holds +neither, so it is a no-op there. + +The downgrade maps every asset-kind row back to ``type = 'upstream'``, so a +pre-017 ``dependency`` type is never recovered: that rename never shipped, so +no row in the wild carries it. In upgrade(), ``_PLURAL`` entries match by +``type``; in downgrade(), they match by ``name``. + +``upgrade`` is a no-op on a database whose ``component_relations`` table has +no ``type`` column: ``create_all()`` always provisions the table from the +current model, so a fresh database already has the final (``name``) shape +before Alembic runs at all. + +Revision ID: 017 +Revises: 016 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "017" +down_revision: str | None = "016" +branch_labels: str | None = None +depends_on: str | None = None + +_TABLE = "component_relations" +_PLURAL = {"destination": "destinations", "target": "targets", "watch": "watches"} + + +def upgrade() -> None: + bind = op.get_bind() + columns = {column["name"] for column in sa.inspect(bind).get_columns(_TABLE)} + if "type" not in columns: + # Fresh database: create_all() already produced the final (name) shape. + return + + op.add_column(_TABLE, sa.Column("name", sa.String(), nullable=True)) + op.execute("UPDATE component_relations SET name = slot WHERE type IN ('resource', 'dependency', 'upstream')") + for type_, name in _PLURAL.items(): + op.execute(f"UPDATE component_relations SET name = '{name}' WHERE type = '{type_}'") + op.execute("DELETE FROM component_relations WHERE name IS NULL OR name = ''") + op.alter_column(_TABLE, "name", nullable=False) + op.drop_index("uq_component_relations_slot", table_name=_TABLE) + op.drop_index("ix_component_relations_org_id_type", table_name=_TABLE) + op.drop_index("ix_component_relations_dst_id_type", table_name=_TABLE) + op.drop_constraint("component_relations_pkey", _TABLE, type_="primary") + op.drop_column(_TABLE, "type") + op.drop_column(_TABLE, "slot") + op.create_primary_key("component_relations_pkey", _TABLE, ["src_id", "name", "dst_id"]) + op.create_index("ix_component_relations_org_id_name", _TABLE, ["org_id", "name"]) + op.create_index("ix_component_relations_dst_id_name", _TABLE, ["dst_id", "name"]) + + +def downgrade() -> None: + op.add_column(_TABLE, sa.Column("type", sa.String(), nullable=True)) + op.add_column(_TABLE, sa.Column("slot", sa.String(), nullable=True, server_default="")) + for type_, name in _PLURAL.items(): + op.execute(f"UPDATE component_relations SET type = '{type_}' WHERE name = '{name}'") + op.execute( + "UPDATE component_relations SET type = 'upstream', slot = name WHERE type IS NULL AND dst_kind = 'asset'" + ) + op.execute("UPDATE component_relations SET type = 'resource', slot = name WHERE type IS NULL") + op.alter_column(_TABLE, "type", nullable=False) + op.drop_index("ix_component_relations_org_id_name", table_name=_TABLE) + op.drop_index("ix_component_relations_dst_id_name", table_name=_TABLE) + op.drop_constraint("component_relations_pkey", _TABLE, type_="primary") + op.drop_column(_TABLE, "name") + op.create_primary_key("component_relations_pkey", _TABLE, ["src_id", "type", "slot", "dst_id"]) + op.create_index( + "uq_component_relations_slot", + _TABLE, + ["src_id", "type", "slot"], + unique=True, + postgresql_where=sa.text("type = 'resource'"), + ) + op.create_index("ix_component_relations_org_id_type", _TABLE, ["org_id", "type"]) + op.create_index("ix_component_relations_dst_id_type", _TABLE, ["dst_id", "type"]) diff --git a/packages/interloper-db/src/interloper_db/migrations/versions/017_upstream_relation.py b/packages/interloper-db/src/interloper_db/migrations/versions/017_upstream_relation.py deleted file mode 100644 index fd5221d9..00000000 --- a/packages/interloper-db/src/interloper_db/migrations/versions/017_upstream_relation.py +++ /dev/null @@ -1,46 +0,0 @@ -"""Rename the asset-to-asset relation to ``upstream`` and let its slots hold several edges. - -The relation type ``dependency`` named the role too loosely (every relation -is a dependency of sorts); ``upstream`` says which way the edge points and -matches the vocabulary the app, the lineage tools and the executor already -use. Persisted rows follow the rename. - -A many-valued upstream slot binds every matching upstream, so the per-slot -uniqueness that made re-binding repoint an edge can no longer be a schema -rule for that type. Single-valued upstream slots keep repointing in -``RelationStore``, which knows the slot contract; resources stay unique by -schema. - -Revision ID: 017 -Revises: 016 -""" - -from __future__ import annotations - -import sqlalchemy as sa -from alembic import op - -# revision identifiers, used by Alembic. -revision: str = "017" -down_revision: str | None = "016" -branch_labels: str | None = None -depends_on: str | None = None - -_INDEX = "uq_component_relations_slot" -_TABLE = "component_relations" -_COLUMNS = ["src_id", "type", "slot"] - - -def upgrade() -> None: - op.execute("UPDATE component_relations SET type = 'upstream' WHERE type = 'dependency'") - op.drop_index(_INDEX, table_name=_TABLE) - op.create_index(_INDEX, _TABLE, _COLUMNS, unique=True, postgresql_where=sa.text("type = 'resource'")) - - -def downgrade() -> None: - # Fails if a slot holds several upstream edges; remove the extra legs first. - op.drop_index(_INDEX, table_name=_TABLE) - op.create_index( - _INDEX, _TABLE, _COLUMNS, unique=True, postgresql_where=sa.text("type IN ('resource', 'dependency')") - ) - op.execute("UPDATE component_relations SET type = 'dependency' WHERE type = 'upstream'") diff --git a/packages/interloper-db/src/interloper_db/models/components.py b/packages/interloper-db/src/interloper_db/models/components.py index f3a8a9ec..1a59059a 100644 --- a/packages/interloper-db/src/interloper_db/models/components.py +++ b/packages/interloper-db/src/interloper_db/models/components.py @@ -128,14 +128,12 @@ def stamp_state(self, **fields: Any) -> None: class ComponentRelation(SQLModel, table=True): - """A typed, directed relation between two components. + """A named, directed relation between two components. - ``type`` names the relation; ``slot`` disambiguates multiple relations of - the same type on one source component (a resource slot name, an upstream - parameter name, empty when the relation has no slot semantics). - - Checks constrain the types the schema knows and permit any type they - don't, so new relation types need no schema change. + ``name`` is the relation name as declared on the owning class: the field + a ``Relation`` is bound to. Whether a given name is single-valued or + many-valued is a class rule the store enforces, not a schema constraint, + so no uniqueness is declared here. """ __tablename__: ClassVar[str] = "component_relations" @@ -152,36 +150,13 @@ class ComponentRelation(SQLModel, table=True): ondelete="CASCADE", name="fk_component_relations_dst", ), - # Relation shapes (which types a kind may declare, which kinds they may - # point at, slotted or not) are enforced by the store from the class - # vocabulary — an open set, so it is deliberately not mirrored in CHECKs. - # Resource slots are single-valued by schema. Upstream slots are - # single-valued only when their class says so, which the store - # enforces from the slot contract (a many-valued slot fans in). - Index( - "uq_component_relations_slot", - "src_id", - "type", - "slot", - unique=True, - postgresql_where=text("type = 'resource'"), - sqlite_where=text("type = 'resource'"), - ), - Index("ix_component_relations_org_id_type", "org_id", "type"), - Index("ix_component_relations_dst_id_type", "dst_id", "type"), + Index("ix_component_relations_org_id_name", "org_id", "name"), + Index("ix_component_relations_dst_id_name", "dst_id", "name"), ) src_id: UUID = SQLField(primary_key=True) - type: str = SQLField(primary_key=True) - slot: str = SQLField(default="", primary_key=True) + name: str = SQLField(primary_key=True) dst_id: UUID = SQLField(primary_key=True) org_id: UUID src_kind: str dst_kind: str - - dst: Component = Relationship( - sa_relationship_kwargs={ - "primaryjoin": "foreign(ComponentRelation.dst_id) == Component.id", - "viewonly": True, - }, - ) diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index a50fbb2e..1c641fcc 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -7,17 +7,21 @@ encrypted into the ``data`` column (fail-closed without a key) and decoded on read — callers only ever see ``config``. - **source**: child asset rows are kept in sync with the catalog class's - ``asset_types`` after every write, including intra-source dependency wiring. + ``asset_types`` after every write, including the sibling relations the + class declares between its own assets. - **asset**: a source-owned asset hydrates through its parent; its drift status cascades through the parent's. - **job**: hydration drift-checks every target before reconstruction. -Relation reads and writes live in the :class:`RelationMixin` layer this -mixin builds on — see :mod:`interloper_db.store.relations`. +Relation reads and writes are not here at all: this store composes a +:class:`~interloper_db.store.relations.RelationStore` and delegates to it, +so the acceptance rules a relation name carries live in one place (see +:mod:`interloper_db.store.relations`). """ from __future__ import annotations +import functools import json from typing import Any, cast from uuid import UUID @@ -44,20 +48,17 @@ from interloper_db.session import commit, session_scope from interloper_db.store.hydration import Hydrator from interloper_db.store.quotas import QUOTA_MAX_ASSETS_PER_SOURCE, QUOTA_MAX_SOURCES, QuotaStore -from interloper_db.store.relations import Binding, RelationStore, _add_relation +from interloper_db.store.relations import RelationStore from interloper_db.store.status import ComponentStatus, asset_status, source_status -# Eager-load set for rows returned to API consumers: the parent, children -# with their relations, and two hops (component → destination → resources). +# Eager-load set for rows returned to API consumers: the parent, the row's +# own relations, and the children with theirs, so the whole unit reads off a +# detached row. COMPONENT_LOAD_OPTIONS = [ selectinload(Component.parent), # ty: ignore[invalid-argument-type] + selectinload(Component.out_relations), # ty: ignore[invalid-argument-type] selectinload(Component.children) # ty: ignore[invalid-argument-type] - .selectinload(Component.out_relations) # ty: ignore[invalid-argument-type] - .selectinload(ComponentRelation.dst), # ty: ignore[invalid-argument-type] - selectinload(Component.out_relations) # ty: ignore[invalid-argument-type] - .selectinload(ComponentRelation.dst) # ty: ignore[invalid-argument-type] - .selectinload(Component.out_relations) # ty: ignore[invalid-argument-type] - .selectinload(ComponentRelation.dst), # ty: ignore[invalid-argument-type] + .selectinload(Component.out_relations), # ty: ignore[invalid-argument-type] ] @@ -102,7 +103,7 @@ def create( config: dict[str, Any] | None = None, encrypted: bool | None = None, children: list[str] | None = None, - relations: dict[str, list[Binding]] | None = None, + relations: dict[str, list[UUID]] | None = None, ) -> Component: """Create a component of any kind. @@ -117,7 +118,7 @@ def create( ``False`` opts into plaintext storage. children: Source kinds only — which child asset keys to enable (``None`` enables all the catalog class declares). - relations: ``{type: [(dst_id, slot), …]}`` — synced per type. + relations: ``{name: [dst_id, …]}``, replaced per name. Returns: The created component row, eager-loaded. @@ -187,7 +188,7 @@ def update( config: dict[str, Any] | None = None, encrypted: bool | None = None, children: list[str] | None = None, - relations: dict[str, list[Binding]] | None = None, + relations: dict[str, list[UUID]] | None = None, ) -> Component: """Update a component's spec. ``None`` leaves a facet untouched. @@ -196,7 +197,7 @@ def update( ``state`` column is left alone, with one exception: a job whose config changes has its cached ``next_run_at`` cleared, so the scheduler re-derives the schedule from the new spec on its next tick instead of - firing once more at the slot the old spec produced. + firing once more at the moment the old spec produced. Args: component_id: The component UUID. @@ -208,7 +209,7 @@ def update( ``False`` opts into plaintext storage. children: Source kinds only — the exact set of child asset keys to keep enabled. - relations: ``{type: [(dst_id, slot), …]}`` — synced per type. + relations: ``{name: [dst_id, …]}``, replaced per name. Returns: The updated component row, eager-loaded. @@ -242,11 +243,13 @@ def update( def delete(self, component_id: UUID) -> None: """Delete a component. Children and out-bound relations cascade via FK. - In-bound relations follow their declared ``on_delete`` semantics: - consumption relations (a bound connection, a required dependency) - block the deletion; orchestration pointers (a job's ``target``, a - hook's ``watch``, optional dependency slots) detach — the relation - row cascades away and the referrer keeps working with reduced scope. + In-bound relations follow the ``on_delete`` the referrer's class + declares for the name they are filed under, and nothing else: a + consuming relation (a bound ``connection``, a bound ``destination``, + a required upstream) blocks the deletion; one declared + ``on_delete="detach"`` (a job's ``targets``, a hook's ``watches``, an + optional upstream) detaches, its row cascading away while the + referrer keeps working with reduced scope. Args: component_id: The component UUID. @@ -276,13 +279,13 @@ def delete(self, component_id: UUID) -> None: def _blocking_referrers(self, session: Session, db_component: Component) -> list[dict[str, str | None]]: """Components outside a component's subtree whose relations into it block deletion. - Deleting a relation destination cascades the binding row, which would - leave a *consuming* referrer silently broken at its next run — those - relations refuse the deletion. Relations whose vocabulary declares - ``on_delete="detach"`` (and optional slots) are skipped: cascading - them is the intended outcome. Relations internal to the subtree (a - source's own asset dependencies) don't count, and a referrer that is - a source-owned asset is reported as its parent source — the unit the + Deleting a relation destination cascades the edge row, which would + leave a *consuming* referrer silently broken at its next run, so + those relations refuse the deletion. An edge whose name the referrer + declares ``on_delete="detach"`` is skipped: cascading it is the + intended outcome. Edges internal to the subtree (a source's + own sibling relations) don't count, and a referrer that is a + source-owned asset is reported as its parent source, the unit the user can act on. Args: @@ -340,7 +343,11 @@ def load(self, component_id: UUID) -> il.Component: """Hydrate a framework component of any kind from its row. Source-owned assets hydrate through their parent source and are - extracted from it; jobs drift-check every target first. Fails closed + extracted from it; jobs drift-check every target first. One cache + backs the whole call (see :meth:`_load`), so a component reached + several times within the same document (a job's own target and, + through a cross-source upstream, that same target again) hydrates + once and every consumer binds the identical instance. Fails closed on any catalog drift: see :meth:`_load` for what a missing row, a drifted key or a failed reconstruction raises. @@ -353,13 +360,34 @@ def load(self, component_id: UUID) -> il.Component: with tracer().start_as_current_span( "interloper.store.load", attributes={attributes.TARGET_ID: str(component_id)} ): - return self._load(component_id) + return self._load(component_id, {}) - def _load(self, component_id: UUID) -> il.Component: + def _load( + self, + component_id: UUID, + cache: dict[UUID, il.Component], + chain: tuple[tuple[UUID, str], ...] = (), + ) -> il.Component: """Hydrate a component row (the traced body of :meth:`load`). + A spec reference (``{"ref": id}``) is resolved from the document + first, which reconstruction does on its own, and only then through + :meth:`_resolve_reference`, which re-enters here with the same + *cache* and *chain* rather than starting a fresh call: a job that + targets both a source and a consumer of that source's asset holds + one instance of it, not two, and an owned asset's parent (loaded + through :meth:`_load_owned_asset`) shares the cache the same way. + *chain* is the trail of ids currently being hydrated, paired with + the catalog key each was hydrated under; a reference back onto one + of them is a cycle, reported from the trail rather than left to + exhaust the stack. + Args: component_id: The component UUID. + cache: Components already hydrated within this :meth:`load` call, + by id, consulted before doing any work. + chain: Ids currently being hydrated in this call, in resolution + order. Defaults to ``()``, the top-level call's empty trail. Returns: The reconstructed framework component. @@ -367,9 +395,17 @@ def _load(self, component_id: UUID) -> il.Component: Raises: NotFoundError: If the component is not found. ComponentDriftError: If a catalog key no longer resolves. - HydrationError: If the stored payload does not decrypt, or if - reconstruction fails. + HydrationError: If the stored payload does not decrypt, if + reconstruction fails, or if a reference revisits an id + already being hydrated earlier in the same call. """ + if component_id in cache: + return cache[component_id] + cyclic_key = next((key for id_, key in chain if id_ == component_id), None) + if cyclic_key is not None: + trail = " -> ".join(key for _, key in chain) + raise HydrationError(f"Reference cycle while hydrating: {trail} -> {cyclic_key}") + with session_scope(self._engine) as session: db_component = session.get(Component, component_id) if not db_component: @@ -395,28 +431,74 @@ def _load(self, component_id: UUID) -> il.Component: # Reconstruction happens outside the session: it imports classes and, # for owned assets, recursively loads the parent source. + chain = (*chain, (component_id, db_component.key)) if owned_asset: - return self._load_owned_asset(db_component.parent_id, db_component.key, component_id) - try: - return il.Component.from_spec(spec) - except Exception as e: - # format_exception, never str(e): a ValidationError here carries the - # decrypted payload of sensitive kinds in its input_value dumps, and - # this message is persisted into run events and shown in the UI. - raise HydrationError( - f"Failed to hydrate {db_component.kind} '{db_component.key}' ({db_component.id}): {format_exception(e)}" - ) from e - - def _load_owned_asset(self, parent_id: UUID, key: str, asset_id: UUID) -> il.Asset: + component = self._load_owned_asset(db_component.parent_id, db_component.key, component_id, cache, chain) + else: + resolve = functools.partial(self._resolve_reference, cache=cache, chain=chain) + try: + component = il.Component.from_spec(spec, resolve=resolve) + except (ComponentDriftError, NotFoundError): + # A dedicated handler (the API's drift endpoint, say) needs to + # tell these apart from a generic hydration failure, so they + # pass through untouched rather than folding into the catch-all. + raise + except Exception as e: + # format_exception, never str(e): a ValidationError here carries the + # decrypted payload of sensitive kinds in its input_value dumps, and + # this message is persisted into run events and shown in the UI. + raise HydrationError( + f"Failed to hydrate {db_component.kind} '{db_component.key}' ({db_component.id}): " + f"{format_exception(e)}" + ) from e + cache[component_id] = component + return component + + def _resolve_reference( + self, + reference: str, + *, + cache: dict[UUID, il.Component], + chain: tuple[tuple[UUID, str], ...], + ) -> il.Component: + """Hydrate the component a spec reference names from outside its document. + + Args: + reference: Id of the referenced component, as the spec carries it. + cache: Components already hydrated within the enclosing + :meth:`load` call, consulted (and extended) instead of + hydrating a fresh instance for a component reached again. + chain: Ids currently being hydrated in the enclosing call, for + :meth:`_load`'s cycle check. + + Returns: + The referenced component, hydrated through the store. + """ + return self._load(UUID(reference), cache, chain) + + def _load_owned_asset( + self, + parent_id: UUID, + key: str, + asset_id: UUID, + cache: dict[UUID, il.Component], + chain: tuple[tuple[UUID, str], ...], + ) -> il.Asset: """Hydrate a source-owned asset through its parent source. - The parent source is the unit of reconstruction — loading it binds - all its assets — and the child is picked out by key. + The parent source is the unit of reconstruction: loading it binds all + its assets, and the child is picked out by key. The parent loads + through the same *cache* and *chain* as the asset itself, so a + source hydrates once even when several of its owned assets are each + reached independently within one :meth:`load` call. Args: parent_id: UUID of the owning source component. key: Catalog key of the asset to pick out of the source. asset_id: UUID of the asset row, for the drift error message. + cache: Components already hydrated within the enclosing + :meth:`load` call. + chain: Ids currently being hydrated in the enclosing call. Returns: The bound asset instance. @@ -424,7 +506,7 @@ def _load_owned_asset(self, parent_id: UUID, key: str, asset_id: UUID) -> il.Ass Raises: ComponentDriftError: If the source no longer declares the key. """ - source = cast(il.Source, self.load(parent_id)) + source = cast(il.Source, self._load(parent_id, cache, chain)) for asset in source.assets: if asset.key == key: return asset @@ -722,10 +804,10 @@ def _ensure_children(self, session: Session, db_source: Component, child_keys: l When ``child_keys`` is provided, only those assets will exist — missing ones are created, extra ones are removed. Removal follows the delete guard's semantics: blocking relations from outside the source - (a required cross-source dependency) raise ``InUseError``; detaching + (a required cross-source upstream) raise ``InUseError``; detaching ones and intra-source edges cascade. ``None`` is the source-creation default and enables every asset the catalog class declares. Existing - rows keep their IDs (and therefore their cross-source deps, event + rows keep their IDs (and therefore their cross-source upstreams, event references, and per-asset overrides). Args: @@ -787,45 +869,7 @@ def _ensure_children(self, session: Session, db_source: Component, child_keys: l session.add(children[key]) session.flush() - self._wire_intra_upstreams(session, source_cls, children) - - @staticmethod - def _wire_intra_upstreams( - session: Session, - source_cls: type[il.Source], - children_by_key: dict[str, Component], - ) -> None: - """Top up missing intra-source upstream relations from class metadata. - - Idempotent over the full child set — assets enabled after their - siblings still get the edges *into* them wired. Slots that already - hold an edge are never touched, so manual bindings survive. - - Args: - session: Open session the relations are added to. - source_cls: The catalog class declaring the upstreams. - children_by_key: The source's enabled child rows, keyed by asset key. - """ - source_key = source_cls.key - child_ids = [child.id for child in children_by_key.values()] - bound = { - (row[0], row[1]) - for row in session.exec( - select(ComponentRelation.src_id, ComponentRelation.slot).where( - col(ComponentRelation.src_id).in_(child_ids), - ComponentRelation.type == "upstream", - ) - ).all() - } - for asset_type in source_cls.asset_types: - asset_key = asset_type.key - child = children_by_key.get(asset_key) - if child is None: - continue - for param_name, sibling_key in asset_type.sibling_upstreams(source_key, children_by_key).items(): - if (child.id, param_name) in bound: - continue - _add_relation(session, child, children_by_key[sibling_key], "upstream", param_name) + self._relations.bind_siblings(session, source_cls, children) def job_partition_granularity(self, session: Session, job_id: UUID) -> TimeGranularity | None: """Resolve the granularity a job's partitioned targets share. @@ -850,7 +894,7 @@ def job_partition_granularity(self, session: Session, job_id: UUID) -> TimeGranu """ granularities: set[TimeGranularity] = set() targets = session.exec( - select(ComponentRelation).where(ComponentRelation.src_id == job_id, ComponentRelation.type == "target") + select(ComponentRelation).where(ComponentRelation.src_id == job_id, ComponentRelation.name == "targets") ).all() for relation in targets: target = session.get(Component, relation.dst_id) @@ -898,7 +942,9 @@ def _check_job_targets(self, session: Session, db_job: Component) -> None: ComponentDriftError: If a target's catalog key is disabled or missing. """ targets = session.exec( - select(ComponentRelation).where(ComponentRelation.src_id == db_job.id, ComponentRelation.type == "target") + select(ComponentRelation).where( + ComponentRelation.src_id == db_job.id, ComponentRelation.name == "targets" + ) ).all() for relation in targets: target = session.get(Component, relation.dst_id) diff --git a/packages/interloper-db/src/interloper_db/store/hydration.py b/packages/interloper-db/src/interloper_db/store/hydration.py index 1c0ff0a0..b1bbe6be 100644 --- a/packages/interloper-db/src/interloper_db/store/hydration.py +++ b/packages/interloper-db/src/interloper_db/store/hydration.py @@ -13,10 +13,17 @@ One builder covers every kind: a component's init is its ``config`` (or its decrypted ``data`` for secret-bearing kinds) plus whatever its outgoing -relations and children contribute. Relations are mapped through the row's -own vocabulary (the catalog class's definition, anchor as drift fallback), -so the walk needs no kind dispatch: an asset simply has no ``target`` -relations, a destination no ``upstream`` ones. +relations and children contribute. Relations are read by name and checked +against the row's own vocabulary (the catalog class's declaration, the +kind's anchor as drift fallback), so the walk needs no kind dispatch: an +asset simply holds no ``targets`` edges, a destination no upstream ones. + +A target is written out by the rule ``Component.to_spec`` follows: one +owned by a source travels inside that source's own spec, so it is always a +``{"ref": id}``, and a parentless one is written out in full the first time +the walk reaches it and referenced afterwards. Resolving a reference the +document does not carry is the caller's job (see +:meth:`~interloper_db.store.components.ComponentStore._load`). The Store wraps this pattern in thin ``load_*`` convenience methods, but any caller can use the hydrator directly to assemble a spec (for example, @@ -32,8 +39,9 @@ from interloper.catalog.base import Catalog from interloper.component import KINDS -from interloper.errors import CatalogKeyError, HydrationError, format_exception +from interloper.errors import CatalogKeyError, ComponentDriftError, HydrationError, format_exception from interloper.serializable import Spec +from interloper.source.base import SourceDefinition from sqlmodel import Session, select from interloper_db.models import Component, ComponentRelation @@ -63,18 +71,28 @@ def __init__( self._catalog = catalog self._decrypt = decrypt - def build_component_spec(self, session: Session, db_component: Component) -> Spec: + def build_component_spec( + self, + session: Session, + db_component: Component, + *, + seen: set[str] | None = None, + ) -> Spec: """Build a spec for a component row of any kind. Args: session: Active DB session (used to walk relations and children). db_component: The component row. + seen: Ids the walk has already written out in full, extended with + this row's own. One set is shared by every spec of a document, + which is what turns a repeated target into a reference. + Defaults to ``None``, which starts a document of this row alone. Returns: A ``Spec`` with the row's ``id`` and a fully resolved init payload (nested components as nested specs). """ - init = self._build_init(session, db_component) + init = self._build_init(session, db_component, seen=seen) return Spec( path=self._resolve_path(session, db_component), id=str(db_component.id) if db_component.id else "", @@ -116,68 +134,118 @@ def decode_data(self, db_component: Component) -> dict[str, Any]: # -- Internals ------------------------------------------------------------- - def _build_init(self, session: Session, db_component: Component) -> dict[str, Any]: + def _build_init( + self, + session: Session, + db_component: Component, + *, + seen: set[str] | None = None, + ) -> dict[str, Any]: """Build the init payload for a component row. - Relations are mapped through the kind's own vocabulary: each type - fills its declared ``field``, shaped by its definition — slotted - types as ``{slot: value}``, unslotted ones as lists, carrying - nested specs (``inline``) or bare instance ids. - Children are the one non-relation contribution: they embed as the - ``assets`` override map, since the parent source is the unit of - reconstruction. + Each relation name the row holds edges under sits in the payload + under that name, a list when the declared relation is ``many`` and a + single value otherwise. Children are the one non-relation + contribution: they embed as the ``assets`` override map, since the + parent source is the unit of reconstruction. A child the parent's + declaration has dropped is refused as drift by :meth:`_check_declared` + before its own relations are read: those rows are declared by the + class the child no longer belongs to, so reading them would report + the parent's drift as an undeclared relation name on the child. Args: session: Active DB session, used to read relations and children. db_component: The component row whose init payload is built. + seen: Ids the walk has already written out in full, extended with + this row's own before anything else runs (so a cycle back + onto it is caught the same way whether this is the top of the + walk or a nested call). Defaults to ``None``, which starts a + document of this row alone. Returns: A dict suitable for use as a ``Spec.init``. Raises: - HydrationError: If the row carries a relation type its kind's - vocabulary does not declare. + HydrationError: If the row holds edges under a relation name its + class does not declare, or holds more than one row under a + relation its class declares single-valued. """ + seen = set() if seen is None else seen + seen.add(str(db_component.id) if db_component.id else "") if KINDS[db_component.kind].sensitive: init = self.decode_data(db_component) else: init = dict(db_component.config or {}) - vocabulary = self._catalog.vocabulary(db_component.kind, db_component.key) - for relation_type, rels in self._relations_by_type(session, db_component.id).items(): - definition = vocabulary.get(relation_type) - if definition is None: + vocabulary = self._catalog.vocabulary( + db_component.kind, db_component.key, parent_key=db_component.parent_key(session) + ) + for name, rows in self._relations_by_name(session, db_component.id).items(): + relation = vocabulary.get(name) + if relation is None: raise HydrationError( - f"Component {db_component.id} ({db_component.kind}) has '{relation_type}' relations, " - "which its kind's vocabulary does not declare" + f"Component {db_component.id} ({db_component.kind}) has '{name}' relations " + "its class does not declare" + ) + values = [self._dst_value(session, row, seen) for row in rows] + if relation.many: + init[name] = values + elif len(values) > 1: + raise HydrationError( + f"Component {db_component.id} ({db_component.kind}) holds {len(values)} rows under " + f"single-valued relation '{name}'" ) - if definition.inline: - values = [self._dst_spec(session, rel).model_dump(mode="json") for rel in rels] else: - values = [str(rel.dst_id) for rel in rels] - init[definition.field] = ( - {rel.slot: value for rel, value in zip(rels, values)} if definition.slotted else values - ) + init[name] = values[0] children = session.exec( select(Component).where(Component.parent_id == db_component.id).order_by(Component.created_at) # ty: ignore[invalid-argument-type] ).all() - if assets := { - child.key: {"id": str(child.id), **self._build_init(session, child)} for child in children - }: + assets: dict[str, Any] = {} + for child in children: + self._check_declared(db_component, child) + assets[child.key] = {"id": str(child.id), **self._build_init(session, child, seen=seen)} + if assets: init["assets"] = assets return init - def _relations_by_type(self, session: Session, src_id: UUID | None) -> dict[str, list[ComponentRelation]]: - """Group a component's outgoing relations by type, ordered stably. + def _check_declared(self, db_parent: Component, db_child: Component) -> None: + """Refuse a child row whose key its parent no longer declares. + + The same question :func:`~interloper_db.store.status.asset_status` + asks of an asset row, asked here of the parent's whole child set: the + catalog's own declaration of the parent is authoritative, and a child + outside it has drifted out of the source. A parent that does not + resolve as a source declares nothing to check against, and is left to + :meth:`_resolve_path` to report. + + Args: + db_parent: The owning component row, whose declaration decides. + db_child: The child row whose key is checked. + + Raises: + ComponentDriftError: If the parent's declaration does not name + the child's key. + """ + definition = self._catalog.get(db_parent.key) + if not isinstance(definition, SourceDefinition): + return + if db_child.key not in {asset.key for asset in definition.assets}: + raise ComponentDriftError( + f"Asset '{db_child.key}' ({db_child.id}) is no longer declared by source " + f"'{db_parent.key}' ({db_parent.id}); its catalog key has drifted." + ) + + def _relations_by_name(self, session: Session, src_id: UUID | None) -> dict[str, list[ComponentRelation]]: + """Group a component's outgoing relations by name, ordered stably. Args: session: Active DB session used to read the relation rows. src_id: The source component's id, or ``None`` for an unflushed row. Returns: - A ``{type: relations}`` mapping ordered by ``(slot, dst_id)``, or + A ``{name: relations}`` mapping ordered by ``(name, dst_id)``, or ``{}`` when ``src_id`` is ``None``. """ if src_id is None: @@ -185,31 +253,36 @@ def _relations_by_type(self, session: Session, src_id: UUID | None) -> dict[str, rows = session.exec( select(ComponentRelation) .where(ComponentRelation.src_id == src_id) - .order_by(ComponentRelation.slot, ComponentRelation.dst_id) # ty: ignore[invalid-argument-type] + .order_by(ComponentRelation.name, ComponentRelation.dst_id) # ty: ignore[invalid-argument-type] ).all() grouped: dict[str, list[ComponentRelation]] = {} - for rel in rows: - grouped.setdefault(rel.type, []).append(rel) + for row in rows: + grouped.setdefault(row.name, []).append(row) return grouped - def _dst_spec(self, session: Session, rel: ComponentRelation) -> Spec: - """Build the spec of a relation's destination component. + def _dst_value(self, session: Session, row: ComponentRelation, seen: set[str]) -> dict[str, Any]: + """Write out one edge's destination, in full or as a reference. Args: session: Active DB session used to load the destination row. - rel: The relation whose ``dst_id`` is resolved. + row: The edge whose ``dst_id`` is written out. + seen: Ids the walk has already written out in full, extended with + the destination's own when it is written out here. Returns: - The destination component's ``Spec``. + The destination's own spec as a JSON-able mapping, or the + ``{"ref": id}`` reference standing in for it. Raises: - HydrationError: If the relation points at a component row that - does not exist. + HydrationError: If the edge points at a component row that does + not exist. """ - db_dst = session.get(Component, rel.dst_id) + db_dst = session.get(Component, row.dst_id) if db_dst is None: # defensive: FKs make this unreachable - raise HydrationError(f"Relation {rel.src_id} -[{rel.type}]-> {rel.dst_id} points at a missing component") - return self.build_component_spec(session, db_dst) + raise HydrationError(f"Relation {row.src_id} -[{row.name}]-> {row.dst_id} points at a missing component") + if db_dst.parent_id is not None or str(db_dst.id) in seen: + return Spec.reference(str(db_dst.id)) + return self.build_component_spec(session, db_dst, seen=seen).model_dump(mode="json") def _resolve_path(self, session: Session, db_component: Component) -> str: """Look up a component's import path via the catalog. diff --git a/packages/interloper-db/src/interloper_db/store/relations.py b/packages/interloper-db/src/interloper_db/store/relations.py index 0ceef34c..84f0492c 100644 --- a/packages/interloper-db/src/interloper_db/store/relations.py +++ b/packages/interloper-db/src/interloper_db/store/relations.py @@ -1,36 +1,37 @@ -"""Relation policy: validated reads and writes for the edge table. - -The class vocabulary is the contract. It resolves parent-aware, a -source-owned asset's definition (upstream slots, ``optional`` flags) -lives on the parent source's definition, with the kind's anchor as the -drift fallback. Writes enforce the declared shape: relation type, dst -kind, slot names, and each slot's expected destination identity (resolved -through :meth:`~interloper.component.relation.ComponentIdentity.resolve` for upstream -slots). Unbinding follows the vocabulary's ``on_unbind`` semantics: bound -required slots of a blocking type refuse it. Rows are stamped with the +"""Relation policy: validated reads and writes for the named edge table. + +An edge is a name, and the class declaring that name is the contract. The +vocabulary resolves parent-aware, since a source-owned asset's declaration +lives on the source that owns it, with the kind's anchor as the drift +fallback. Every write asks the declared +:class:`~interloper.component.relation.Relation` whether it accepts the +candidate, so the database enforces exactly what the framework enforces in +memory: the declared kinds, and the identity the declared keys expect +(bare, qualified or wildcard). No component fills its own relation, and a +bare key stays inside the owner's own source instance, which identities +alone cannot tell apart. Single-valued names hold one edge and repoint on +rewrite; ``many`` names accumulate. A non-optional name cannot be +emptied, only repointed. Every write takes the source row's lock, so the +guards cannot be read around concurrently. Rows are stamped with the denormalized ``org_id``/``src_kind``/``dst_kind`` triple the composite foreign keys verify. """ from __future__ import annotations -from collections.abc import Iterable from uuid import UUID import interloper as il from interloper.catalog.base import Catalog -from interloper.component.relation import ComponentIdentity from interloper.errors import ConfigError, NotFoundError from sqlalchemy import Engine -from sqlalchemy.exc import IntegrityError -from sqlmodel import Session, select +from sqlmodel import Session, col, select from interloper_db.models import Component, ComponentRelation from interloper_db.session import commit, session_scope -# One relation binding: (destination component id, slot). Slot is "" for -# slotless relation types. -Binding = tuple[UUID, str] +# One relation binding: the destination component id. +Binding = UUID class RelationStore: @@ -48,405 +49,381 @@ def __init__(self, engine: Engine, catalog: Catalog) -> None: # -- Public API ------------------------------------------------------------ - def list_all(self, org_id: UUID, *, type: str | None = None) -> list[ComponentRelation]: - """List an organisation's component relations, optionally by type. + def list_all( + self, + org_id: UUID, + *, + name: str | None = None, + src_kind: str | None = None, + dst_kind: str | None = None, + ) -> list[ComponentRelation]: + """List an organisation's component relations, optionally filtered. Args: org_id: Organisation whose relations are listed. - type: Relation type to restrict the listing to. None (the default) - lists every type. + name: Relation name to restrict the listing to. None (the default) + lists every name. + src_kind: Kind the source component must have. None (the default) + accepts any kind. + dst_kind: Kind the destination component must have. None (the + default) accepts any kind. Returns: - The organisation's relation rows, in no guaranteed order. + The organisation's matching relation rows, in no guaranteed order. """ with session_scope(self._engine) as session: statement = select(ComponentRelation).where(ComponentRelation.org_id == org_id) - if type: - statement = statement.where(ComponentRelation.type == type) + if name: + statement = statement.where(ComponentRelation.name == name) + if src_kind: + statement = statement.where(ComponentRelation.src_kind == src_kind) + if dst_kind: + statement = statement.where(ComponentRelation.dst_kind == dst_kind) return list(session.exec(statement).all()) - def add(self, component_id: UUID, *, type: str, dst_id: UUID, slot: str = "") -> ComponentRelation: - """Add one relation from a component. + def add(self, component_id: UUID, *, name: str, dst_id: UUID) -> ComponentRelation: + """Bind one component to another under a declared relation name. - Slotted types upsert per slot: re-binding an already-bound slot - repoints it to the new destination, and re-adding an identical - relation is a no-op returning the existing row. + A ``many`` name accumulates; a single-valued one repoints, so + rebinding it needs no :meth:`remove` first. Re-adding an edge that is + already there returns it untouched. An undeclared name, a missing or + cross-org endpoint and a destination the declared relation refuses all + propagate from the checks this delegates to (:meth:`_lock`, + :meth:`_relation`, :meth:`_resolve`). Args: component_id: Source component the relation originates from. - type: Relation type, which the source kind's vocabulary must - declare. + name: Relation name, which the source's class must declare. dst_id: Destination component the relation points at. Must belong to the same organisation as the source. - slot: Slot the relation fills, for slotted types. Empty (the - default) for slotless ones. Returns: - The relation (created, repointed, or already present). - - Raises: - NotFoundError: If either endpoint is missing or cross-org. - ConfigError: If the kind's vocabulary doesn't declare the type or - the slot, or the destination doesn't match the slot's shape. + The relation row (created, repointed, or already present). """ with session_scope(self._engine) as session: - src = session.get(Component, component_id) - if not src: - raise NotFoundError(f"Component {component_id} not found") - definition = self._check_vocabulary(session, src, type) - dst = self._resolve_dst(session, src, definition, type, slot, dst_id) - relation = self._upsert_relation(session, src, dst, type, slot, per_slot=definition.slotted) - try: - commit(session) - except IntegrityError: - raise ConfigError( - f"Relation '{type}'{f' slot {slot!r}' if slot else ''} on '{src.key}' " - f"was modified concurrently; retry" - ) - return relation + src = self._lock(session, component_id) + relation = self._relation(session, src, name) + dst = self._resolve(session, src, relation, name, dst_id) + existing = self._rows(session, src.id, name) + if match := next((row for row in existing if row.dst_id == dst.id), None): + return match + if not relation.many: + for row in existing: + session.delete(row) + session.flush() + row = self._insert(session, src, dst, name) + commit(session) + return row - def remove(self, component_id: UUID, *, type: str, dst_id: UUID) -> None: - """Remove a component's relations of one type toward one destination. + def remove(self, component_id: UUID, *, name: str, dst_id: UUID) -> None: + """Detach one component from another under a declared relation name. + + Takes the source row's lock before reading the edges, so the + non-optional guard below cannot be read around by a concurrent write. + A missing source propagates as a ``NotFoundError`` from + :meth:`_lock`. + + Unlike every other write path this takes no undeclared-name guard, + deliberately: a name the class no longer declares is exactly the row + someone needs to clear, and refusing it would leave the component + unloadable with no way out. Args: - component_id: Source component the relations originate from. - type: Relation type to remove. - dst_id: Destination the removed relations point at. Every slot - bound to it under *type* is removed. + component_id: Source component the relation originates from. + name: Relation name the edge is filed under. + dst_id: Destination the removed edge points at. An edge that isn't + there is a no-op. Raises: - ConfigError: If a matching edge fills a slot the vocabulary - refuses to unbind (a bound required dependency) — repoint - the slot or remove the dependent asset instead. + ConfigError: If the edge is the last one of a non-optional + relation, which may be repointed but not emptied. """ with session_scope(self._engine) as session: - statement = select(ComponentRelation).where( - ComponentRelation.src_id == component_id, - ComponentRelation.type == type, - ComponentRelation.dst_id == dst_id, - ) - relations = session.exec(statement).all() - src = session.get(Component, component_id) if relations else None - if src is not None: - definition = self._relation_vocabulary(session, src).get(type) - if blocked := self._blocked_unbinds(definition, (relation.slot for relation in relations)): - raise ConfigError( - f"Required '{type}' slot(s) {blocked} of '{src.key}' cannot be unbound; " - f"repoint them or remove the dependent asset instead" - ) - for relation in relations: - session.delete(relation) + src = self._lock(session, component_id) + rows = self._rows(session, src.id, name) + row = next((candidate for candidate in rows if candidate.dst_id == dst_id), None) + if row is None: + return + relation = self._vocabulary(session, src).get(name) + if len(rows) == 1 and relation is not None and not relation.optional: + raise ConfigError( + f"'{src.key}'.{name} is non-optional and cannot be emptied; " + f"repoint it or remove the dependent component instead" + ) + session.delete(row) commit(session) - # -- Internals ------------------------------------------------------------- - - def _sync_relations( + def bind_siblings( self, session: Session, - src: Component, - relations: dict[str, list[Binding]] | None, + source_cls: type[il.Source], + children_by_key: dict[str, Component], ) -> None: - """Replace the relation types present in *relations* (empty list clears). + """Top up the intra-source edges a source class binds between its own assets. + + The wiring is read from + :meth:`~interloper.source.base.Source.sibling_bindings`, the same + classmethod a live source binds its assets from, so a persisted source + holds exactly the edges its in-memory counterpart does: one edge per + relation name, never onto the declaring asset itself, and nothing for a + declared key that reaches outside the source. Idempotent over the full + child set, so an asset enabled after its siblings still gets the edges + into it wired, while a name that already holds an edge is left alone + and a binding made by hand survives. + + Args: + session: Open session the edges are added to; neither flushed nor + committed here. + source_cls: Catalog class whose declaration decides the wiring. + children_by_key: The source's enabled child rows, keyed by asset key. + """ + bound = { + (row[0], row[1]) + for row in session.exec( + select(ComponentRelation.src_id, ComponentRelation.name).where( + col(ComponentRelation.src_id).in_([child.id for child in children_by_key.values()]) + ) + ).all() + } + for asset_key, names in source_cls.sibling_bindings().items(): + child = children_by_key.get(asset_key) + if child is None: + continue + for name, sibling_key in names.items(): + sibling = children_by_key.get(sibling_key) + if sibling is None or (child.id, name) in bound: + continue + self._insert(session, child, sibling, name) + + # -- Internals ------------------------------------------------------------- - Bound slots the vocabulary refuses to unbind (required dependencies) - must stay bound — repointing (same slot, different destination) is - allowed. + def _sync_relations(self, session: Session, src: Component, bindings: dict[str, list[Binding]] | None) -> None: + """Replace the relation names present in *bindings* (empty list clears). + + Takes the source row's lock, like every other write path, so two + concurrent replacements of one name cannot interleave their deletes + and inserts. An undeclared name and a destination the declared + relation refuses both propagate from the checks this delegates to + (:meth:`_relation`, :meth:`_resolve`). Args: session: Open session the replacement is written through; deletes and inserts are flushed, never committed here. src: Source component whose relations are replaced. - relations: Bindings to install, keyed by relation type. Only the - types present are touched; an empty binding list clears that - type. None leaves every relation untouched. + bindings: Destination ids to install, keyed by relation name. Only + the names present are touched, each replaced wholesale. + None leaves every relation untouched. Raises: - ConfigError: If dropping a binding would unbind a required slot - the vocabulary refuses to unbind. + ConfigError: If a single-valued name is given several + destinations, or if the replacement would empty a + non-optional relation. """ - for relation_type, bindings in (relations or {}).items(): - definition = self._check_vocabulary(session, src, relation_type) - existing = session.exec( - select(ComponentRelation).where( - ComponentRelation.src_id == src.id, ComponentRelation.type == relation_type - ) - ).all() - kept = {slot for _, slot in bindings} - dropped = (relation.slot for relation in existing if relation.slot not in kept) - if blocked := self._blocked_unbinds(definition, dropped): + if not bindings: + return + src = self._lock(session, src.id) + for name, dst_ids in bindings.items(): + relation = self._relation(session, src, name) + if not relation.many and len(dst_ids) > 1: + raise ConfigError(f"'{src.key}'.{name} is single-valued and takes one target at a time") + existing = self._rows(session, src.id, name) + # Gated on existing rows: creation may leave a non-optional name + # unbound, which only hydration refuses; an update that clears + # one is a removal. + if existing and not dst_ids and not relation.optional: raise ConfigError( - f"Required '{relation_type}' slot(s) {blocked} of '{src.key}' cannot be unbound; " - f"repoint them or remove the dependent asset instead" + f"'{src.key}'.{name} is non-optional and cannot be emptied; " + f"repoint it or remove the dependent component instead" ) - for relation in existing: - session.delete(relation) + kept = set(dst_ids) + for row in existing: + if row.dst_id not in kept: + session.delete(row) session.flush() - for dst_id, slot in bindings: - dst = self._resolve_dst(session, src, definition, relation_type, slot, dst_id) - _add_relation(session, src, dst, relation_type, slot) + held = {row.dst_id for row in existing if row.dst_id in kept} + for dst_id in dst_ids: + if dst_id in held: + continue + held.add(dst_id) + self._insert(session, src, self._resolve(session, src, relation, name, dst_id), name) + + def _relation_detaches(self, session: Session, referrer: Component, relation_row: ComponentRelation) -> bool: + """Whether an edge detaches (rather than blocks) when its destination is deleted. + + Consults the referrer's own vocabulary, where ``on_delete`` is the + sole authority: only a name declared ``on_delete="detach"`` detaches. + Whether the name is ``optional`` is a separate question, about leaving + it unbound or emptying it, and does not license deleting a target the + referrer holds. Anything unresolvable (an undeclared name, a drifted + key) blocks, keeping the guard fail-closed. - def _check_vocabulary(self, session: Session, src: Component, relation_type: str) -> il.RelationDefinition: - """Reject relation types the row's class vocabulary doesn't declare. + Args: + session: Open session the referrer's vocabulary is resolved + through. + referrer: Referrer row holding the edge. + relation_row: Edge whose destination is about to be deleted. + + Returns: + True if the edge may be detached, False if it blocks the deletion. + """ + relation = self._vocabulary(session, referrer).get(relation_row.name) + if relation is None: + return False + return relation.on_delete == "detach" + + def _vocabulary(self, session: Session, row: Component) -> dict[str, il.Relation]: + """The relation vocabulary governing a component row. + + Args: + session: Open session the parent row is read through. + row: Component row whose vocabulary is resolved. + + Returns: + The declared relations keyed by name, read from the owning + source's declaration for a source-owned asset. Empty when nothing + resolves. + """ + return self._catalog.vocabulary(row.kind, row.key, parent_key=row.parent_key(session)) + + def _relation(self, session: Session, src: Component, name: str) -> il.Relation: + """The relation a row's class declares under *name*. Args: - session: Open session used to resolve the row's vocabulary. - src: Source component whose vocabulary is consulted. - relation_type: Relation type to check against that vocabulary. + session: Open session the vocabulary is resolved through. + src: Component row whose class declares the relation. + name: Relation name to look up. Returns: - The type's relation definition (slots included for owned assets). + The declared relation. Raises: - ConfigError: If the source kind's vocabulary declares no relation - of that type. + ConfigError: If the row's class declares no relation of that name. """ - vocabulary = self._relation_vocabulary(session, src) - if relation_type not in vocabulary: + vocabulary = self._vocabulary(session, src) + if name not in vocabulary: raise ConfigError( - f"Components of kind '{src.kind}' ('{src.key}') declare no '{relation_type}' relations " - f"(allowed: {sorted(vocabulary) or 'none'})" + f"'{src.key}' ({src.kind}) declares no relation '{name}' (declared: {sorted(vocabulary)})" ) - return vocabulary[relation_type] + return vocabulary[name] - def _resolve_dst( - self, - session: Session, - src: Component, - definition: il.RelationDefinition, - relation_type: str, - slot: str, - dst_id: UUID, - ) -> Component: - """Resolve a relation destination, enforcing existence, same-org, and the vocabulary's shape. + def _identity(self, session: Session, row: Component) -> il.ComponentIdentity: + """What a component row is, for relation matching. + + Args: + session: Open session the parent row is read through. + row: Component row to identify. + + Returns: + The row's identity: its owning source's key (None when it has no + parent) and its own key. + """ + return il.ComponentIdentity(row.parent_key(session), row.key) + + def _resolve(self, session: Session, src: Component, relation: il.Relation, name: str, dst_id: UUID) -> Component: + """Load a relation destination and check the declared relation accepts it. + + Two rules the identity match cannot see are checked here. A component + never fills its own relation, whatever the declared keys match. And a + bare declared key names a component of the owner's *own* source + instance, while identities carry only the catalog key of the owning + source: two instances of one source have the same identity, so the + instance is compared by parent row. Args: session: Open session the destination row is loaded through. src: Source component the relation originates from. - definition: Relation definition the destination must satisfy - (allowed kinds, slotted-ness, declared slots). - relation_type: Relation type, used for error messages. - slot: Slot the relation fills. Empty for slotless types. + relation: Declared relation the destination must satisfy. + name: Relation name, used for error messages. dst_id: Destination component to resolve. Returns: - The destination row, validated against the definition. + The destination row, accepted by the relation. Raises: NotFoundError: If the destination is missing or belongs to another organisation. - ConfigError: If the destination is the source itself, its kind is - not allowed, a slot is given for a slotless type, or the slot - is not declared by the definition. + ConfigError: If the destination is the source itself, if the + relation does not accept the destination's kind or identity, + or if a sibling relation is pointed at another source + instance's component. """ dst = session.get(Component, dst_id) if dst is None or dst.org_id != src.org_id: - raise NotFoundError( - f"Component {dst_id} not found (relation '{relation_type}'{f'/{slot}' if slot else ''})" - ) + raise NotFoundError(f"Component {dst_id} not found (relation '{name}')") if dst.id == src.id: - raise ConfigError(f"Relation '{relation_type}' on '{src.key}' may not point at the component itself") - if dst.kind not in definition.kinds: + raise ConfigError(f"'{src.key}'.{name} cannot point at the component itself") + if not relation.accepts(dst.kind, self._identity(session, dst), owner=self._identity(session, src)): raise ConfigError( - f"Relation '{relation_type}' on kind '{src.kind}' may not point at a '{dst.kind}' " - f"component (allowed: {definition.kinds})" + f"'{src.key}'.{name} does not accept {dst.kind} '{dst.key}' " + f"(declared: kind {relation.kinds}, key {relation.keys or 'any'})" ) - if not definition.slotted and slot: - raise ConfigError(f"Relation '{relation_type}' on kind '{src.kind}' is not slotted (got slot '{slot}')") - # A definition with no declared slots (a kind anchor reached through - # the drift fallback) can't vet slot names or targets — skip, fail-open. - if definition.slotted and definition.slots: - slot_def = definition.slots.get(slot) - if slot_def is None: - raise ConfigError( - f"Relation '{relation_type}' on '{src.key}' declares no slot '{slot}' " - f"(declared: {sorted(definition.slots)})" - ) - self._check_slot_target(session, src, relation_type, slot, slot_def, dst) + if relation.local and src.parent_id is not None and dst.parent_id != src.parent_id: + raise ConfigError(f"'{src.key}'.{name} names a sibling; '{dst.key}' belongs to another source instance") return dst - def _check_slot_target( - self, session: Session, src: Component, relation_type: str, slot: str, slot_def: il.Dependency, dst: Component - ) -> None: - """Enforce a slot's declared destination identity, when it declares one. + def _lock(self, session: Session, component_id: UUID) -> Component: + """Load the source row of a write, holding it for the transaction. - Upstream slot keys resolve through ``ComponentIdentity.resolve``: an - intra-source dep (the declarer's own source) must bind a sibling of - the same source instance; a cross-source one accepts the named - source's asset from any instance. Other slotted types (resources) - declare a plain component key. + The row lock serializes concurrent writes to the same source, so two + rebinds of one single-valued name cannot both read an empty edge set + and both insert. Every write path goes through here, which is the + only guarantee available to a reader: SQLite, what the tests run on, + has no row locks and needs none, since its writes are serialized + database-wide, so no test can observe the lock being taken. Args: - session: Open session the parent rows are loaded through. - src: Source component declaring the slot. - relation_type: Relation type the slot belongs to. ``"upstream"`` selects - the asset-identity resolution described above. - slot: Slot name, used for error messages. - slot_def: Slot definition. An empty ``key`` declares no expected - identity and the check is skipped. - dst: Destination component to validate against the slot. - - Raises: - ConfigError: If the destination's key, or the source instance it - belongs to, doesn't match the slot's declared identity. - """ - if not slot_def.key: - return - if relation_type != "upstream": - if dst.key != slot_def.key: - raise ConfigError( - f"Relation '{relation_type}' slot '{slot}' of '{src.key}' expects a '{slot_def.key}' " - f"component, got '{dst.key}'" - ) - return - src_parent = session.get(Component, src.parent_id) if src.parent_id else None - own_source_key = src_parent.key if src_parent else None - expected = ComponentIdentity.resolve(slot_def.key, own_source_key=own_source_key) - if dst.key != expected.key: - raise ConfigError( - f"Upstream slot '{slot}' of '{src.key}' expects asset '{slot_def.key}', got '{dst.key}'" - ) - if expected.source_key == own_source_key: - if src.parent_id is not None and dst.parent_id != src.parent_id: - raise ConfigError( - f"Upstream slot '{slot}' of '{src.key}' must bind a sibling asset " - f"of the same source instance" - ) - else: - dst_parent = session.get(Component, dst.parent_id) if dst.parent_id else None - if dst_parent is None or dst_parent.key != expected.source_key: - raise ConfigError( - f"Upstream slot '{slot}' of '{src.key}' expects an asset of source " - f"'{expected.source_key}', got one of '{dst_parent.key if dst_parent else 'none'}'" - ) - - def _relation_vocabulary(self, session: Session, db_source: Component) -> dict[str, il.RelationDefinition]: - """A referrer row's relation vocabulary, slots included. - - Args: - session: Open session the parent row is loaded through. - db_source: Referrer row whose vocabulary is resolved. + session: Open session the row is loaded through. + component_id: Source component the write originates from. Returns: - The declared relation definitions keyed by relation type, from the - owning source's declaration for a source-owned asset. Empty when - nothing resolves. - """ - return self._catalog.vocabulary(db_source.kind, db_source.key, parent_key=db_source.parent_key(session)) - - def _relation_detaches(self, session: Session, db_source: Component, relation: ComponentRelation) -> bool: - """Whether a relation detaches (rather than blocks) when its destination is deleted. - - Consults the referrer's own vocabulary: a type declared - ``on_delete="detach"`` detaches, as does a slot the referrer declares - optional (``Dependency.optional=True``), such as an optional upstream - dependency. Anything unresolvable (unknown type, drifted key, - undeclared slot) blocks, keeping the guard fail-closed. - - Args: - session: Open session the referrer's vocabulary is resolved - through. - db_source: Referrer row holding the relation. - relation: Relation whose destination is about to be deleted. + The source row, locked on backends that support it. - Returns: - True if the relation may be detached, False if it blocks the - deletion. + Raises: + NotFoundError: If the component does not exist. """ - definition = self._relation_vocabulary(session, db_source).get(relation.type) - if definition is None: - return False - if definition.on_delete == "detach": - return True - slot = definition.slots.get(relation.slot) - return slot is not None and slot.optional + statement = select(Component).where(Component.id == component_id) + if session.bind is not None and session.bind.dialect.name == "postgresql": + statement = statement.with_for_update() + src = session.exec(statement).first() + if src is None: + raise NotFoundError(f"Component {component_id} not found") + return src @staticmethod - def _blocked_unbinds(definition: il.RelationDefinition | None, slots: Iterable[str]) -> list[str]: - """Bound slots whose explicit unbinding the vocabulary refuses. - - A slot blocks when its type declares ``on_unbind="block"`` and the slot - is not optional. Unknown definitions or slots (drift) don't block. + def _rows(session: Session, src_id: UUID, name: str) -> list[ComponentRelation]: + """The edges a component currently holds under one relation name. Args: - definition: Relation definition the slots belong to. None (an - unresolvable type) blocks nothing. - slots: Slot names about to be unbound. Duplicates are collapsed. + session: Open session the rows are read through. + src_id: Source component the edges originate from. + name: Relation name the edges are filed under. Returns: - The blocking slot names, sorted. Empty when the unbind is allowed. + The matching edge rows, in no guaranteed order. """ - if definition is None or definition.on_unbind != "block": - return [] - return sorted( - {slot for slot in slots if (slot_def := definition.slots.get(slot)) is not None and not slot_def.optional} - ) + statement = select(ComponentRelation).where(ComponentRelation.src_id == src_id, ComponentRelation.name == name) + return list(session.exec(statement).all()) @staticmethod - def _upsert_relation( - session: Session, src: Component, dst: Component, relation_type: str, slot: str, *, per_slot: bool - ) -> ComponentRelation: - """Idempotent relation write: an identical edge is returned as-is. - - With *per_slot* (slotted types), a slot bound to a different destination - is repointed — the old edge is replaced by the new one. + def _insert(session: Session, src: Component, dst: Component, name: str) -> ComponentRelation: + """Add one edge, stamping the denormalized org/kind triple from the rows. Args: - session: Open session the write goes through; flushed, never - committed here. + session: Open session the row is added to; not flushed or committed. src: Source component the relation originates from. dst: Destination component the relation points at. - relation_type: Relation type to write. - slot: Slot the relation fills. Empty for slotless types. - per_slot: Whether the type is slotted, making the slot alone the - identity of the edge (so re-binding repoints it). When False, - the destination is part of that identity. + name: Relation name to file the edge under. Returns: - The matching relation: the untouched existing row, or the newly - added one (pending, not flushed). + The pending edge (added to the session, not flushed). """ - statement = select(ComponentRelation).where( - ComponentRelation.src_id == src.id, - ComponentRelation.type == relation_type, - ComponentRelation.slot == slot, + row = ComponentRelation( + src_id=src.id, + name=name, + dst_id=dst.id, + org_id=src.org_id, + src_kind=src.kind, + dst_kind=dst.kind, ) - if not per_slot: - statement = statement.where(ComponentRelation.dst_id == dst.id) - existing = session.exec(statement).all() - if match := next((relation for relation in existing if relation.dst_id == dst.id), None): - return match - for relation in existing: - session.delete(relation) - if existing: - session.flush() - return _add_relation(session, src, dst, relation_type, slot) - - -def _add_relation( - session: Session, src: Component, dst: Component, relation_type: str, slot: str = "" -) -> ComponentRelation: - """Add one relation, stamping the denormalized org/kind triple from the rows. - - Args: - session: Open session the row is added to; not flushed or committed. - src: Source component the relation originates from. - dst: Destination component the relation points at. - relation_type: Relation type to write. - slot: Slot the relation fills. Empty (the default) for slotless types. - - Returns: - The pending relation (added to the session, not flushed). - """ - relation = ComponentRelation( - src_id=src.id, - type=relation_type, - slot=slot, - dst_id=dst.id, - org_id=src.org_id, - src_kind=src.kind, - dst_kind=dst.kind, - ) - session.add(relation) - return relation - - + session.add(row) + return row diff --git a/packages/interloper-db/tests/models/test_components.py b/packages/interloper-db/tests/models/test_components.py index 73490ef2..ec336984 100644 --- a/packages/interloper-db/tests/models/test_components.py +++ b/packages/interloper-db/tests/models/test_components.py @@ -13,9 +13,9 @@ import pydantic import pytest from sqlalchemy import Engine -from sqlmodel import Session +from sqlmodel import Session, select -from interloper_db.models import Component +from interloper_db.models import Component, ComponentRelation _ORG = uuid4() @@ -97,3 +97,31 @@ def test_a_shape_the_kind_does_not_declare_is_rejected(self): with pytest.raises(pydantic.ValidationError): row.stamp_state(next_run_at=object()) + + +class TestComponentRelation: + """A relation row is keyed by ``(src_id, name, dst_id)``, no ``type``/``slot``.""" + + def test_relation_row_is_keyed_by_name(self, component_db: Engine) -> None: + with Session(component_db) as session: + src = Component(org_id=_ORG, kind="source", key="s") + dst = Component(org_id=_ORG, kind="destination", key="d") + session.add_all([src, dst]) + session.flush() + session.add( + ComponentRelation( + src_id=src.id, + name="destinations", + dst_id=dst.id, + org_id=_ORG, + src_kind="source", + dst_kind="destination", + ) + ) + session.commit() + + row = session.exec(select(ComponentRelation)).one() + + assert row.name == "destinations" + assert not hasattr(row, "slot") + assert not hasattr(row, "type") diff --git a/packages/interloper-db/tests/store/test_components.py b/packages/interloper-db/tests/store/test_components.py index 1a9ab405..6ef98a50 100644 --- a/packages/interloper-db/tests/store/test_components.py +++ b/packages/interloper-db/tests/store/test_components.py @@ -4,7 +4,7 @@ import json from collections.abc import Callable -from typing import Any, ClassVar +from typing import ClassVar from uuid import uuid4 import interloper as il @@ -24,7 +24,7 @@ from sqlalchemy import Engine, create_engine from sqlmodel import Session, select -from interloper_db.models import Component, ComponentRelation +from interloper_db.models import Component from interloper_db.store import Store from interloper_db.store.components import ComponentStore from interloper_db.store.status import ComponentStatus @@ -32,21 +32,163 @@ _ORG = uuid4() +# -- Test components ----------------------------------------------------------- + + +class WireConnection(il.Connection): + """Connection the wire sources bind.""" + + +class PublicToggleConnection(il.Connection): + """Renewable test connection whose ``auto_renew`` is schema-marked public.""" + + api_key: str = il.SecretField() + + def renew(self) -> il.Renewal: + """Keep the class renewable so ``auto_renew`` stays in its schema. + + Returns: + An effectless renewal. + """ + return il.Renewal() + + +class GuardUpstream(il.Asset): + """Upstream asset of the delete-guard tests.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class GuardRequired(il.Asset): + """Asset whose required ``up`` relation blocks its upstream's deletion.""" + + up = il.Relation("asset", "guard_upstream") + + def data(self, context: il.ExecutionContext, up: il.Upstream) -> list[dict]: + return [] + + +class GuardOptional(il.Asset): + """Asset whose ``up`` relation detaches when its upstream goes.""" + + up = il.Relation("asset", "guard_upstream", optional=True, on_delete="detach") + + def data(self, context: il.ExecutionContext, up: il.Upstream | None) -> list[dict]: + return [] + + +class GuardOptionalBlocking(il.Asset): + """Asset whose ``up`` relation is optional yet keeps the default ``block``.""" + + up = il.Relation("asset", "guard_upstream", optional=True) + + def data(self, context: il.ExecutionContext, up: il.Upstream | None) -> list[dict]: + return [] + + +class WireUpSource(il.Source): + """Upstream source whose ``totals`` reads its sibling ``rows``.""" + + class Rows(il.Asset): + """Root asset of the source.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset reading the source's own ``rows``.""" + + rows = il.Relation("asset", "rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class WireDownSource(il.Source): + """Downstream source: a required connection and a required cross-source upstream.""" + + connection: WireConnection + + class Consumer(il.Asset): + """Asset reading ``wire_up_source.rows``.""" + + rows = il.Relation("asset", "wire_up_source.rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class WireUpNarrowedSource(il.Source): + """``wire_up_source`` as a later release declares it: no ``totals``.""" + + key = "wire_up_source" + + class Rows(il.Asset): + """The one asset the narrowed class still declares.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class WireDownOptionalSource(il.Source): + """Downstream source whose asset reads ``wire_up_source.rows`` detachably.""" + + class Reader(il.Asset): + """Asset with a detaching cross-source upstream.""" + + rows = il.Relation("asset", "wire_up_source.rows", optional=True, on_delete="detach") + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class DiscriminatedSource(il.Source): + """Source class whose instances are discriminated by ``account_id``.""" + + account_id: str = il.InputField(default="", discriminator=True) + + class DiscriminatedRows(il.Asset): + """Asset whose table name carries the instance discriminator.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + @pytest.fixture def store(component_db: Engine) -> Store: - """A store over the in-memory database (no catalog needed for these). + """A store whose catalog carries every class declared in this module. + + Returns: + A store reading and writing the fixture database. + """ + catalog = il.Catalog.from_assets( + [ + GuardUpstream, + GuardRequired, + GuardOptional, + GuardOptionalBlocking, + WireUpSource, + WireDownSource, + WireDownOptionalSource, + DiscriminatedSource, + ] + ) + return Store(catalog=catalog) + + +@pytest.fixture +def connection(store: Store) -> Component: + """A plaintext connection row the wire sources accept. Returns: - A store with an empty catalog, reading and writing the fixture database. + The created connection component. """ - return Store(catalog=il.Catalog(components={})) + return store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) -def _relations(session: Session, src_id, type: str | None = None) -> list[ComponentRelation]: - statement = select(ComponentRelation).where(ComponentRelation.src_id == src_id) - if type: - statement = statement.where(ComponentRelation.type == type) - return list(session.exec(statement).all()) +def _child(source: Component, key: str) -> Component: + return next(child for child in source.children if child.key == key) class TestCrud: @@ -68,12 +210,22 @@ def test_children_rejected_for_childless_kinds(self, store: Store): store.components.create(_ORG, kind="destination", key="dest", children=["a"]) def test_unknown_child_keys_rejected(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) with pytest.raises(ConfigError, match=r"declares no asset\(s\) \['typo'\]"): store.components.create(_ORG, kind="source", key="demo_source", children=["a", "typo"]) + def test_create_with_relations_by_name(self, store: Store, connection: Component): + destination = store.components.create(_ORG, kind="destination", key="dest") + + source = store.components.create( + _ORG, + kind="source", + key="wire_down_source", + relations={"connection": [connection.id], "destinations": [destination.id]}, + ) + + assert {relation.name for relation in source.out_relations} == {"connection", "destinations"} + def test_delete_refuses_source_owned_assets(self, store: Store, component_db: Engine): job = store.components.create(_ORG, kind="job", key="cron_job") # any parentable stand-in row with Session(component_db) as session: @@ -85,8 +237,6 @@ def test_delete_refuses_source_owned_assets(self, store: Store, component_db: En store.components.delete(child_id) def test_delete_source_removes_child_rows(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") assert source.children @@ -100,8 +250,8 @@ def test_delete_source_removes_child_rows(self, component_db: Engine): def test_delete_cascades_outbound_relations(self, store: Store): job = store.components.create(_ORG, kind="job", key="cron_job", name="J") - asset = store.components.create(_ORG, kind="asset", key="a") - store.relations.add(job.id, type="target", dst_id=asset.id) + asset = store.components.create(_ORG, kind="asset", key="guard_upstream") + store.relations.add(job.id, name="targets", dst_id=asset.id) store.components.delete(job.id) assert store.relations.list_all(_ORG) == [] @@ -123,193 +273,173 @@ def test_get_component_checks_kind(self, store: Store): class TestDeleteInUseGuard: - """A relation destination cannot be deleted while external referrers exist.""" + """A relation destination cannot be deleted while consuming referrers exist.""" - def _connection(self, store: Store) -> Component: - return store.components.create(_ORG, kind="connection", key="conn", name="Conn", config={}, encrypted=False) + def test_bound_connection_blocks_delete_and_names_referrer(self, store: Store, connection: Component): + source = store.components.create( + _ORG, kind="source", key="wire_down_source", name="Down", relations={"connection": [connection.id]} + ) - def test_bound_connection_blocks_delete_and_names_referrer(self, store: Store): - conn = self._connection(store) - asset = store.components.create(_ORG, kind="asset", key="a", name="A", relations={"resource": [(conn.id, "c")]}) + with pytest.raises(InUseError) as excinfo: + store.components.delete(connection.id) + assert excinfo.value.referrers == [ + {"id": str(source.id), "kind": "source", "key": "wire_down_source", "name": "Down"} + ] + assert "in use by Down" in str(excinfo.value) + + def test_bound_destination_blocks_delete_and_names_referrer(self, store: Store, connection: Component): + destination = store.components.create(_ORG, kind="destination", key="dest") + source = store.components.create( + _ORG, + kind="source", + key="wire_down_source", + name="Down", + relations={"connection": [connection.id], "destinations": [destination.id]}, + ) + # ``destinations`` is optional and many-valued yet keeps the default + # ``block``: a source writing to a destination consumes it, so the + # destination may not be deleted from under it. with pytest.raises(InUseError) as excinfo: - store.components.delete(conn.id) - assert excinfo.value.referrers == [{"id": str(asset.id), "kind": "asset", "key": "a", "name": "A"}] - assert "in use by A" in str(excinfo.value) + store.components.delete(destination.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(source.id)] + assert "in use by Down" in str(excinfo.value) + + def test_delete_succeeds_after_repointing(self, store: Store, connection: Component): + other = store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + source = store.components.create( + _ORG, kind="source", key="wire_down_source", relations={"connection": [connection.id]} + ) - def test_delete_succeeds_after_unbinding(self, store: Store): - conn = self._connection(store) - asset = store.components.create(_ORG, kind="asset", key="a", relations={"resource": [(conn.id, "c")]}) + # A required relation is repointed rather than emptied, which is what + # releases the connection the source used to consume. + store.relations.add(source.id, name="connection", dst_id=other.id) - store.relations.remove(asset.id, type="resource", dst_id=conn.id) - store.components.delete(conn.id) + store.components.delete(connection.id) with pytest.raises(NotFoundError): - store.components.get(conn.id) + store.components.get(connection.id) - def test_job_target_detaches(self, store: Store): - asset = store.components.create(_ORG, kind="asset", key="a") - job = store.components.create( - _ORG, kind="job", key="cron_job", name="J", relations={"target": [(asset.id, "")]} - ) + def test_job_targets_detach(self, store: Store): + asset = store.components.create(_ORG, kind="asset", key="guard_upstream") + job = store.components.create(_ORG, kind="job", key="cron_job", name="J", relations={"targets": [asset.id]}) store.components.delete(asset.id) assert store.components.get(job.id).id == job.id assert store.relations.list_all(_ORG) == [] - def test_hook_watch_detaches(self, store: Store): - asset = store.components.create(_ORG, kind="asset", key="a") - hook = store.components.create( - _ORG, kind="hook", key="webhook", name="H", relations={"watch": [(asset.id, "")]} - ) + def test_hook_watches_detach(self, store: Store): + asset = store.components.create(_ORG, kind="asset", key="guard_upstream") + hook = store.components.create(_ORG, kind="hook", key="webhook", name="H", relations={"watches": [asset.id]}) store.components.delete(asset.id) assert store.components.get(hook.id).id == hook.id assert store.relations.list_all(_ORG) == [] def test_blocking_relation_wins_over_detaching(self, store: Store): - conn = self._connection(store) - asset = store.components.create(_ORG, kind="asset", key="a", name="A", relations={"resource": [(conn.id, "c")]}) - store.components.create(_ORG, kind="job", key="cron_job", relations={"target": [(asset.id, "")]}) + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") + consumer = store.components.create( + _ORG, kind="asset", key="guard_required", name="A", relations={"up": [upstream.id]} + ) + store.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [consumer.id]}) - # The asset both consumes the connection (blocks its deletion) and is - # a job target (detachable) — deleting the asset succeeds, deleting - # the connection does not. + # The consumer both requires the upstream (blocking its deletion) and + # is a job target (detachable), so deleting the consumer succeeds + # while deleting the upstream does not. with pytest.raises(InUseError): - store.components.delete(conn.id) - store.components.delete(asset.id) + store.components.delete(upstream.id) + store.components.delete(consumer.id) - def test_referrer_through_child_reports_parent(self, store: Store, component_db: Engine): - conn = self._connection(store) - parent = store.components.create(_ORG, kind="job", key="cron_job", name="P") # parentable stand-in - with Session(component_db) as session: - child = Component(org_id=_ORG, kind="asset", key="a", parent_id=parent.id) - session.add(child) - session.commit() - child_id = child.id - store.relations.add(child_id, type="resource", dst_id=conn.id, slot="c") + def test_referrer_through_child_reports_parent(self, store: Store): + up = store.components.create(_ORG, kind="source", key="wire_up_source", name="Up") + down = store.components.create(_ORG, kind="source", key="wire_down_source", name="Down") + store.relations.add(_child(down, "consumer").id, name="rows", dst_id=_child(up, "rows").id) with pytest.raises(InUseError) as excinfo: - store.components.delete(conn.id) - assert [r["id"] for r in excinfo.value.referrers] == [str(parent.id)] + store.components.delete(up.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(down.id)] - def test_intra_subtree_relations_do_not_block(self, store: Store, component_db: Engine): - parent = store.components.create(_ORG, kind="job", key="cron_job", name="P") # parentable stand-in - with Session(component_db) as session: - a = Component(org_id=_ORG, kind="asset", key="a", parent_id=parent.id) - b = Component(org_id=_ORG, kind="asset", key="b", parent_id=parent.id) - session.add(a) - session.add(b) - session.commit() - a_id, b_id = a.id, b.id - store.relations.add(b_id, type="upstream", dst_id=a_id, slot="a") + def test_intra_subtree_relations_do_not_block(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_up_source") + assert store.relations.list_all(_ORG, name="rows") != [] # the source's own sibling edge - store.components.delete(parent.id) + store.components.delete(source.id) with pytest.raises(NotFoundError): - store.components.get(parent.id) - - -class GuardUpstream(il.Asset): - """Upstream asset for the delete-guard dependency tests.""" - - -class GuardRequired(il.Asset): - """Asset with a required dependency on ``guard_upstream``.""" + store.components.get(source.id) - depends_on: ClassVar[dict[str, Any]] = {"up": "guard_upstream"} +class TestUpstreamDeleteSemantics: + """``on_delete`` alone decides: a ``block`` upstream relation refuses the deletion, a ``detach`` one gives way.""" -class GuardOptional(il.Asset): - """Asset with an optional dependency on ``guard_upstream``.""" - - depends_on: ClassVar[dict[str, Any]] = {"up": il.Dependency(key="guard_upstream", optional=True)} - - -class TestDependencyDeleteSemantics: - """Required dependency slots block deletion; optional slots detach.""" - - @pytest.fixture - def dep_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([GuardUpstream, GuardRequired, GuardOptional])) - - def test_required_dependency_blocks(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up.id, "up")]} + def test_delete_blocks_on_blocking_referrer_and_detaches_the_detaching_one(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") + blocking = store.components.create( + _ORG, kind="asset", key="guard_required", name="Req", relations={"up": [upstream.id]} ) - - with pytest.raises(InUseError) as excinfo: - dep_store.components.delete(up.id) - assert [r["id"] for r in excinfo.value.referrers] == [str(down.id)] - - def test_optional_dependency_detaches(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_optional", relations={"upstream": [(up.id, "up")]} + detaching = store.components.create( + _ORG, kind="asset", key="guard_optional", name="Opt", relations={"up": [upstream.id]} ) - dep_store.components.delete(up.id) - assert dep_store.components.get(down.id).id == down.id - assert dep_store.relations.list_all(_ORG) == [] - - -class WireUpSource(il.Source): - """Upstream source for the cross-source dependency tests.""" - - class Rows(il.Asset): - """Upstream asset (key ``rows``).""" - - -class WireDownSource(il.Source): - """Downstream source whose asset requires ``wire_up_source.rows``.""" - - class Consumer(il.Asset): - """Asset with a required cross-source dependency.""" - - depends_on: ClassVar[dict[str, Any]] = {"rows": "wire_up_source.rows"} - + with pytest.raises(InUseError) as excinfo: + store.components.delete(upstream.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(blocking.id)] -class WireDownOptionalSource(il.Source): - """Downstream source whose asset optionally consumes ``wire_up_source.rows``.""" + store.components.delete(blocking.id) + store.components.delete(upstream.id) - class Reader(il.Asset): - """Asset with an optional cross-source dependency.""" - - depends_on: ClassVar[dict[str, Any]] = {"rows": il.Dependency(key="wire_up_source.rows", optional=True)} + assert store.components.get(detaching.id).id == detaching.id + assert store.relations.list_all(_ORG, name="up") == [] + def test_an_optional_relation_keeping_the_default_policy_blocks(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") + referrer = store.components.create( + _ORG, kind="asset", key="guard_optional_blocking", name="Opt", relations={"up": [upstream.id]} + ) -def _child(source: Component, key: str) -> Component: - return next(child for child in source.children if child.key == key) + # The two knobs are independent: the relation may be left unbound + # (``optional``), which says nothing about deleting what it holds. + with pytest.raises(InUseError) as excinfo: + store.components.delete(upstream.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(referrer.id)] class TestIntraSourceWiring: - """Intra-source dependency edges are derived idempotently over the full child set.""" + """Sibling relation edges are derived idempotently over the full child set.""" @pytest.fixture def demo_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource + """A store whose catalog carries the demo source's ``a -> b,c,d -> e`` DAG. + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DemoSource])) + def test_create_source_binds_sibling_relations(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_up_source") + + rows = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") + assert {(row.name, row.dst_id) for row in rows} == {("rows", _child(source, "rows").id)} + def test_full_dag_wired_on_create(self, demo_store: Store): demo_store.components.create(_ORG, kind="source", key="demo_source") - edges = demo_store.relations.list_all(_ORG, type="upstream") + edges = demo_store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") assert len(edges) == 6 # b,c,d -> a and e -> b,c,d def test_children_enabled_later_get_inbound_edges(self, demo_store: Store): source = demo_store.components.create(_ORG, kind="source", key="demo_source", children=["b", "e"]) - assert [r.slot for r in demo_store.relations.list_all(_ORG, type="upstream")] == ["b"] # only e -> b + assert [row.name for row in demo_store.relations.list_all(_ORG)] == ["b"] # only e -> b updated = demo_store.components.update(source.id, children=["a", "b", "e"]) - edges = demo_store.relations.list_all(_ORG, type="upstream") - by_slot = {r.slot: (r.src_id, r.dst_id) for r in edges} - assert set(by_slot) == {"a", "b"} - assert by_slot["a"] == (_child(updated, "b").id, _child(updated, "a").id) + edges = demo_store.relations.list_all(_ORG) + by_name = {row.name: (row.src_id, row.dst_id) for row in edges} + assert set(by_name) == {"a", "b"} + assert by_name["a"] == (_child(updated, "b").id, _child(updated, "a").id) def test_wiring_is_idempotent(self, demo_store: Store): source = demo_store.components.create(_ORG, kind="source", key="demo_source") demo_store.components.update(source.id, name="renamed") demo_store.components.update(source.id, children=["a", "b", "c", "d", "e"]) - assert len(demo_store.relations.list_all(_ORG, type="upstream")) == 6 + assert len(demo_store.relations.list_all(_ORG)) == 6 def test_update_without_children_leaves_child_set_untouched(self, demo_store: Store): source = demo_store.components.create(_ORG, kind="source", key="demo_source", children=["b", "e"]) @@ -320,50 +450,31 @@ def test_update_without_children_leaves_child_set_untouched(self, demo_store: St class TestChildRemovalGuard: """Narrowing a source's child set honors the delete guard's semantics.""" - @pytest.fixture - def wire_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([WireUpSource, WireDownSource, WireDownOptionalSource])) - - def test_removing_child_with_required_external_dep_blocked(self, wire_store: Store): - up = wire_store.components.create(_ORG, kind="source", key="wire_up_source", name="Up") - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source", name="Down") - wire_store.relations.add( - _child(down, "consumer").id, type="upstream", dst_id=_child(up, "rows").id, slot="rows" - ) + def test_removing_child_with_required_external_upstream_blocked(self, store: Store): + up = store.components.create(_ORG, kind="source", key="wire_up_source", name="Up") + down = store.components.create(_ORG, kind="source", key="wire_down_source", name="Down") + store.relations.add(_child(down, "consumer").id, name="rows", dst_id=_child(up, "rows").id) with pytest.raises(InUseError) as excinfo: - wire_store.components.update(up.id, children=[]) + store.components.update(up.id, children=[]) assert [r["id"] for r in excinfo.value.referrers] == [str(down.id)] - assert wire_store.relations.list_all(_ORG, type="upstream") != [] + assert store.relations.list_all(_ORG, name="rows") != [] - def test_removing_child_with_optional_external_dep_detaches(self, wire_store: Store): - up = wire_store.components.create(_ORG, kind="source", key="wire_up_source") - down = wire_store.components.create(_ORG, kind="source", key="wire_down_optional_source") - wire_store.relations.add( - _child(down, "reader").id, type="upstream", dst_id=_child(up, "rows").id, slot="rows" - ) + def test_removing_child_with_detaching_external_upstream_detaches(self, store: Store): + up = store.components.create(_ORG, kind="source", key="wire_up_source") + down = store.components.create(_ORG, kind="source", key="wire_down_optional_source") + store.relations.add(_child(down, "reader").id, name="rows", dst_id=_child(up, "rows").id) - updated = wire_store.components.update(up.id, children=[]) + updated = store.components.update(up.id, children=[]) assert updated.children == [] - assert wire_store.relations.list_all(_ORG, type="upstream") == [] + assert store.relations.list_all(_ORG, name="rows") == [] def test_intra_source_reshape_not_blocked(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") updated = store.components.update(source.id, children=["a"]) assert [child.key for child in updated.children] == ["a"] - assert store.relations.list_all(_ORG, type="upstream") == [] - - -class DiscriminatedSource(il.Source): - """Source class whose instances are discriminated by ``account_id``.""" - - account_id: str = il.InputField(default="", discriminator=True) - - class DiscriminatedRows(il.Asset): - """Asset whose table name carries the instance discriminator.""" + assert store.relations.list_all(_ORG) == [] class TestSourceCollisionGuard: @@ -371,8 +482,11 @@ class TestSourceCollisionGuard: @pytest.fixture def guard_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource + """A store carrying one discriminated and one undiscriminated source. + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DemoSource, DiscriminatedSource])) def test_same_alias_rejected(self, guard_store: Store): @@ -422,6 +536,11 @@ class TestDerivedNames: @pytest.fixture def name_store(self, component_db: Engine) -> Store: + """A store carrying the discriminated source, whose name is derivable. + + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DiscriminatedSource])) def test_blank_name_defaults_to_instance_name(self, name_store: Store): @@ -455,13 +574,32 @@ def test_unresolvable_key_leaves_name_blank(self, store: Store): row = store.components.create(_ORG, kind="destination", key="ghost") assert row.name is None + def test_a_drifted_key_derives_no_name(self, component_db: Engine): + writer = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = writer.components.create(_ORG, kind="source", key="demo_source", config={}) + reader = Store(catalog=il.Catalog(components={})) + + assert reader.components._derived_name(row, row.config) is None + + def test_a_kind_mismatch_derives_no_name(self, component_db: Engine): + # The stored kind and the catalog class disagree, so nothing is derivable. + store = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = store.components.create(_ORG, kind="source", key="demo_source") + row.kind = "destination" + + assert store.components._derived_name(row, {}) is None + + def test_an_unconstructable_config_derives_no_name(self, component_db: Engine): + store = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = store.components.create(_ORG, kind="source", key="demo_source") + + assert store.components._derived_name(row, {"random_failure_probability": "not-a-float"}) is None + class TestTelemetry: """Hydration is traced where it happens.""" def test_load_emits_a_span_per_hydration(self, component_db: Engine, span_exporter): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") @@ -479,8 +617,6 @@ class TestQuotaGates: def _store(self, **limits: int | None) -> Store: from types import SimpleNamespace - from interloper_assets.demo.source import DemoSource - return Store(catalog=il.Catalog.from_assets([DemoSource]), quota_defaults=SimpleNamespace(**limits)) def test_source_limit_blocks_creation(self, component_db: Engine): @@ -532,6 +668,11 @@ class TestStatus: @pytest.fixture def demo_store(self, component_db: Engine) -> Store: + """A store carrying the demo source. + + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DemoSource])) def test_live_source_is_ok(self, demo_store: Store): @@ -720,22 +861,8 @@ def test_update_refreshes_updated_at(self, store: Store): assert updated.updated_at >= updated.created_at -class PublicToggleConnection(il.Connection): - """Renewable test connection whose ``auto_renew`` is schema-marked public.""" - - api_key: str = il.SecretField() - - def renew(self) -> il.Renewal: - """Keep the class renewable so ``auto_renew`` stays in its schema. - - Returns: - An effectless renewal. - """ - return il.Renewal() - - class TestPublicConfig: - """The x-public projection over a secret payload.""" + """The ``x-public`` subset a collection response may disclose.""" def test_public_subset_disclosed_from_encrypted_payload(self, component_db: Engine): catalog = il.Catalog(components={PublicToggleConnection.key: PublicToggleConnection.definition()}) @@ -749,12 +876,25 @@ def test_public_subset_disclosed_from_encrypted_payload(self, component_db: Engi assert store.components.public_config(row) == {"auto_renew": False} - def test_drifted_key_discloses_nothing(self, component_db: Engine): + def test_a_schema_without_public_fields_discloses_nothing(self, component_db: Engine): + store = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = store.components.create(_ORG, kind="source", key="demo_source") + + assert store.components.public_config(row) == {} + + def test_a_drifted_key_discloses_nothing(self, component_db: Engine): store = Store(catalog=il.Catalog(components={}), encrypt=lambda b: b[::-1], decrypt=lambda b: b[::-1]) row = store.components.create(_ORG, kind="connection", key="gone", config={"token": "s3cret"}) assert store.components.public_config(row) == {} + def test_a_drifted_source_key_discloses_nothing(self, component_db: Engine): + writer = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = writer.components.create(_ORG, kind="source", key="demo_source") + reader = Store(catalog=il.Catalog(components={})) + + assert reader.components.public_config(row) == {} + class TestHydrateUnreadable: """Hydrating an unreadable row fails as a decryption error, not as drift.""" @@ -835,9 +975,7 @@ def test_a_drifted_key_raises_component_drift(self, component_db: Engine): def test_an_unreadable_payload_raises_hydration_error(self, component_db: Engine): # A different remedy from drift: the config needs re-entering or re-keying. - catalog = il.Catalog( - components={FacebookAdsConnection.key: FacebookAdsConnection.definition()} - ) + catalog = il.Catalog(components={FacebookAdsConnection.key: FacebookAdsConnection.definition()}) writer = Store(catalog=catalog, encrypt=lambda b: b, decrypt=lambda b: b) row = writer.components.create( _ORG, @@ -866,70 +1004,17 @@ def test_an_asset_the_source_no_longer_declares_raises(self, component_db: Engin with pytest.raises(ComponentDriftError, match="is no longer declared by source"): store.components.load(orphan_id) + def test_a_child_the_class_dropped_drifts_the_whole_source(self, component_db: Engine): + # The dropped child's own relation rows are declared by the class it + # fell out of, so reading them would report this drift as an + # undeclared relation name on the child. + writer = Store(catalog=il.Catalog.from_assets([WireUpSource])) + source = writer.components.create(_ORG, kind="source", key="wire_up_source") + assert sorted(child.key for child in source.children) == ["rows", "totals"] + reader = Store(catalog=il.Catalog.from_assets([WireUpNarrowedSource])) -class TestPublicConfig: - """The ``x-public`` subset a collection response may disclose.""" - - def test_a_schema_without_public_fields_discloses_nothing(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = store.components.create(_ORG, kind="source", key="demo_source") - - assert store.components.public_config(row) == {} - - def test_a_drifted_key_discloses_nothing(self, component_db: Engine): - writer = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = writer.components.create(_ORG, kind="source", key="demo_source") - reader = Store(catalog=il.Catalog(components={})) - - assert reader.components.public_config(row) == {} - - -class TestDerivedName: - """The display name derived from a component's own config.""" - - def test_a_drifted_key_derives_no_name(self, component_db: Engine): - writer = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = writer.components.create(_ORG, kind="source", key="demo_source", config={}) - reader = Store(catalog=il.Catalog(components={})) - - assert reader.components._derived_name(row, row.config) is None - - def test_a_kind_mismatch_derives_no_name(self, component_db: Engine): - # The stored kind and the catalog class disagree, so nothing is derivable. - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = store.components.create(_ORG, kind="source", key="demo_source") - row.kind = "destination" - - assert store.components._derived_name(row, {}) is None - - def test_an_unconstructable_config_derives_no_name(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = store.components.create(_ORG, kind="source", key="demo_source") - - assert store.components._derived_name(row, {"random_failure_probability": "not-a-float"}) is None - - -class TestSourceCollision: - """Two instances of one source must not silently share a materialization target.""" - - def test_colliding_datasets_are_refused(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "shared"}) - - with pytest.raises(ConfigError): - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "shared"}) - - def test_distinct_datasets_are_allowed(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "one"}) - - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "two"}) - - def test_another_org_is_not_a_collision(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "shared"}) - - store.components.create(uuid4(), kind="source", key="demo_source", config={"dataset": "shared"}) + with pytest.raises(ComponentDriftError, match="Asset 'totals' .* is no longer declared by source"): + reader.components.load(source.id) class TestEnsureChildrenDrift: @@ -957,9 +1042,7 @@ def test_a_job_with_no_targets_has_none(self, component_db: Engine): def test_a_partitioned_source_target_reports_its_granularity(self, component_db: Engine): store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") - job = store.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(source.id, "")]} - ) + job = store.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [source.id]}) with Session(component_db) as session: assert store.components.job_partition_granularity(session, job.id) is TimeGranularity.DAY @@ -970,9 +1053,7 @@ def test_a_partitioned_asset_target_reports_its_granularity(self, component_db: with Session(component_db) as session: asset = session.exec(select(Component).where(Component.parent_id == source.id)).one() asset_id = asset.id - job = store.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(asset_id, "")]} - ) + job = store.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [asset_id]}) with Session(component_db) as session: assert store.components.job_partition_granularity(session, job.id) is TimeGranularity.DAY @@ -983,10 +1064,7 @@ def test_targets_disagreeing_on_granularity_fail_closed(self, component_db: Engi daily = store.components.create(_ORG, kind="source", key="demo_source") monthly = store.components.create(_ORG, kind="source", key="demo_monthly_source") job = store.components.create( - _ORG, - kind="job", - key="cron_job", - relations={"target": [(daily.id, ""), (monthly.id, "")]}, + _ORG, kind="job", key="cron_job", relations={"targets": [daily.id, monthly.id]} ) with Session(component_db) as session, pytest.raises( @@ -998,9 +1076,7 @@ def test_a_drifted_target_contributes_nothing(self, component_db: Engine): # Drift is the run path's problem, not the scheduler's. writer = Store(catalog=il.Catalog.from_assets([DemoSource])) source = writer.components.create(_ORG, kind="source", key="demo_source") - job = writer.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(source.id, "")]} - ) + job = writer.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [source.id]}) reader = Store(catalog=il.Catalog(components={})) with Session(component_db) as session: @@ -1013,9 +1089,7 @@ class TestCheckJobTargets: def test_a_drifted_target_raises(self, component_db: Engine): writer = Store(catalog=il.Catalog.from_assets([DemoSource])) source = writer.components.create(_ORG, kind="source", key="demo_source") - job = writer.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(source.id, "")]} - ) + job = writer.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [source.id]}) reader = Store(catalog=il.Catalog(components={})) with Session(component_db) as session: diff --git a/packages/interloper-db/tests/store/test_hydration.py b/packages/interloper-db/tests/store/test_hydration.py index dbfbf6cb..f852ef25 100644 --- a/packages/interloper-db/tests/store/test_hydration.py +++ b/packages/interloper-db/tests/store/test_hydration.py @@ -1,39 +1,282 @@ """Round-trip tests: generic store writes → generic hydration → live framework objects. -These exercise the full pipeline against a real (SQLite) database using the -real ``DemoSource`` catalog component: create rows through the generic store -surface, hydrate through the one generic spec builder, and assert on the -reconstructed framework instances. +These exercise the full pipeline against a real (SQLite) database using real +catalog classes: create rows through the generic store surface, hydrate +through the one generic spec builder, and assert on the reconstructed +framework instances. """ from __future__ import annotations -from typing import ClassVar -from uuid import uuid4 +from typing import Any +from uuid import UUID, uuid4 import interloper as il import pydantic import pytest -from interloper.errors import HydrationError +from interloper.errors import ComponentDriftError, HydrationError +from interloper.serializable import Spec from interloper_assets.demo.source import DemoSource, demo_asset from sqlalchemy import Engine +from sqlmodel import Session +from interloper_db.models import Component, ComponentRelation from interloper_db.store import Store +from interloper_db.store.hydration import Hydrator _ORG = uuid4() -_CATALOG = il.Catalog.from_assets([DemoSource, demo_asset]) + + +class ShopConnection(il.Connection): + """Connection the shop source binds.""" + + +class Warehouse(il.Destination): + """Destination the test sources write to.""" + + def read(self, context: Any) -> Any: # pragma: no cover + """Never read by these tests. + + Args: + context: The IO context of the read. + + Returns: + Nothing; the destination is a persistence fixture only. + """ + return None + + def write(self, context: Any, data: Any) -> None: # pragma: no cover + """Never written by these tests. + + Args: + context: The IO context of the write. + data: The payload that would be written. + """ + + +class Shop(il.Source): + """Upstream source: a bound connection and one asset other sources read.""" + + connection: ShopConnection + + class Orders(il.Asset): + """The asset the finance source reads by qualified key.""" + + back = il.Relation("asset", "finance.revenue", optional=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: # pragma: no cover + """Never materialized by these tests. + + Args: + context: The execution context of the materialization. + + Returns: + No rows. + """ + return [] + + +class Finance(il.Source): + """Downstream source whose assets both read ``shop.orders``.""" + + class Revenue(il.Asset): + """Asset with a cross-source upstream.""" + + orders = il.Relation("asset", "shop.orders") + + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> list[dict]: # pragma: no cover + """Never materialized by these tests. + + Args: + context: The execution context of the materialization. + orders: The upstream leg read from ``shop.orders``. + + Returns: + No rows. + """ + return [] + + class Cogs(il.Asset): + """A second asset reading the same cross-source upstream as ``Revenue``.""" + + orders = il.Relation("asset", "shop.orders") + + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> list[dict]: # pragma: no cover + """Never materialized by these tests. + + Args: + context: The execution context of the materialization. + orders: The upstream leg read from ``shop.orders``. + + Returns: + No rows. + """ + return [] + + +_CATALOG = il.Catalog.from_assets([DemoSource, demo_asset, Shop, Finance, Warehouse]) @pytest.fixture def store(component_db: Engine) -> Store: - """A store over the in-memory database with the demo catalog. + """A store over the in-memory database with the tests' catalog. Returns: - A store carrying the demo catalog, reading and writing the fixture database. + A store carrying the demo and shop/finance classes, reading and + writing the fixture database. """ return Store(catalog=_CATALOG) +def _child(source: Component, key: str) -> Component: + """Pick one child row of a source row by asset key. + + Args: + source: The source row whose children are searched. + key: The child asset's catalog key. + + Returns: + The matching child row. + """ + return next(child for child in source.children if child.key == key) + + +def _asset(source: il.Component, key: str) -> il.Asset: + """Pick one asset instance of a hydrated source by key. + + Args: + source: The hydrated source. + key: The asset's catalog key. + + Returns: + The matching asset instance. + """ + assert isinstance(source, il.Source) + return next(asset for asset in source.assets if asset.key == key) + + +def _init(store: Store, component_id: UUID) -> dict[str, Any]: + """Build the init payload of one row, in a session of its own. + + Args: + store: The store whose hydrator builds the payload. + component_id: Id of the row to build. + + Returns: + The row's init payload. + """ + with Session(store.engine) as session: + row = session.get(Component, component_id) + assert row is not None + return store.components._hydrator._build_init(session, row) + + +class TestBuildInit: + """The one rule that decides how a relation target is written out.""" + + def test_parentless_target_nests(self, store: Store): + connection = store.components.create(_ORG, kind="connection", key="shop_connection", config={}, encrypted=False) + warehouse = store.components.create(_ORG, kind="destination", key="warehouse") + shop = store.components.create( + _ORG, + kind="source", + key="shop", + relations={"connection": [connection.id], "destinations": [warehouse.id]}, + ) + + init = _init(store, shop.id) + + assert init["connection"]["id"] == str(connection.id) + assert init["connection"]["path"].endswith("ShopConnection") + assert [target["id"] for target in init["destinations"]] == [str(warehouse.id)] + + def test_asset_target_is_a_reference(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=_child(shop, "orders").id) + + init = _init(store, finance.id) + + assert init["assets"]["revenue"]["orders"] == Spec.reference(str(_child(shop, "orders").id)) + + def test_a_source_owned_asset_carries_its_sibling_relations(self, store: Store): + source = store.components.create(_ORG, kind="source", key="demo_source") + + init = _init(store, source.id) + + assert init["assets"]["e"]["b"] == Spec.reference(str(_child(source, "b").id)) + assert init["assets"]["b"]["a"] == Spec.reference(str(_child(source, "a").id)) + + def test_a_target_reached_twice_is_written_out_once_then_referenced(self, store: Store): + warehouse = store.components.create(_ORG, kind="destination", key="warehouse") + shop = store.components.create(_ORG, kind="source", key="shop", relations={"destinations": [warehouse.id]}) + finance = store.components.create( + _ORG, kind="source", key="finance", relations={"destinations": [warehouse.id]} + ) + job = store.components.create( + _ORG, + kind="job", + key="cron_job", + config={"cron": "0 6 * * *"}, + relations={"targets": [shop.id, finance.id]}, + ) + + init = _init(store, job.id) + + emitted = [target["init"]["destinations"][0] for target in init["targets"]] + assert [Spec.is_reference(value) for value in emitted] == [False, True] + assert emitted[0]["id"] == str(warehouse.id) + assert emitted[1] == Spec.reference(str(warehouse.id)) + + def test_a_relation_name_the_class_does_not_declare_is_an_actionable_error( + self, store: Store, component_db: Engine + ): + shop = store.components.create(_ORG, kind="source", key="shop") + warehouse = store.components.create(_ORG, kind="destination", key="warehouse") + with Session(component_db) as session: + session.add( + ComponentRelation( + src_id=shop.id, + name="mystery", + dst_id=warehouse.id, + org_id=_ORG, + src_kind="source", + dst_kind="destination", + ) + ) + session.commit() + + with pytest.raises(HydrationError, match="has 'mystery' relations its class does not declare"): + _init(store, shop.id) + + def test_two_rows_under_a_single_valued_relation_is_an_actionable_error( + self, store: Store, component_db: Engine + ): + shop = store.components.create(_ORG, kind="source", key="shop") + # Hand-inserted: the store's own writes repoint a single-valued name + # instead of accumulating, so only a rogue writer produces this row. + connections = [ + store.components.create(_ORG, kind="connection", key="shop_connection", config={}, encrypted=False) + for _ in range(2) + ] + with Session(component_db) as session: + for connection in connections: + session.add( + ComponentRelation( + src_id=shop.id, + name="connection", + dst_id=connection.id, + org_id=_ORG, + src_kind="source", + dst_kind="connection", + ) + ) + session.commit() + + with pytest.raises(HydrationError, match="holds 2 rows under single-valued relation 'connection'"): + _init(store, shop.id) + + class TestSourceRoundTrip: """Sources with child assets, intra-source deps, and overrides.""" @@ -42,11 +285,13 @@ def test_create_source_creates_children_and_intra_deps(self, store: Store): assert db_source.kind == "source" assert sorted(child.key for child in db_source.children) == ["a", "b", "c", "d", "e"] - deps = store.relations.list_all(_ORG, type="upstream") - by_child = {} + deps = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") + by_child: dict[str, set[tuple[str, str]]] = {} children_by_id = {child.id: child.key for child in db_source.children} - for rel in deps: - by_child.setdefault(children_by_id[rel.src_id], set()).add((rel.slot, children_by_id[rel.dst_id])) + for relation in deps: + by_child.setdefault(children_by_id[relation.src_id], set()).add( + (relation.name, children_by_id[relation.dst_id]) + ) assert by_child == { "b": {("a", "a")}, "c": {("a", "a")}, @@ -65,22 +310,19 @@ def test_load_hydrates_with_stable_ids_and_deps(self, store: Store): assert source.hello == "there" rows_by_key = {child.key: str(child.id) for child in db_source.children} - assets_by_key = {type(asset).key: asset for asset in source.assets} + assets_by_key = {asset.key: asset for asset in source.assets} assert {key: asset.id for key, asset in assets_by_key.items()} == rows_by_key - assert assets_by_key["e"].upstreams == { - "b": [rows_by_key["b"]], - "c": [rows_by_key["c"]], - "d": [rows_by_key["d"]], - } + e = assets_by_key["e"] + assert {name: getattr(e, name).id for name in ("b", "c", "d")} == {name: rows_by_key[name] for name in "bcd"} def test_source_owned_asset_loads_through_its_parent(self, store: Store): db_source = store.components.create(_ORG, kind="source", key="demo_source", name="Demo") - child = next(child for child in db_source.children if child.key == "a") + child = _child(db_source, "a") store.components.update(child.id, config={"materializable": False}) asset = store.components.load(child.id) assert isinstance(asset, il.Asset) - assert type(asset).key == "a" + assert asset.key == "a" assert asset.materializable is False def test_children_selection_drops_rows_and_relations(self, store: Store): @@ -90,8 +332,86 @@ def test_children_selection_drops_rows_and_relations(self, store: Store): refreshed = store.components.get(db_source.id, kind="source") assert sorted(child.key for child in refreshed.children) == ["a", "b"] # e (and its dependency relations) are gone; b keeps its dep on a. - remaining = store.relations.list_all(_ORG, type="upstream") - assert [rel.slot for rel in remaining] == ["a"] + remaining = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") + assert [relation.name for relation in remaining] == ["a"] + + +class TestCrossSourceUpstream: + """A reference no document carries is resolved through the store.""" + + def test_load_resolves_cross_source_upstream_through_store(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=shop_orders.id) + + hydrated = store.components.load(finance.id) + + orders = _asset(hydrated, "revenue").bound("orders") + assert isinstance(orders, il.Asset) + assert orders.id == str(shop_orders.id) + assert orders.parent is not None + assert orders.parent.key == "shop" + + def test_an_owned_asset_referencing_another_source_loads(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + revenue_row = _child(finance, "revenue") + store.relations.add(revenue_row.id, name="orders", dst_id=shop_orders.id) + + revenue = store.components.load(revenue_row.id) + + assert isinstance(revenue, il.Asset) + assert revenue.orders.id == str(shop_orders.id) # ty: ignore[unresolved-attribute] + + def test_a_shared_upstream_reached_twice_hydrates_once(self, store: Store, monkeypatch: pytest.MonkeyPatch): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=shop_orders.id) + store.relations.add(_child(finance, "cogs").id, name="orders", dst_id=shop_orders.id) + + original_build = Hydrator.build_component_spec + shop_hydrations: list[UUID] = [] + + def spy(self: Hydrator, session: Session, db_component: Component, *, seen: set[str] | None = None) -> Spec: + if db_component.key == "shop": + shop_hydrations.append(db_component.id) + return original_build(self, session, db_component, seen=seen) + + monkeypatch.setattr(Hydrator, "build_component_spec", spy) + + hydrated = store.components.load(finance.id) + + revenue_orders = _asset(hydrated, "revenue").bound("orders") + cogs_orders = _asset(hydrated, "cogs").bound("orders") + assert isinstance(revenue_orders, il.Asset) + assert isinstance(cogs_orders, il.Asset) + assert revenue_orders is cogs_orders + assert revenue_orders.parent is cogs_orders.parent + assert shop_hydrations == [shop.id] + + def test_a_reference_cycle_across_sources_raises_with_its_trail(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + revenue = _child(finance, "revenue") + store.relations.add(revenue.id, name="orders", dst_id=shop_orders.id) + store.relations.add(shop_orders.id, name="back", dst_id=revenue.id) + + with pytest.raises(HydrationError, match="Reference cycle while hydrating: shop"): + store.components.load(shop.id) + + def test_a_reference_to_a_drifted_component_raises_component_drift_error(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=_child(shop, "orders").id) + + reader = Store(catalog=il.Catalog.from_assets([DemoSource, demo_asset, Finance, Warehouse])) + + with pytest.raises(ComponentDriftError, match="shop"): + reader.components.load(finance.id) class TestStandaloneAsset: @@ -101,7 +421,7 @@ def test_create_and_load(self, store: Store): db_asset = store.components.create(_ORG, kind="asset", key="demo_asset", config={"materializable": False}) asset = store.components.load(db_asset.id) assert isinstance(asset, il.Asset) - assert type(asset).key == "demo_asset" + assert asset.key == "demo_asset" assert asset.id == str(db_asset.id) assert asset.materializable is False @@ -117,12 +437,12 @@ def test_create_and_read_back(self, store: Store): key="cron_job", name="Demo Daily", config={"cron": "0 6 * * *", "tags": ["daily"], "enabled": True}, - relations={"target": [(db_source.id, "")]}, + relations={"targets": [db_source.id]}, ) assert db_job.name == "Demo Daily" assert db_job.config == {"cron": "0 6 * * *", "tags": ["daily"], "enabled": True} assert db_job.state is None - assert [rel.dst_id for rel in db_job.out_relations] == [db_source.id] + assert [relation.dst_id for relation in db_job.out_relations] == [db_source.id] assert [job.id for job in store.components.list_all(_ORG, kinds=["job"])] == [db_job.id] @@ -135,23 +455,39 @@ def test_load_hydrates_targets(self, store: Store): key="cron_job", name="Demo Daily", config={"cron": "0 6 * * *"}, - relations={"target": [(db_source.id, ""), (db_asset.id, "")]}, + relations={"targets": [db_source.id, db_asset.id]}, ) job = store.components.load(db_job.id) assert isinstance(job, il.CronJob) assert job.cron == "0 6 * * *" - assert {type(target).key for target in job.targets} == {"demo_source", "demo_asset"} - assert {type(asset).key for asset in il.DAG(*job.targets).operations} == {"a", "b", "c", "d", "e", "demo_asset"} + assert {target.key for target in job.targets} == {"demo_source", "demo_asset"} + assert {asset.key for asset in il.DAG(*job.targets).operations} == {"a", "b", "c", "d", "e", "demo_asset"} + + def test_load_hydrates_a_target_that_is_a_source_owned_asset(self, store: Store): + db_source = store.components.create(_ORG, kind="source", key="demo_source", name="Demo") + owned_asset = _child(db_source, "a") + db_job = store.components.create( + _ORG, + kind="job", + key="cron_job", + name="Demo Daily", + config={"cron": "0 6 * * *"}, + relations={"targets": [owned_asset.id]}, + ) + + job = store.components.load(db_job.id) + assert isinstance(job, il.CronJob) + target = next(iter(job.targets)) + assert target.key == "a" + assert target.parent is not None + assert target.parent.key == "demo_source" def test_update_preserves_state_but_drops_the_cached_schedule(self, store: Store): db_job = store.components.create(_ORG, kind="job", key="cron_job", name="Job", config={"cron": "0 6 * * *"}) # Simulate the scheduler's targeted state write. - from sqlmodel import Session - from interloper_db.engine import get_engine - from interloper_db.models import Component with Session(get_engine()) as session: row = session.get(Component, db_job.id) @@ -168,29 +504,26 @@ def test_update_preserves_state_but_drops_the_cached_schedule(self, store: Store class FakeLinker(il.Component): """Test-only kind whose vocabulary the hydrator has never seen.""" - relation_types: ClassVar[dict[str, il.RelationDefinition]] = { - "link": il.RelationDefinition(kinds=["source"], field="links") - } - links: list[il.Component] = pydantic.Field(default_factory=list) + links: list[il.Component] = il.Relation("source", many=True, optional=True) il.KINDS.register(FakeLinker.kind, FakeLinker.anchor()) class TestOpenVocabulary: - """A novel kind + relation type persists and hydrates with no per-type code.""" + """A novel kind + relation name persists and hydrates with no per-kind code.""" - def test_custom_relation_type_round_trips(self, store: Store): + def test_a_custom_relation_name_round_trips(self, store: Store): store._catalog.components["fake_linker"] = FakeLinker.definition() db_source = store.components.create(_ORG, kind="source", key="demo_source", name="Demo") db_linker = store.components.create( - _ORG, kind="fake_linker", key="fake_linker", name="L", relations={"link": [(db_source.id, "")]} + _ORG, kind="fake_linker", key="fake_linker", name="L", relations={"links": [db_source.id]} ) linker = store.components.load(db_linker.id) assert isinstance(linker, FakeLinker) - assert [type(linked).key for linked in linker.links] == ["demo_source"] + assert [linked.key for linked in linker.links] == ["demo_source"] assert linker.links[0].id == str(db_source.id) @@ -203,7 +536,7 @@ def test_load_failure_message_omits_input_values(self, store: Store, monkeypatch class Probe(pydantic.BaseModel): app_secret: str - def raise_validation_error(spec): + def raise_validation_error(spec, catalog=None, *, resolve=None): Probe.model_validate({"token": "s3cret-value"}) monkeypatch.setattr(il.Component, "from_spec", raise_validation_error) @@ -226,9 +559,7 @@ def test_no_data_decodes_to_an_empty_dict(self, store: Store): def test_an_encrypted_row_without_a_cipher_is_an_actionable_error(self, component_db: Engine): writer = Store(catalog=_CATALOG, encrypt=lambda b: b, decrypt=lambda b: b) - row = writer.components.create( - _ORG, kind="connection", key="demo_connection", config={"token": "s3cret"} - ) + row = writer.components.create(_ORG, kind="connection", key="shop_connection", config={"token": "s3cret"}) row.data = row.data or b"payload" row.encrypted = True reader = Store(catalog=_CATALOG) @@ -237,14 +568,12 @@ def test_an_encrypted_row_without_a_cipher_is_an_actionable_error(self, componen reader.components._hydrator.decode_data(row) -class TestRelationsByType: +class TestRelationsByName: """The grouped relation lookup used while building a spec.""" def test_a_row_with_no_id_has_no_relations(self, store: Store): - from sqlmodel import Session - with Session(store.engine) as session: - assert store.components._hydrator._relations_by_type(session, None) == {} + assert store.components._hydrator._relations_by_name(session, None) == {} class TestResolvePath: @@ -252,7 +581,6 @@ class TestResolvePath: def test_a_drifted_key_is_an_actionable_error(self, component_db: Engine): from interloper.errors import CatalogKeyError - from sqlmodel import Session writer = Store(catalog=_CATALOG) row = writer.components.create(_ORG, kind="source", key="demo_source") diff --git a/packages/interloper-db/tests/store/test_organisations.py b/packages/interloper-db/tests/store/test_organisations.py index 0c50339e..debd80e4 100644 --- a/packages/interloper-db/tests/store/test_organisations.py +++ b/packages/interloper-db/tests/store/test_organisations.py @@ -34,7 +34,7 @@ def _seed_org_data(self, session: SQLSession, org_id) -> None: session.add(asset) session.add( ComponentRelation( - src_id=asset.id, dst_id=source.id, org_id=org_id, src_kind="asset", dst_kind="source", type="owner" + src_id=asset.id, dst_id=source.id, org_id=org_id, src_kind="asset", dst_kind="source", name="owner" ) ) backfill = Backfill(org_id=org_id, start_key="2026-01-01", end_key="2026-01-02") diff --git a/packages/interloper-db/tests/store/test_relations.py b/packages/interloper-db/tests/store/test_relations.py index 601ec0d1..9d544df9 100644 --- a/packages/interloper-db/tests/store/test_relations.py +++ b/packages/interloper-db/tests/store/test_relations.py @@ -2,14 +2,11 @@ from __future__ import annotations -from typing import Any, ClassVar -from uuid import uuid4 +from uuid import UUID, uuid4 import interloper as il import pytest from interloper.errors import ConfigError, NotFoundError -from interloper_assets.demo.source import DemoSource -from interloper_assets.facebook_ads.source import FacebookAds from sqlalchemy import Engine from sqlmodel import Session, select @@ -19,20 +16,10 @@ _ORG = uuid4() -@pytest.fixture -def store(component_db: Engine) -> Store: - """A store over the in-memory database (no catalog needed for these). - - Returns: - A store with an empty catalog, reading and writing the fixture database. - """ - return Store(catalog=il.Catalog(components={})) - - -def _relations(session: Session, src_id, type: str | None = None) -> list[ComponentRelation]: +def _relations(session: Session, src_id: UUID, name: str | None = None) -> list[ComponentRelation]: statement = select(ComponentRelation).where(ComponentRelation.src_id == src_id) - if type: - statement = statement.where(ComponentRelation.type == type) + if name: + statement = statement.where(ComponentRelation.name == name) return list(session.exec(statement).all()) @@ -40,292 +27,554 @@ def _child(source: Component, key: str) -> Component: return next(child for child in source.children if child.key == key) +class WireConnection(il.Connection): + """Connection the wire sources bind.""" + + class GuardUpstream(il.Asset): - """Upstream asset for the unbind-guard dependency tests.""" + """Upstream asset for the unbind-guard tests.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class GuardOther(il.Asset): + """Asset no guard relation declares a key for.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] class GuardRequired(il.Asset): - """Asset with a required dependency on ``guard_upstream``.""" + """Asset with a required upstream on ``guard_upstream``.""" + + up = il.Relation("asset", "guard_upstream") - depends_on: ClassVar[dict[str, Any]] = {"up": "guard_upstream"} + def data(self, context: il.ExecutionContext, up: il.Upstream) -> list[dict]: + return [] class GuardOptional(il.Asset): - """Asset with an optional dependency on ``guard_upstream``.""" + """Asset with an optional upstream on ``guard_upstream``.""" + + up = il.Relation("asset", "guard_upstream", optional=True) + + def data(self, context: il.ExecutionContext, up: il.Upstream | None) -> list[dict]: + return [] - depends_on: ClassVar[dict[str, Any]] = {"up": il.Dependency(key="guard_upstream", optional=True)} + +class Matcher(il.Asset): + """Asset fanning in the ``campaigns`` asset of every source.""" + + campaigns: list[il.Asset] = il.Relation("asset", "*.campaigns", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] class WireUpSource(il.Source): - """Upstream source for the cross-source dependency tests.""" + """Upstream source whose ``totals`` reads its sibling ``rows``.""" + + class Rows(il.Asset): + """Root asset of the source.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset reading the source's own ``rows``.""" + + rows = il.Relation("asset", "rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class WireOtherSource(il.Source): + """Another source declaring a ``rows`` asset, of a different source key.""" class Rows(il.Asset): - """Upstream asset (key ``rows``).""" + """Homonym of ``WireUpSource.Rows``, owned by another source.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] class WireDownSource(il.Source): - """Downstream source whose asset requires ``wire_up_source.rows``.""" + """Downstream source: a bound connection and a cross-source upstream.""" + + connection: WireConnection class Consumer(il.Asset): - """Asset with a required cross-source dependency.""" + """Asset reading ``wire_up_source.rows``.""" - depends_on: ClassVar[dict[str, Any]] = {"rows": "wire_up_source.rows"} + rows = il.Relation("asset", "wire_up_source.rows") + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] -class TestRelations: - """Replace semantics, vocabulary validation, denormalized stamping.""" - def test_sync_stamps_org_and_kinds(self, store: Store, component_db: Engine): - dest = store.components.create(_ORG, kind="destination", key="dest") - asset = store.components.create(_ORG, kind="asset", key="a", relations={"destination": [(dest.id, "")]}) +class FirstCampaignSource(il.Source): + """Source declaring a ``campaigns`` asset.""" - with Session(component_db) as session: - (relation,) = _relations(session, asset.id) - assert (relation.type, relation.slot, relation.dst_id) == ("destination", "", dest.id) - assert (relation.org_id, relation.src_kind, relation.dst_kind) == (_ORG, "asset", "destination") - - def test_update_replaces_only_the_given_type(self, store: Store, component_db: Engine): - dest = store.components.create(_ORG, kind="destination", key="dest") - first = store.components.create(_ORG, kind="connection", key="first", config={}, encrypted=False) - second = store.components.create(_ORG, kind="connection", key="second", config={}, encrypted=False) - asset = store.components.create( - _ORG, - kind="asset", - key="a", - relations={"destination": [(dest.id, "")], "resource": [(first.id, "conn")]}, - ) + class Campaigns(il.Asset): + """Campaign entities of the first provider.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class SecondCampaignSource(il.Source): + """Another source declaring a ``campaigns`` asset.""" + + class Campaigns(il.Asset): + """Campaign entities of the second provider.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class TwoKeySource(il.Source): + """Source naming two of its own assets on one single-valued relation.""" + + class First(il.Asset): + """One of the two candidate siblings.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Second(il.Asset): + """The other candidate sibling.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset declaring both sibling keys under one single-valued name.""" + + rows = il.Relation("asset", ["first", "second"]) + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class SelfKeySource(il.Source): + """Source whose only asset declares a bare key naming its own key.""" + + class Rows(il.Asset): + """Asset whose ``peers`` relation names ``rows``, which is its own key.""" + + peers: list[il.Asset] = il.Relation("asset", "rows", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class MixedKeySource(il.Source): + """Source mixing a bare sibling key and a qualified cross-source one on one relation.""" + + class Rows(il.Asset): + """The sibling the bare key names.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset fanning in its own source's ``rows`` and another source's.""" + + rows: list[il.Asset] = il.Relation("asset", ["rows", "wire_up_source.rows"], many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class SelfCampaignSource(il.Source): + """Source whose ``campaigns`` asset fans in every source's ``campaigns``, its own included.""" + + class Campaigns(il.Asset): + """Asset whose wildcard relation matches its own identity.""" + + peers: list[il.Asset] = il.Relation("asset", "*.campaigns", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class InstanceSource(il.Source): + """Discriminated source, so two of its instances coexist in one organisation.""" + + account_id: str = il.InputField(default="", discriminator=True) + + class Rows(il.Asset): + """Root asset of the instance.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset reading its own instance's ``rows``.""" + + rows = il.Relation("asset", "rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class ConsumerSource(il.Source): + """Source reading another source's ``rows`` by qualified key and by wildcard.""" + + class Qualified(il.Asset): + """Asset pinned to ``instance_source.rows``, whichever instance it comes from.""" + + rows: list[il.Asset] = il.Relation("asset", "instance_source.rows", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Wild(il.Asset): + """Asset fanning in the ``rows`` asset of any source.""" + + rows: list[il.Asset] = il.Relation("asset", "*.rows", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] - store.components.update(asset.id, relations={"resource": [(second.id, "conn")]}) + +@pytest.fixture +def store(component_db: Engine) -> Store: + """A store whose catalog carries every class the relation tests declare. + + Returns: + A store reading and writing the fixture database. + """ + catalog = il.Catalog.from_assets( + [ + GuardUpstream, + GuardOther, + GuardRequired, + GuardOptional, + Matcher, + WireUpSource, + WireOtherSource, + WireDownSource, + FirstCampaignSource, + SecondCampaignSource, + TwoKeySource, + SelfKeySource, + MixedKeySource, + SelfCampaignSource, + InstanceSource, + ConsumerSource, + ] + ) + return Store(catalog=catalog) + + +@pytest.fixture +def connection(store: Store) -> Component: + """A plaintext connection row the wire sources accept. + + Returns: + The created connection component. + """ + return store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + + +class TestAdd: + """``add`` binds by name, through the declared relation's acceptance rule.""" + + def test_single_valued_relation_repoints(self, store: Store, connection: Component, component_db: Engine): + other = store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + source = store.components.create(_ORG, kind="source", key="wire_down_source") + + store.relations.add(source.id, name="connection", dst_id=connection.id) + store.relations.add(source.id, name="connection", dst_id=other.id) with Session(component_db) as session: - assert [r.dst_id for r in _relations(session, asset.id, "resource")] == [second.id] - assert len(_relations(session, asset.id, "destination")) == 1 + assert [r.dst_id for r in _relations(session, source.id, "connection")] == [other.id] + + def test_many_valued_relation_accumulates(self, store: Store, component_db: Engine): + matcher = store.components.create(_ORG, kind="asset", key="matcher") + first = store.components.create(_ORG, kind="source", key="first_campaign_source") + second = store.components.create(_ORG, kind="source", key="second_campaign_source") + + store.relations.add(matcher.id, name="campaigns", dst_id=_child(first, "campaigns").id) + store.relations.add(matcher.id, name="campaigns", dst_id=_child(second, "campaigns").id) - def test_empty_list_clears_the_type(self, store: Store, component_db: Engine): - dest = store.components.create(_ORG, kind="destination", key="dest") - asset = store.components.create(_ORG, kind="asset", key="a", relations={"destination": [(dest.id, "")]}) - store.components.update(asset.id, relations={"destination": []}) with Session(component_db) as session: - assert _relations(session, asset.id) == [] + assert len(_relations(session, matcher.id, "campaigns")) == 2 + + def test_wildcard_key_refuses_a_parentless_asset(self, store: Store): + matcher = store.components.create(_ORG, kind="asset", key="matcher") + standalone = store.components.create(_ORG, kind="asset", key="campaigns") + + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(matcher.id, name="campaigns", dst_id=standalone.id) + + def test_rejects_a_kind_the_relation_does_not_declare(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + destination = store.components.create(_ORG, kind="destination", key="dest") + + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(source.id, name="connection", dst_id=destination.id) + + def test_rejects_an_undeclared_name(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + + with pytest.raises(ConfigError, match="declares no relation 'nope'"): + store.relations.add(source.id, name="nope", dst_id=connection.id) + + def test_rejects_an_undeclared_key(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_other") + required = store.components.create(_ORG, kind="asset", key="guard_required") + + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(required.id, name="up", dst_id=upstream.id) + + def test_checks_a_declared_key_against_the_parent_source(self, store: Store): + wire_up = store.components.create(_ORG, kind="source", key="wire_up_source") + other = store.components.create(_ORG, kind="source", key="wire_other_source") + down = store.components.create(_ORG, kind="source", key="wire_down_source", config={}) + consumer = _child(down, "consumer") + + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(consumer.id, name="rows", dst_id=_child(other, "rows").id) + + relation = store.relations.add(consumer.id, name="rows", dst_id=_child(wire_up, "rows").id) + assert relation.dst_id == _child(wire_up, "rows").id + + def test_identical_add_returns_the_existing_row(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + + first = store.relations.add(source.id, name="connection", dst_id=connection.id) + second = store.relations.add(source.id, name="connection", dst_id=connection.id) + + assert (second.src_id, second.name, second.dst_id) == (first.src_id, first.name, first.dst_id) + assert len(store.relations.list_all(_ORG, name="connection")) == 1 + + def test_stamps_the_denormalized_org_and_kinds(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + + relation = store.relations.add(source.id, name="connection", dst_id=connection.id) + + assert (relation.org_id, relation.src_kind, relation.dst_kind) == (_ORG, "source", "connection") + + def test_a_missing_source_raises(self, store: Store, connection: Component): + missing = uuid4() + + with pytest.raises(NotFoundError, match=f"Component {missing} not found"): + store.relations.add(missing, name="connection", dst_id=connection.id) + + def test_a_cross_org_target_raises(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + foreign = store.components.create(uuid4(), kind="connection", key="wire_connection", config={}, encrypted=False) - def test_rejects_types_outside_the_kind_vocabulary(self, store: Store): - dest = store.components.create(_ORG, kind="destination", key="dest") - with pytest.raises(ConfigError): - store.components.create(_ORG, kind="destination", key="d2", relations={"target": [(dest.id, "")]}) + with pytest.raises(NotFoundError, match=f"Component {foreign.id} not found"): + store.relations.add(source.id, name="connection", dst_id=foreign.id) - def test_rejects_missing_and_cross_org_destinations(self, store: Store): - other = store.components.create(uuid4(), kind="destination", key="dest") - with pytest.raises(NotFoundError): - store.components.create(_ORG, kind="asset", key="a", relations={"destination": [(uuid4(), "")]}) - with pytest.raises(NotFoundError): - store.components.create(_ORG, kind="asset", key="b", relations={"destination": [(other.id, "")]}) - def test_add_and_remove_relation(self, store: Store): - upstream = store.components.create(_ORG, kind="asset", key="a") - downstream = store.components.create(_ORG, kind="asset", key="b") +class TestRemove: + """``remove`` refuses to empty a non-optional relation.""" - relation = store.relations.add(downstream.id, type="upstream", dst_id=upstream.id, slot="a") - assert (relation.src_id, relation.dst_id, relation.slot) == (downstream.id, upstream.id, "a") - assert len(store.relations.list_all(_ORG, type="upstream")) == 1 + def test_last_row_of_a_required_relation_is_refused(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [upstream.id]}) + + with pytest.raises(ConfigError, match="non-optional"): + store.relations.remove(required.id, name="up", dst_id=upstream.id) + assert len(store.relations.list_all(_ORG, name="up")) == 1 + + def test_last_row_of_an_optional_relation_detaches(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + optional = store.components.create(_ORG, kind="asset", key="guard_optional", relations={"up": [upstream.id]}) + + store.relations.remove(optional.id, name="up", dst_id=upstream.id) + + assert store.relations.list_all(_ORG, name="up") == [] + + def test_an_absent_row_is_a_no_op(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required") + + store.relations.remove(required.id, name="up", dst_id=upstream.id) - store.relations.remove(downstream.id, type="upstream", dst_id=upstream.id) assert store.relations.list_all(_ORG) == [] -class TestRelationKindEnforcement: - """Relation writes are checked against the vocabulary's allowed kinds.""" +class TestListAll: + """``list_all`` filters by name and by either endpoint's kind.""" + + def test_filters_by_kinds(self, store: Store, connection: Component): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [upstream.id]}) + source = store.components.create(_ORG, kind="source", key="wire_down_source") + store.relations.add(source.id, name="connection", dst_id=connection.id) + + rows = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") + + assert {row.name for row in rows} == {"up"} + + def test_filters_by_name(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + store.relations.add(source.id, name="connection", dst_id=connection.id) + + assert len(store.relations.list_all(_ORG, name="connection")) == 1 + assert store.relations.list_all(_ORG, name="up") == [] - @pytest.fixture - def demo_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource, demo_asset + def test_is_scoped_to_the_organisation(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + store.relations.add(source.id, name="connection", dst_id=connection.id) - return Store(catalog=il.Catalog.from_assets([DemoSource, demo_asset])) + assert store.relations.list_all(uuid4()) == [] - def test_class_vocabulary_governs_writes(self, demo_store: Store): - db_job = demo_store.components.create(_ORG, kind="job", key="cron_job", name="J") - # TriggerHook declares `target`; WebhookHook does not. - ok = demo_store.components.create( - _ORG, kind="hook", key="trigger_hook", name="T", relations={"target": [(db_job.id, "")]} + +class TestSyncRelations: + """``_sync_relations`` replaces each listed name wholesale.""" + + def test_replaces_a_many_valued_relation_wholesale(self, store: Store, component_db: Engine): + first = store.components.create(_ORG, kind="source", key="first_campaign_source") + second = store.components.create(_ORG, kind="source", key="second_campaign_source") + matcher = store.components.create( + _ORG, + kind="asset", + key="matcher", + relations={"campaigns": [_child(first, "campaigns").id, _child(second, "campaigns").id]}, ) - assert ok.id is not None - with pytest.raises(ConfigError, match="'webhook_hook'.*declare no 'target' relations"): - demo_store.components.create( - _ORG, kind="hook", key="webhook_hook", name="W", relations={"target": [(db_job.id, "")]} - ) - def test_relation_to_disallowed_kind_rejected(self, demo_store: Store): - db_source = demo_store.components.create(_ORG, kind="source", key="demo_source", name="Demo") - db_job = demo_store.components.create(_ORG, kind="job", key="cron_job", name="Job") - # A job's 'target' may point at sources/assets — never at another job. - with pytest.raises(ConfigError, match="may not point at a 'job'"): - demo_store.components.create( - _ORG, kind="job", key="cron_job", name="Bad", relations={"target": [(db_job.id, "")]} - ) - # Sanity: the allowed kind passes. - ok = demo_store.components.create( - _ORG, kind="job", key="cron_job", name="Good", relations={"target": [(db_source.id, "")]} + store.components.update(matcher.id, relations={"campaigns": [_child(second, "campaigns").id]}) + + with Session(component_db) as session: + assert [r.dst_id for r in _relations(session, matcher.id, "campaigns")] == [_child(second, "campaigns").id] + + def test_leaves_the_names_it_is_not_given_alone(self, store: Store, connection: Component, component_db: Engine): + destination = store.components.create(_ORG, kind="destination", key="dest") + source = store.components.create( + _ORG, + kind="source", + key="wire_down_source", + relations={"connection": [connection.id], "destinations": [destination.id]}, ) - assert ok.id is not None + store.components.update(source.id, relations={"destinations": []}) -class TestDependencySlotValidation: - """Dependency writes are checked against the declared slots and their target identity.""" + with Session(component_db) as session: + assert len(_relations(session, source.id, "connection")) == 1 + assert _relations(session, source.id, "destinations") == [] - @pytest.fixture - def demo_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource + def test_refuses_to_empty_a_non_optional_relation(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [upstream.id]}) - return Store(catalog=il.Catalog.from_assets([DemoSource])) + with pytest.raises(ConfigError, match="non-optional"): + store.components.update(required.id, relations={"up": []}) - @pytest.fixture - def wire_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([WireUpSource, WireDownSource])) + def test_repointing_a_non_optional_relation_is_allowed(self, store: Store): + first = store.components.create(_ORG, kind="asset", key="guard_upstream") + second = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [first.id]}) - def test_undeclared_slot_rejected(self, demo_store: Store): - source = demo_store.components.create(_ORG, kind="source", key="demo_source") - with pytest.raises(ConfigError, match="declares no slot 'nope'"): - demo_store.relations.add( - _child(source, "b").id, type="upstream", dst_id=_child(source, "a").id, slot="nope" - ) + store.components.update(required.id, relations={"up": [second.id]}) - def test_wrong_target_key_rejected(self, demo_store: Store): - source = demo_store.components.create(_ORG, kind="source", key="demo_source") - with pytest.raises(ConfigError, match="expects asset 'demo_source.a', got 'e'"): - demo_store.relations.add( - _child(source, "b").id, type="upstream", dst_id=_child(source, "e").id, slot="a" - ) + (row,) = store.relations.list_all(_ORG, name="up") + assert row.dst_id == second.id + + def test_rejects_an_undeclared_name(self, store: Store, connection: Component): + with pytest.raises(ConfigError, match="declares no relation 'nope'"): + store.components.create(_ORG, kind="source", key="wire_down_source", relations={"nope": [connection.id]}) - def test_self_edge_rejected(self, store: Store): - asset = store.components.create(_ORG, kind="asset", key="a") - with pytest.raises(ConfigError, match="itself"): - store.relations.add(asset.id, type="upstream", dst_id=asset.id, slot="x") - - def test_cross_instance_sibling_rejected(self, demo_store: Store): - first = demo_store.components.create(_ORG, kind="source", key="demo_source") - second = demo_store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "other"}) - with pytest.raises(ConfigError, match="sibling asset of the same source instance"): - demo_store.relations.add( - _child(first, "b").id, type="upstream", dst_id=_child(second, "a").id, slot="a" + def test_rejects_several_targets_on_a_single_valued_relation(self, store: Store, connection: Component): + other = store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + + with pytest.raises(ConfigError, match="single-valued"): + store.components.create( + _ORG, + kind="source", + key="wire_down_source", + relations={"connection": [connection.id, other.id]}, ) - def test_cross_source_dep_accepts_any_instance(self, wire_store: Store): - up_two = wire_store.components.create(_ORG, kind="source", key="wire_up_source", config={"dataset": "two"}) - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source") - relation = wire_store.relations.add( - _child(down, "consumer").id, type="upstream", dst_id=_child(up_two, "rows").id, slot="rows" - ) - assert relation.dst_id == _child(up_two, "rows").id - def test_cross_source_dep_rejects_wrong_source(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource +class TestSelfEdge: + """No component fills its own relation, whatever the declared keys match.""" - store = Store(catalog=il.Catalog.from_assets([DemoSource, WireDownSource])) - demo = store.components.create(_ORG, kind="source", key="demo_source") - down = store.components.create(_ORG, kind="source", key="wire_down_source") - with pytest.raises(ConfigError, match="expects asset 'wire_up_source.rows'"): - store.relations.add( - _child(down, "consumer").id, type="upstream", dst_id=_child(demo, "a").id, slot="rows" - ) + def test_add_refuses_the_component_itself(self, store: Store): + source = store.components.create(_ORG, kind="source", key="self_campaign_source") + campaigns = _child(source, "campaigns") + with pytest.raises(ConfigError, match="cannot point at the component itself"): + store.relations.add(campaigns.id, name="peers", dst_id=campaigns.id) -class TestRelationUpsert: - """Slotted relation writes upsert per slot.""" + def test_a_replacement_refuses_the_component_itself(self, store: Store): + source = store.components.create(_ORG, kind="source", key="self_campaign_source") + campaigns = _child(source, "campaigns") - @pytest.fixture - def wire_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([WireUpSource, WireDownSource])) + with pytest.raises(ConfigError, match="cannot point at the component itself"): + store.components.update(campaigns.id, relations={"peers": [campaigns.id]}) - def test_rebinding_a_slot_repoints_it(self, wire_store: Store): - up_one = wire_store.components.create(_ORG, kind="source", key="wire_up_source") - up_two = wire_store.components.create(_ORG, kind="source", key="wire_up_source", config={"dataset": "two"}) - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source") - consumer = _child(down, "consumer") - wire_store.relations.add(consumer.id, type="upstream", dst_id=_child(up_one, "rows").id, slot="rows") - wire_store.relations.add(consumer.id, type="upstream", dst_id=_child(up_two, "rows").id, slot="rows") +class TestSiblingInstanceScope: + """A relation declaring only bare keys stays inside the owner's own source instance.""" + + def test_refuses_another_instances_asset(self, store: Store): + first = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "a"}) + second = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "b"}) - (edge,) = wire_store.relations.list_all(_ORG, type="upstream") - assert edge.dst_id == _child(up_two, "rows").id + with pytest.raises(ConfigError, match="belongs to another source instance"): + store.relations.add(_child(first, "totals").id, name="rows", dst_id=_child(second, "rows").id) - def test_identical_add_is_a_noop(self, wire_store: Store): - up = wire_store.components.create(_ORG, kind="source", key="wire_up_source") - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source") - consumer, rows = _child(down, "consumer"), _child(up, "rows") + def test_a_qualified_key_accepts_either_instance(self, store: Store, component_db: Engine): + first = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "a"}) + second = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "b"}) + qualified = _child(store.components.create(_ORG, kind="source", key="consumer_source"), "qualified") - first = wire_store.relations.add(consumer.id, type="upstream", dst_id=rows.id, slot="rows") - second = wire_store.relations.add(consumer.id, type="upstream", dst_id=rows.id, slot="rows") + store.relations.add(qualified.id, name="rows", dst_id=_child(first, "rows").id) + store.relations.add(qualified.id, name="rows", dst_id=_child(second, "rows").id) - assert (second.src_id, second.dst_id, second.slot) == (first.src_id, first.dst_id, first.slot) - assert len(wire_store.relations.list_all(_ORG, type="upstream")) == 1 + with Session(component_db) as session: + assert {row.dst_id for row in _relations(session, qualified.id, "rows")} == { + _child(first, "rows").id, + _child(second, "rows").id, + } + def test_a_wildcard_key_accepts_either_instance(self, store: Store, component_db: Engine): + first = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "a"}) + second = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "b"}) + wild = _child(store.components.create(_ORG, kind="source", key="consumer_source"), "wild") -class TestRequiredDependencyUnbindGuard: - """Bound required dependency slots refuse unbinding (repoint instead).""" + store.relations.add(wild.id, name="rows", dst_id=_child(first, "rows").id) + store.relations.add(wild.id, name="rows", dst_id=_child(second, "rows").id) - @pytest.fixture - def dep_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([GuardUpstream, GuardRequired, GuardOptional])) + with Session(component_db) as session: + assert {row.dst_id for row in _relations(session, wild.id, "rows")} == { + _child(first, "rows").id, + _child(second, "rows").id, + } - def test_remove_required_dependency_refused(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up.id, "up")]} - ) - with pytest.raises(ConfigError, match="cannot be unbound"): - dep_store.relations.remove(down.id, type="upstream", dst_id=up.id) - assert len(dep_store.relations.list_all(_ORG, type="upstream")) == 1 - - def test_remove_optional_dependency_allowed(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_optional", relations={"upstream": [(up.id, "up")]} - ) - dep_store.relations.remove(down.id, type="upstream", dst_id=up.id) - assert dep_store.relations.list_all(_ORG, type="upstream") == [] - def test_sync_clear_of_required_dependency_refused(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up.id, "up")]} - ) - with pytest.raises(ConfigError, match="cannot be unbound"): - dep_store.components.update(down.id, relations={"upstream": []}) - - def test_sync_repoint_of_required_dependency_allowed(self, dep_store: Store): - up_one = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - up_two = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up_one.id, "up")]} - ) - dep_store.components.update(down.id, relations={"upstream": [(up_two.id, "up")]}) - (edge,) = dep_store.relations.list_all(_ORG, type="upstream") - assert edge.dst_id == up_two.id +class TestIntraSourceWiring: + """A source's own sibling edges come from ``Source.sibling_bindings``.""" + def test_creating_a_source_wires_its_sibling_relations(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_up_source") -class TestAddValidation: - """``add`` vets the source, the vocabulary and the slot before writing.""" + (row,) = store.relations.list_all(_ORG, name="rows") + assert (row.src_id, row.dst_id) == (_child(source, "totals").id, _child(source, "rows").id) - def test_a_missing_source_raises(self, store: Store): - missing = uuid4() + def test_two_declared_keys_on_a_single_valued_relation_wire_one_edge(self, store: Store): + source = store.components.create(_ORG, kind="source", key="two_key_source") - with pytest.raises(NotFoundError, match=f"Component {missing} not found"): - store.relations.add(missing, type="resource", dst_id=uuid4(), slot="connection") + (row,) = store.relations.list_all(_ORG, name="rows") + sibling_key = TwoKeySource.sibling_bindings()["totals"]["rows"] + assert (row.src_id, row.dst_id) == (_child(source, "totals").id, _child(source, sibling_key).id) - def test_a_slot_on_an_unslotted_relation_is_refused(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - source = store.components.create(_ORG, kind="source", key="demo_source") - destination = store.components.create(_ORG, kind="destination", key="dest") + def test_a_key_naming_the_assets_own_key_wires_no_self_edge(self, store: Store): + store.components.create(_ORG, kind="source", key="self_key_source") - with pytest.raises(ConfigError, match="is not slotted"): - store.relations.add(source.id, type="destination", dst_id=destination.id, slot="nope") + assert SelfKeySource.sibling_bindings() == {} + assert store.relations.list_all(_ORG) == [] - def test_a_slot_expecting_another_key_is_refused(self, component_db: Engine): - # The slot declares the component key it accepts; anything else would - # be wired into a resource the source cannot use. - catalog = il.Catalog.from_assets([FacebookAds]) - store = Store(catalog=catalog, encrypt=lambda b: b, decrypt=lambda b: b) - source = store.components.create(_ORG, kind="source", key="facebook_ads") - wrong = store.components.create(_ORG, kind="destination", key="dest") + def test_a_relation_mixing_bare_and_qualified_keys_wires_the_bare_one(self, store: Store): + source = store.components.create(_ORG, kind="source", key="mixed_key_source") - with pytest.raises(ConfigError): - store.relations.add(source.id, type="resource", dst_id=wrong.id, slot="connection") + (row,) = store.relations.list_all(_ORG, name="rows") + assert (row.src_id, row.dst_id) == (_child(source, "totals").id, _child(source, "rows").id) diff --git a/packages/interloper-mcp/tests/test_tools.py b/packages/interloper-mcp/tests/test_tools.py index 8519fd47..52ec548c 100644 --- a/packages/interloper-mcp/tests/test_tools.py +++ b/packages/interloper-mcp/tests/test_tools.py @@ -40,7 +40,7 @@ async def test_only_read_only_tools_are_exposed(store: Store, catalog: il.Catalo names = {t.name for t in tools} assert len(names) == 20 - forbidden = {n for n in names if n.startswith(("trigger_", "toggle_", "create_", "request_"))} + forbidden = {n for n in names if n.startswith(("trigger_", "toggle_", "create_", "request_", "bind_", "unbind_"))} assert forbidden == set() assert {"list_jobs", "list_definitions", "get_full_lineage", "freshness_check"} <= names diff --git a/packages/interloper-scheduler/src/interloper_scheduler/executor.py b/packages/interloper-scheduler/src/interloper_scheduler/executor.py index f7dc972f..61970e8a 100644 --- a/packages/interloper-scheduler/src/interloper_scheduler/executor.py +++ b/packages/interloper-scheduler/src/interloper_scheduler/executor.py @@ -1,11 +1,12 @@ """Run executor: the envelope that assembles a run's operations and drives the runner. -The executor owns the run lifecycle — load, mark running, trace, terminal -status, failure event — and the platform side of graph assembly: flattening -the hydrated target workload into its operations, joining upstream -dependencies from the store as non-materializable context, and skipping the -retry lineage's prior successes. The runner executes the operations; their -returned effects (config and state fields) are applied generically to each +The executor owns the run lifecycle: load, mark running, trace, terminal +status, failure event, and skipping the retry lineage's prior successes. +Flattening the hydrated target workload into its operations and joining +bound upstreams the run itself does not materialize are the framework's own +concern (``Workload.operations()``, ``DAG._include_read_only_upstreams()``), +not the executor's. The runner executes the operations; their returned +effects (config and state fields) are applied generically to each operation's component row after the run. """ @@ -14,7 +15,7 @@ import asyncio import datetime as dt import logging -from typing import Any, cast +from typing import Any from uuid import UUID import interloper as il @@ -117,7 +118,6 @@ def execute(self, run_id: UUID) -> bool: self._store.runs.complete(run_id, success=True) return True - self._resolve_upstream(operations) if retry_of: successes = self._prior_successes(retry_of) for operation in operations: @@ -161,33 +161,6 @@ def _mark_running(session: Session, db_run: Run) -> None: session.add(db_run) session.commit() - def _resolve_upstream(self, operations: list[il.Operation]) -> None: - """Add transitive upstream dependencies to *operations* as non-materializable. - - Platform-side graph assembly: hydrated nodes carry their dependencies - as row ids, so the walk loads each unseen id from the store and - follows the dependencies it declares in turn. Joined upstream nodes - are read from their destinations, never recomputed. - - Args: - operations: The nodes to walk from, extended in place. - """ - visited = {operation.id for operation in operations} - frontier = list(operations) - while frontier: - next_frontier: list[il.Operation] = [] - for operation in frontier: - for upstream_ids in operation.upstreams.values(): - for dependency_id in upstream_ids: - if dependency_id in visited: - continue - visited.add(dependency_id) - upstream = cast(il.Asset, self._store.components.load(UUID(dependency_id))) - upstream.materializable = False - operations.append(upstream) - next_frontier.append(upstream) - frontier = next_frontier - def _prior_successes(self, retry_of: UUID) -> set[UUID]: """Node row ids that already succeeded in the retry lineage. diff --git a/packages/interloper-scheduler/src/interloper_scheduler/hooks.py b/packages/interloper-scheduler/src/interloper_scheduler/hooks.py index a764e804..0e252fb3 100644 --- a/packages/interloper-scheduler/src/interloper_scheduler/hooks.py +++ b/packages/interloper-scheduler/src/interloper_scheduler/hooks.py @@ -188,7 +188,7 @@ def _matching_hooks(self, session: Session, run: Run, target: Component) -> list .join(ComponentRelation, onclause=ComponentRelation.src_id == Component.id) # ty: ignore[invalid-argument-type] .where(Component.kind == "hook") .where(Component.org_id == run.org_id) - .where(ComponentRelation.type == "watch") + .where(ComponentRelation.name == "watches") .where(col(ComponentRelation.dst_id).in_(watched_ids)) .distinct() ).all() diff --git a/packages/interloper-scheduler/tests/test_cron.py b/packages/interloper-scheduler/tests/test_cron.py index 1c410abd..fff43652 100644 --- a/packages/interloper-scheduler/tests/test_cron.py +++ b/packages/interloper-scheduler/tests/test_cron.py @@ -269,7 +269,7 @@ def _job_targeting(store: Store, *source_keys: str, config: dict[str, Any]) -> U key="cron_job", name="J", config=config, - relations={"target": [(tid, "") for tid in targets]}, + relations={"targets": targets}, ) with Session(store.engine) as session: db_job = session.get(Component, row.id) diff --git a/packages/interloper-scheduler/tests/test_executor.py b/packages/interloper-scheduler/tests/test_executor.py index b6692484..61f4deca 100644 --- a/packages/interloper-scheduler/tests/test_executor.py +++ b/packages/interloper-scheduler/tests/test_executor.py @@ -414,91 +414,71 @@ def test_empty_effects_write_nothing(self, hydrated_asset: il.Asset) -> None: assert store.stamped == [] -class TestResolveUpstream: - """Upstream dependencies join the graph as read-only context.""" +class _UpstreamFixture(il.Asset): + """Plain asset fixture standing in for a hydrated upstream.""" - @staticmethod - def _asset(dependencies: dict[str, str] | None = None) -> il.Asset: - @il.asset() - def node() -> list[dict[str, Any]]: - return [{"x": 1}] - - instance = node(id=str(uuid4()), destinations=[il.MemoryDestination()]) - if dependencies: - instance.dependencies = dependencies - return instance + def data(self) -> list[dict[str, Any]]: + """Return one row. - def test_a_dependency_is_loaded_and_made_non_materializable(self) -> None: - # Joined upstream nodes are read from their destinations, never recomputed. - upstream = self._asset() - target = self._asset({"up": upstream.id}) - store = _RecordingStore(None) - store.components = SimpleNamespace(load=lambda _id: upstream) - executor = _executor(store) - operations: list[il.Operation] = [target] + Returns: + One row. + """ + return [{"x": 1}] - executor._resolve_upstream(operations) - assert operations == [target, upstream] - assert upstream.materializable is False +class _DownstreamFixture(il.Asset): + """Asset fixture whose optional relation accepts any asset.""" - def test_the_walk_is_transitive(self) -> None: - grandparent = self._asset() - parent = self._asset({"up": grandparent.id}) - target = self._asset({"up": parent.id}) - by_id = {parent.id: parent, grandparent.id: grandparent} - store = _RecordingStore(None) - store.components = SimpleNamespace(load=lambda component_id: by_id[str(component_id)]) - executor = _executor(store) - operations: list[il.Operation] = [target] + upstream: il.Asset | None = il.Relation("asset", optional=True) - executor._resolve_upstream(operations) + def data(self) -> list[dict[str, Any]]: + """Return one row. - assert [operation.id for operation in operations] == [target.id, parent.id, grandparent.id] + Returns: + One row. + """ + return [{"y": 1}] - def test_a_shared_dependency_is_loaded_once(self) -> None: - shared = self._asset() - first = self._asset({"up": shared.id}) - second = self._asset({"up": shared.id}) - loads: list[str] = [] - store = _RecordingStore(None) - def load(component_id: Any) -> il.Asset: - loads.append(str(component_id)) - return shared +class TestUpstreamJoinsReadOnly: + """A bound upstream the run itself does not materialize joins the DAG read-only. - store.components = SimpleNamespace(load=load) - executor = _executor(store) - operations: list[il.Operation] = [first, second] + The hydrator (Task 4) binds the upstream on the hydrated component + directly; the DAG (phase 1 Task 6) is what joins it as a read-only node. + The executor no longer walks anything itself, so this exercises the real + ``RunExecutor.execute`` path, capturing the assembled DAG through + ``_run_dag`` to inspect what it built. + """ - executor._resolve_upstream(operations) + def test_a_bound_upstream_is_joined_and_made_non_materializable( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + il.MemoryDestination.clear() + upstream = _UpstreamFixture(id=str(uuid4()), destinations=[il.MemoryDestination()]) + target = _DownstreamFixture( + id=str(uuid4()), destinations=[il.MemoryDestination()], upstream=upstream + ) - assert loads == [shared.id] + run = Run(id=uuid4(), component_id=uuid4(), org_id=uuid4(), status="dispatched") + monkeypatch.setattr(executor_module, "Session", lambda _engine: _FakeSession(run)) - def test_a_dependency_already_in_the_graph_is_not_reloaded(self) -> None: - upstream = self._asset() - target = self._asset({"up": upstream.id}) - store = _RecordingStore(None) - store.components = SimpleNamespace( - load=lambda _id: pytest.fail("an in-graph dependency must not be reloaded") - ) - executor = _executor(store) - operations: list[il.Operation] = [target, upstream] + built: list[il.DAG] = [] + real_run_dag = RunExecutor._run_dag - executor._resolve_upstream(operations) + def _capturing_run_dag(self: RunExecutor, dag: il.DAG, *args: Any, **kwargs: Any) -> il.RunResult: + built.append(dag) + return real_run_dag(self, dag, *args, **kwargs) - assert len(operations) == 2 + monkeypatch.setattr(RunExecutor, "_run_dag", _capturing_run_dag) - def test_no_dependencies_is_a_no_op(self) -> None: - target = self._asset() - store = _RecordingStore(None) - store.components = SimpleNamespace(load=lambda _id: pytest.fail("nothing to load")) + store = _RecordingStore(target) executor = _executor(store) - operations: list[il.Operation] = [target] - executor._resolve_upstream(operations) + assert executor.execute(run.id) is True - assert operations == [target] + (dag,) = built + assert dag.operation_map[upstream.id].materializable is False + assert dag.predecessors[target.id] == [upstream.id] class TestRetrySkipsPriorSuccesses: diff --git a/packages/interloper-scheduler/tests/test_hooks.py b/packages/interloper-scheduler/tests/test_hooks.py index 5c989680..79dbee06 100644 --- a/packages/interloper-scheduler/tests/test_hooks.py +++ b/packages/interloper-scheduler/tests/test_hooks.py @@ -129,7 +129,7 @@ def test_trigger_hook_cascades_with_partition(self, store: Store): hook = store.components.create( _ORG, kind="hook", key="trigger_hook", name="Cascade", config={"events": ["run_completed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) run = _terminal_run(store, leaf.id) @@ -155,7 +155,7 @@ def test_claim_prevents_refiring(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="Cascade", config={"events": ["run_completed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) _terminal_run(store, leaf.id) @@ -174,7 +174,7 @@ def test_event_type_mismatch_does_not_fire(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="OnFailureOnly", config={"events": ["run_failed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) _terminal_run(store, leaf.id, status="success") @@ -200,7 +200,7 @@ def fake_post(url: str, **kwargs: Any) -> httpx.Response: store.components.create( _ORG, kind="hook", key="webhook_hook", name="OnAnyAsset", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) _terminal_run(store, child.id) @@ -220,7 +220,7 @@ def boom(*args: Any, **kwargs: Any) -> None: store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_failed"], "url": "https://example.test/x"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) run = _terminal_run(store, source.id, status="failed") @@ -240,7 +240,7 @@ def test_self_targeting_trigger_is_refused(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="Ouroboros", config={"events": ["run_completed"]}, - relations={"watch": [(source.id, "")], "target": [(source.id, "")]}, + relations={"watches": [source.id], "targets": [source.id]}, ) _terminal_run(store, source.id) @@ -259,7 +259,7 @@ def test_metadata_carries_component_identity(self, store: Store, monkeypatch: py store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) _terminal_run(store, source.id) @@ -278,7 +278,7 @@ def test_metadata_falls_back_to_key_when_unnamed(self, store: Store, monkeypatch store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) # components.create always derives a name, so the nullable column is # the only way the fallback is reachable. @@ -301,7 +301,7 @@ def test_failure_metadata_carries_the_run_error(self, store: Store, monkeypatch: store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_failed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) run = _terminal_run(store, source.id, status="failed") # The error text lives on the run's event rows, not the run itself. @@ -318,7 +318,7 @@ def test_success_metadata_has_no_error(self, store: Store, monkeypatch: pytest.M store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) _terminal_run(store, source.id) @@ -336,7 +336,7 @@ def test_chain_to_unwatched_target_is_allowed(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="Chain", config={"events": ["run_completed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) _terminal_run(store, leaf.id) diff --git a/packages/interloper-slack/tests/test_hook.py b/packages/interloper-slack/tests/test_hook.py index 0eac7621..65edc99e 100644 --- a/packages/interloper-slack/tests/test_hook.py +++ b/packages/interloper-slack/tests/test_hook.py @@ -96,8 +96,8 @@ def test_http_error_propagates(self, slack): class TestDefinition: - def test_declares_a_connection_slot(self): - assert SlackHook.resource_types["connection"] is SlackConnection + def test_declares_a_connection_relation(self): + assert SlackHook.relations["connection"].target is SlackConnection def test_defaults_to_failures_only(self): assert _hook().events == ["run_failed"] @@ -105,4 +105,4 @@ def test_defaults_to_failures_only(self): def test_is_a_catalogued_hook(self): definition = SlackHook.definition() assert (definition.kind, definition.key, definition.name) == ("hook", "slack_hook", "Slack") - assert definition.relations["resource"].slots["connection"].key == "slack_connection" + assert definition.relations["connection"].key == "slack_connection" diff --git a/packages/interloper-toolkit/src/interloper_toolkit/__init__.py b/packages/interloper-toolkit/src/interloper_toolkit/__init__.py index 399a2953..e747a16b 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/__init__.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/__init__.py @@ -1,4 +1,4 @@ -"""Read-only tool functions shared by AI surfaces (agent, MCP server). +"""Tool functions shared by AI surfaces (agent, MCP server). Every function takes a :class:`~interloper_toolkit.context.ToolkitContext` as its first argument and returns `` | ToolError`` — typed @@ -6,9 +6,18 @@ the literal ``status`` field, never raising. The docstrings are LLM-facing: both the ADK agent and the MCP server surface them verbatim as tool descriptions. + +Almost every function here is read-only; the sole exceptions are +:func:`interloper_toolkit.collection.bind_relation` and +:func:`interloper_toolkit.collection.unbind_relation`, which write and are +re-exported here as this package's whole write surface. A surface that must +stay read-only (the MCP server's own registration is one) never registers +those two; the ADK agent, whose own write tools already live beside them, +does. """ +from interloper_toolkit.collection import bind_relation, unbind_relation from interloper_toolkit.context import ToolkitContext, serialize from interloper_toolkit.models import ToolError -__all__ = ["ToolError", "ToolkitContext", "serialize"] +__all__ = ["ToolError", "ToolkitContext", "bind_relation", "serialize", "unbind_relation"] diff --git a/packages/interloper-toolkit/src/interloper_toolkit/catalog.py b/packages/interloper-toolkit/src/interloper_toolkit/catalog.py index ea271a11..0c398a13 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/catalog.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/catalog.py @@ -122,7 +122,7 @@ def list_definitions( def get_definition(ctx: ToolkitContext, key: str) -> DefinitionDetail | ToolError: """Get a component definition's full catalog detail. - For a source this includes the config schema, resource slots, + For a source this includes the config schema, declared relations, destination types, and all its assets with their schemas. This is the catalog definition (the component *type*), not an instance from the org's collection. diff --git a/packages/interloper-toolkit/src/interloper_toolkit/collection.py b/packages/interloper-toolkit/src/interloper_toolkit/collection.py index 83319c8b..8945cdde 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/collection.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/collection.py @@ -1,15 +1,30 @@ -"""Collection tools — the org's component instances, read-only. +"""Collection tools: the org's component instances. -Creation and connection operations stay with the agent; this module is -shared with surfaces that must stay read-only. +``list_components`` is read-only, shared with surfaces that must stay +read-only (kind-specific creation and connection operations stay with the +agent). ``bind_relation`` and ``unbind_relation`` are the exception: they +write, generically over every kind's declared relations, so a caller must +never register them alongside a read-only tool set (see +``interloper_mcp.tools``, deliberately read-only), only wherever that +surface's own write tools already live. """ from __future__ import annotations +from uuid import UUID + from interloper.component import KINDS +from interloper.errors import ConfigError, NotFoundError from interloper_toolkit.context import ToolkitContext -from interloper_toolkit.models import ComponentCounts, ComponentList, ComponentSummary, ToolError +from interloper_toolkit.models import ( + BindResult, + ComponentCounts, + ComponentList, + ComponentSummary, + ToolError, + UnbindResult, +) def list_components( @@ -58,3 +73,48 @@ def list_components( return ComponentList(kind=kind, count=len(results), components=results) except Exception as e: return ToolError(error=str(e)) + + +# -- Relations (write) --------------------------------------------------------- + + +def bind_relation(ctx: ToolkitContext, component_id: str, name: str, dst_id: str) -> BindResult | ToolError: + """Bind one component to another under a declared relation name. + + Works on any kind: a source's connection, a job's watched assets, a + destination target, whatever the component's own class declares under + that name. A ``many`` name accumulates; a single-valued one repoints, so + rebinding it needs no prior unbind_relation call. Recap what the binding + changes and get the user's explicit confirmation BEFORE calling this. + + Args: + component_id: UUID of the component the relation originates from. + name: Relation name, which the component's class must declare. + dst_id: UUID of the destination component the relation points at. + Must belong to the same organisation as the source. + """ + try: + row = ctx.store.relations.add(UUID(component_id), name=name, dst_id=UUID(dst_id)) + except (ConfigError, NotFoundError, ValueError) as e: + return ToolError(error=str(e)) + return BindResult(src_id=str(row.src_id), name=row.name, dst_id=str(row.dst_id), dst_kind=row.dst_kind) + + +def unbind_relation(ctx: ToolkitContext, component_id: str, name: str, dst_id: str) -> UnbindResult | ToolError: + """Detach one component from another under a declared relation name. + + A non-optional relation cannot be emptied, only repointed with + bind_relation. Recap what the component loses and get the user's + explicit confirmation BEFORE calling this. + + Args: + component_id: UUID of the component the relation originates from. + name: Relation name the edge is filed under. + dst_id: UUID of the destination the removed edge points at. Removing + an edge that isn't there is a no-op. + """ + try: + ctx.store.relations.remove(UUID(component_id), name=name, dst_id=UUID(dst_id)) + except (ConfigError, NotFoundError, ValueError) as e: + return ToolError(error=str(e)) + return UnbindResult(src_id=component_id, name=name, dst_id=dst_id) diff --git a/packages/interloper-toolkit/src/interloper_toolkit/lineage.py b/packages/interloper-toolkit/src/interloper_toolkit/lineage.py index 6a6305c5..c4626bc6 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/lineage.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/lineage.py @@ -10,11 +10,11 @@ AssetRef, CrossSourceDependencies, CrossSourceEdge, - DependencyEdge, DownstreamResult, ImpactAnalysis, LineageItem, LineageResult, + RelationEdge, ToolError, UpstreamResult, ) @@ -27,19 +27,19 @@ def get_upstream(ctx: ToolkitContext, asset_id: str) -> UpstreamResult | ToolErr asset_id: UUID of the asset to inspect. Returns the list of upstream assets that this asset depends on, - including the parameter name used for each dependency. + including the relation name used for each dependency. """ try: - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") target = UUID(asset_id) upstream = [] for dep in deps: if dep.src_id == target: asset = ctx.store.components.get(dep.dst_id, kind="asset") - upstream.append(DependencyEdge( + upstream.append(RelationEdge( asset_id=str(dep.dst_id), - param_name=dep.slot, + param_name=dep.name, asset_key=asset.key, source_id=str(asset.parent_id), )) @@ -58,16 +58,16 @@ def get_downstream(ctx: ToolkitContext, asset_id: str) -> DownstreamResult | Too Returns the list of assets that directly depend on this asset. """ try: - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") target = UUID(asset_id) downstream = [] for dep in deps: if dep.dst_id == target: asset = ctx.store.components.get(dep.src_id, kind="asset") - downstream.append(DependencyEdge( + downstream.append(RelationEdge( asset_id=str(dep.src_id), - param_name=dep.slot, + param_name=dep.name, asset_key=asset.key, source_id=str(asset.parent_id), )) @@ -165,7 +165,7 @@ def cross_source_dependencies(ctx: ToolkitContext) -> CrossSourceDependencies | different sources. """ try: - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") assets = ctx.store.components.list_all(ctx.org_id, kinds=["asset"]) asset_source: dict[UUID, UUID | None] = {} @@ -186,7 +186,7 @@ def cross_source_dependencies(ctx: ToolkitContext) -> CrossSourceDependencies | downstream=asset_info.get(dep.src_id, AssetRef()), upstream_asset_id=str(dep.dst_id), upstream=asset_info.get(dep.dst_id, AssetRef()), - param_name=dep.slot, + param_name=dep.name, )) return CrossSourceDependencies(cross_source_count=len(cross_deps), dependencies=cross_deps) @@ -201,7 +201,7 @@ def _build_adjacency( ctx: ToolkitContext, direction: str, ) -> tuple[dict[UUID, list[UUID]], dict[UUID, dict[str, str]]]: - """Build an adjacency map and asset info lookup from all dependencies. + """Build an adjacency map and asset info lookup from all asset-to-asset relations. Args: ctx: The toolkit context. @@ -211,7 +211,7 @@ def _build_adjacency( ``(adjacency_map, asset_info_map)`` — info values feed :class:`LineageItem` kwargs (asset_key, source_id, source_key). """ - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") assets = ctx.store.components.list_all(ctx.org_id, kinds=["asset"]) asset_info: dict[UUID, dict[str, str]] = {} diff --git a/packages/interloper-toolkit/src/interloper_toolkit/models.py b/packages/interloper-toolkit/src/interloper_toolkit/models.py index 2c95b8bc..8c532681 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/models.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/models.py @@ -180,11 +180,30 @@ class ComponentList(BaseModel): components: list[ComponentSummary] +class BindResult(BaseModel): + """One relation edge created or repointed by ``bind_relation``.""" + + status: Literal["success"] = "success" + src_id: str + name: str + dst_id: str + dst_kind: str + + +class UnbindResult(BaseModel): + """One relation edge removed by ``unbind_relation``.""" + + status: Literal["success"] = "success" + src_id: str + name: str + dst_id: str + + # -- Lineage -------------------------------------------------------------------- -class DependencyEdge(BaseModel): - """A direct dependency edge from the perspective of one asset.""" +class RelationEdge(BaseModel): + """A direct asset-to-asset relation edge from the perspective of one asset.""" asset_id: str param_name: str @@ -197,7 +216,7 @@ class UpstreamResult(BaseModel): status: Literal["success"] = "success" asset_id: str - upstream: list[DependencyEdge] + upstream: list[RelationEdge] class DownstreamResult(BaseModel): @@ -205,7 +224,7 @@ class DownstreamResult(BaseModel): status: Literal["success"] = "success" asset_id: str - downstream: list[DependencyEdge] + downstream: list[RelationEdge] class LineageItem(BaseModel): diff --git a/packages/interloper-toolkit/tests/test_toolkit.py b/packages/interloper-toolkit/tests/test_toolkit.py index 005a5be4..b4fc3b69 100644 --- a/packages/interloper-toolkit/tests/test_toolkit.py +++ b/packages/interloper-toolkit/tests/test_toolkit.py @@ -1,8 +1,10 @@ -"""Tests for the shared read-only toolkit (``interloper_toolkit``). +"""Tests for the shared toolkit (``interloper_toolkit``). A real Store over in-memory SQLite plus a hand-built dumped catalog; the properties under test are the structured ``status`` contract, org scoping, -and the pure logic (lineage traversal, coverage math, schema search). +and the pure logic (lineage traversal, coverage math, schema search), plus +the two write tools (``bind_relation``, ``unbind_relation``) against a +catalog that declares real relations. """ from __future__ import annotations @@ -16,18 +18,49 @@ import interloper as il import pytest from interloper_db import engine as engine_module -from interloper_db.models import Backfill, Component, ComponentRelation, Run +from interloper_db.models import Backfill, Component, ComponentRelation, Quota, Run from interloper_db.store import Store from sqlalchemy import Engine, event from sqlalchemy.pool import StaticPool from sqlmodel import Session -from interloper_toolkit import ToolkitContext, analytics, lineage, scheduling +from interloper_toolkit import ToolkitContext, analytics, collection, lineage, scheduling from interloper_toolkit import catalog as catalog_tools ORG_ID = uuid4() OTHER_ORG_ID = uuid4() + +class DemoConnection(il.Connection): + """Connection the lineage-by-name test sources bind.""" + + +class ShopSource(il.Source): + """Source owning the asset a cross-source relation points at by name.""" + + connection: DemoConnection + + class Orders(il.Asset): + """Order rows another source's asset depends on.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class FinanceSource(il.Source): + """Source whose report asset depends on shop_source's orders by name.""" + + connection: DemoConnection + + class Revenue(il.Asset): + """Revenue rows, computed from shop_source's orders.""" + + orders = il.Relation("asset", "shop_source.orders") + + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> list[dict]: + return [] + + CATALOG_DUMP: dict[str, Any] = { "facebook_ads": { "kind": "source", @@ -72,7 +105,7 @@ @pytest.fixture def toolkit_db() -> Iterator[Engine]: - """A fresh in-memory database with component, relation, and run tables. + """A fresh in-memory database with component, relation, run, and quota tables. Yields: The engine bound to that database, disposed once the test finishes. @@ -89,7 +122,7 @@ def _configure_connection(dbapi_connection: Any, _record: Any) -> None: dbapi_connection.execute("PRAGMA foreign_keys=ON") dbapi_connection.create_function("gen_random_uuid", 0, lambda: uuid4().hex) - for model in (Component, ComponentRelation, Backfill, Run): + for model in (Component, ComponentRelation, Backfill, Run, Quota): model.__table__.create(eng) # ty: ignore[unresolved-attribute] try: yield eng @@ -99,8 +132,18 @@ def _configure_connection(dbapi_connection: Any, _record: Any) -> None: @pytest.fixture -def ctx(toolkit_db: Engine) -> ToolkitContext: - store = Store(catalog=il.Catalog(components={})) +def store(toolkit_db: Engine) -> Store: + """A store whose catalog knows the lineage-by-name test sources. + + Returns: + A store reading and writing the fixture database. + + """ + return Store(catalog=il.Catalog.from_assets([ShopSource, FinanceSource])) + + +@pytest.fixture +def ctx(store: Store) -> ToolkitContext: return ToolkitContext(store=store, catalog=CATALOG_DUMP, org_id=ORG_ID) @@ -116,12 +159,8 @@ def _seed_chain(org_id: Any = ORG_ID) -> dict[str, Any]: b = Component(org_id=org_id, kind="asset", key="b", parent_id=source.id) c = Component(org_id=org_id, kind="asset", key="c", parent_id=source.id) deps = [ - ComponentRelation( - src_id=b.id, dst_id=a.id, type="upstream", slot="a", org_id=org_id, src_kind="asset", dst_kind="asset" - ), - ComponentRelation( - src_id=c.id, dst_id=b.id, type="upstream", slot="b", org_id=org_id, src_kind="asset", dst_kind="asset" - ), + ComponentRelation(src_id=b.id, dst_id=a.id, name="a", org_id=org_id, src_kind="asset", dst_kind="asset"), + ComponentRelation(src_id=c.id, dst_id=b.id, name="b", org_id=org_id, src_kind="asset", dst_kind="asset"), ] ids = {"source": source.id, "a": a.id, "b": b.id, "c": c.id} with Session(engine_module.get_engine()) as session: @@ -157,6 +196,68 @@ def test_other_orgs_edges_are_invisible(self, ctx: ToolkitContext): assert result.status == "success" assert result.lineage_count == 0 + def test_get_upstream_reports_relation_name(self, ctx: ToolkitContext, store: Store): + shop = store.components.create(ORG_ID, kind="source", key="shop_source") + finance = store.components.create(ORG_ID, kind="source", key="finance_source") + orders = next(child for child in shop.children if child.key == "orders") + revenue = next(child for child in finance.children if child.key == "revenue") + store.relations.add(revenue.id, name="orders", dst_id=orders.id) + + result = lineage.get_upstream(ctx, str(revenue.id)) + + assert result.status == "success" + assert [(edge.param_name, edge.asset_id) for edge in result.upstream] == [("orders", str(orders.id))] + + def test_lineage_ignores_non_asset_relations(self, ctx: ToolkitContext, store: Store): + shop = store.components.create(ORG_ID, kind="source", key="shop_source") + finance = store.components.create(ORG_ID, kind="source", key="finance_source") + orders = next(child for child in shop.children if child.key == "orders") + revenue = next(child for child in finance.children if child.key == "revenue") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + store.relations.add(revenue.id, name="orders", dst_id=orders.id) + store.relations.add(finance.id, name="destinations", dst_id=bq.id) + + result = lineage.get_full_lineage(ctx, str(revenue.id), direction="upstream") + + assert result.status == "success" + assert result.lineage_count == 1 + assert all(item.asset_key for item in result.lineage) + + +class TestBindRelation: + def test_bind_relation_creates_row(self, ctx: ToolkitContext, store: Store): + source = store.components.create(ORG_ID, kind="source", key="shop_source") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + + result = collection.bind_relation(ctx, str(source.id), "destinations", str(bq.id)) + + assert result.status == "success" + assert (result.name, result.dst_kind) == ("destinations", "destination") + + def test_bind_relation_wrong_kind_is_tool_error(self, ctx: ToolkitContext, store: Store): + source = store.components.create(ORG_ID, kind="source", key="shop_source") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + + result = collection.bind_relation(ctx, str(source.id), "connection", str(bq.id)) + + assert result.status == "error" + assert "does not accept" in result.error + + def test_bind_relation_bad_uuid_is_tool_error(self, ctx: ToolkitContext): + result = collection.bind_relation(ctx, "not-a-uuid", "destinations", "also-not-a-uuid") + + assert result.status == "error" + + def test_unbind_relation_removes_row(self, ctx: ToolkitContext, store: Store): + source = store.components.create(ORG_ID, kind="source", key="shop_source") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + store.relations.add(source.id, name="destinations", dst_id=bq.id) + + result = collection.unbind_relation(ctx, str(source.id), "destinations", str(bq.id)) + + assert result.status == "success" + assert store.relations.list_all(ORG_ID, name="destinations") == [] + class TestCatalog: def test_search_fields_matches_across_sources(self, ctx: ToolkitContext):