From 6063e183fff4df728d2b196dd1c7228737a3dc8e Mon Sep 17 00:00:00 2001 From: sloemodzn Date: Wed, 23 Sep 2026 08:03:37 +0530 Subject: [PATCH 1/2] refactor(db): one renderer for the container-path sentence (#1065) Fifteen provider files built their own `container path is [...]` refusal, eleven deriving the level list locally and four carrying a private `shapeList()`, two of which had already drifted. `assertContainerPathShape` now produces all of them, the way `assertObjectPathShape` does for object paths since #1023. Reading the fifteen showed they were not one shape. Three policies were hiding in them and the descriptor carries each one rather than collapsing them: - `shapes`: most engines accept exactly the declared depth; Couchbase, Trino, DuckDB and SQL Server accept every prefix up to it, because naming only the outer levels is a real address there. - `shapeNames`: Trino and PostgreSQL spell the shape from `level.id`, not the label, which only a varied declaration shows. Every other engine prints the label, which is the engine's own word for a person reading a refusal. - `emptyShapes`: an engine declaring no container level says why in its own words rather than printing an empty join that reads as a formatting bug. PostgreSQL's readers look the `schema` level up rather than reading by position, so a declaration that names none is refused by `containerSchema` itself, next to the reads it protects, and not by a field on the shared type that no other engine would ever set. `object-route.ts` cited line numbers in two of the shifted files, and the test that pins every citation is updated to the lines its anchors now sit on. Four `containerShapes()` helpers, two `declaredLevels()` copies and the imports they orphaned are gone. `tests/unit/db/container-path-renderer.test.ts` is the mechanical half of the acceptance criterion: it reads the provider tree and fails if any provider builds the sentence itself, matches the phrase in any quote style after comments are stripped, pins the fifteen engine files by name rather than behind a floor, or keeps a private container shape helper. Each descriptor field is pinned by a provider test, and ignoring one reddens exactly its own engine. --- src/lib/api/object-route.ts | 8 +- src/lib/db/object-kinds.ts | 86 +++++++++ .../providers/document/couchbase/objects.ts | 49 +++-- src/lib/db/providers/document/mongodb.ts | 28 ++- src/lib/db/providers/embedded/libredb.ts | 21 ++- src/lib/db/providers/keyvalue/redis.ts | 25 ++- src/lib/db/providers/sql/cassandra/objects.ts | 28 ++- .../db/providers/sql/clickhouse/objects.ts | 28 ++- src/lib/db/providers/sql/druid/objects.ts | 26 ++- src/lib/db/providers/sql/duckdb/objects.ts | 54 ++---- src/lib/db/providers/sql/libsql/objects.ts | 41 ++--- src/lib/db/providers/sql/mssql.ts | 47 ++--- src/lib/db/providers/sql/mysql.ts | 25 ++- src/lib/db/providers/sql/oracle.ts | 25 ++- src/lib/db/providers/sql/postgres.ts | 29 ++- src/lib/db/providers/sql/sqlite.ts | 39 ++-- src/lib/db/providers/sql/trino/objects.ts | 58 +++--- .../integration/db/postgres-provider.test.ts | 30 ++++ tests/unit/db/container-path-renderer.test.ts | 170 ++++++++++++++++++ 19 files changed, 573 insertions(+), 244 deletions(-) create mode 100644 tests/unit/db/container-path-renderer.test.ts diff --git a/src/lib/api/object-route.ts b/src/lib/api/object-route.ts index 35d2bf0ae..9da06f530 100644 --- a/src/lib/api/object-route.ts +++ b/src/lib/api/object-route.ts @@ -673,9 +673,9 @@ function boundText(part: ObjectSourcePart, limit: number): ObjectSourcePart { * was the only engine that had landed; the day-one set is now three and the count was re-measured * rather than the digit bumped, because what it counts is what the paragraph is for. * - * There are THREE producers of `edit`: `providers/sql/postgres.ts:3246`, gated on + * There are THREE producers of `edit`: `providers/sql/postgres.ts:3269`, gated on * `kindAcceptsSourceEdits(capabilities, kind)`; `providers/sql/trino/index.ts:1279` and - * `providers/keyvalue/redis.ts:1948`, both gated on `spec.acceptsSourceEdits === true`, which is the + * `providers/keyvalue/redis.ts:1957`, both gated on `spec.acceptsSourceEdits === true`, which is the * same fact read through the same declaration. All three sit on the READABLE arm, verified rather * than assumed: no producer attaches `edit` to a part carrying `unavailable`. * @@ -687,7 +687,7 @@ function boundText(part: ObjectSourcePart, limit: number): ObjectSourcePart { * Rule 2's producer set GREW and its character changed, which is the part a bumped digit would have * hidden. On PostgreSQL it is a by-product: that site spreads `truncated` and `edit` from a single * read, so a routine over `SOURCE_CHARACTER_LIMIT` reaches it. On Redis it is a DECIDED POSITION, - * stated at `redis.ts:1941-1947`: the affordance is offered on a truncated part deliberately, because + * stated at `redis.ts:1950-1956`: the affordance is offered on a truncated part deliberately, because * the bound is the CALLER's and the same object read without one is whole, so a provider that withheld * it there would be answering a property of the REQUEST as a property of the object. Rule 2 is what * makes that position safe on the standalone path, and the pane's predicate and `buildObjectEdit`'s @@ -707,7 +707,7 @@ function boundText(part: ObjectSourcePart, limit: number): ObjectSourcePart { * * THE BOUND, on both sides of the same constant. `edit-plan/route.ts:74` refuses a SUBMITTED text * longer than `EDIT_CHARACTER_LIMIT`, and all three day-one providers refuse a READ definition longer - * than it inside `buildObjectEdit`: `providers/sql/postgres.ts:3403`, `providers/keyvalue/redis.ts:2038` + * than it inside `buildObjectEdit`: `providers/sql/postgres.ts:3426`, `providers/keyvalue/redis.ts:2047` * and `providers/sql/trino/index.ts:1473`. The second is what closes the class rather than narrowing * it: a plan is minted only from the build's own read, so a definition the pane could only have shown * truncated never reaches a plan at all, whatever the client POSTs. diff --git a/src/lib/db/object-kinds.ts b/src/lib/db/object-kinds.ts index e741933ae..f8a2a0521 100644 --- a/src/lib/db/object-kinds.ts +++ b/src/lib/db/object-kinds.ts @@ -108,6 +108,92 @@ export function assertObjectPathShape( ); } +/** + * The engine half of `assertContainerPathShape`: the identity its error carries, and the + * three things the fifteen hoisted copies disagreed on. + */ +export type ContainerPathShapeEngine = { + /** The engine code the thrown `QueryError` is stamped with. */ + code: DatabaseType; + /** The message's opening subject, article included: "A MySQL", "An Oracle". */ + label: string; + /** + * Which path depths this engine accepts. `exact` takes the declared depth and nothing + * else; `prefixes` takes every depth up to it, because a container that names only the + * outer levels is a real address on those engines (a bucket with no scope, a catalog + * with no schema). Both refuse a path longer than the declaration. + */ + shapes: "exact" | "prefixes"; + /** + * What the message prints in place of a shape list when the declaration carries no + * container level at all. The empty join would read as a formatting bug rather than as + * the fact it is, so each engine spells it in its own words. + */ + emptyShapes: string; + /** + * Which field of a declared level spells the shape. `label` is the engine's own word for + * a person reading a refusal, which is what most engines print; `id` is what Trino and + * PostgreSQL print, because a declaration whose label is prose would otherwise describe + * a shape no read accepts, since every read binds its segment by `id`. On both engines' + * own declarations the two are the same word, so only a varied declaration shows it. + */ + shapeNames: "id" | "label"; +}; + +/** + * The path shapes a container is addressed by, spelled for a message: `[database]`, + * `[bucket] or [bucket, scope]`, or nothing at all for an engine that declares no level. + * + * An engine with no declared level accepts exactly one shape, the empty path, and prints + * `emptyShapes` rather than `[]` when a caller sends anything else. + */ +function containerShapeNames( + capabilities: ProviderCapabilities, + engine: ContainerPathShapeEngine, +): readonly string[][] { + const names = declaredLevels(capabilities).map((level) => + engine.shapeNames === "id" ? level.id : level.label.toLowerCase(), + ); + // The two families differ on exactly one case, the declaration that names no level. An + // `exact` engine accepts the empty path there, because the depth it asks for is zero; a + // `prefixes` engine accepts nothing at all, since every prefix of an empty list is a + // shape it never declared, and the message says so in the engine's own words. + if (engine.shapes === "exact") return names.length === 0 ? [[]] : [names]; + return names.map((_, index) => names.slice(0, index + 1)); +} + +function renderContainerShapes(shapes: readonly string[][], engine: ContainerPathShapeEngine): string { + if (shapes.length === 0 || shapes.every((shape) => shape.length === 0)) return engine.emptyShapes; + return shapes.map((shape) => `[${shape.join(", ")}]`).join(" or "); +} + +/** + * Refuses a container path that is not one of the shapes the DECLARATION describes. + * + * Hoisted from fifteen provider-local copies (#1065): eleven threw on a depth mismatch and + * four carried their own `shapeList()`, and two of those four had already drifted. The + * depth comes from `containerDepth()` through `declaredLevels` and the segment names from + * the declared labels, so the check and its message are the same array and nothing here + * can inherit a hardcoded 1. An engine's own opening words, its accepted depths and its + * empty-declaration wording travel through the descriptor, because those differ by design. + * + * It raises rather than reading a segment and carrying on: `undefined` bound to a + * parameter answers an empty folder that looks exactly like a container holding nothing, + * and a path one segment too long would bind the object's own name as the missing level. + */ +export function assertContainerPathShape( + capabilities: ProviderCapabilities, + container: readonly string[], + engine: ContainerPathShapeEngine, +): void { + const shapes = containerShapeNames(capabilities, engine); + if (shapes.some((shape) => shape.length === container.length)) return; + throw new QueryError( + `${engine.label} container path is ${renderContainerShapes(shapes, engine)}, received ${JSON.stringify(container)}`, + engine.code, + ); +} + /** * Whether THIS KIND accepts a row write. Absent and undeclared both read as false. * diff --git a/src/lib/db/providers/document/couchbase/objects.ts b/src/lib/db/providers/document/couchbase/objects.ts index 457adabf1..1480fca7b 100644 --- a/src/lib/db/providers/document/couchbase/objects.ts +++ b/src/lib/db/providers/document/couchbase/objects.ts @@ -107,7 +107,14 @@ */ import { QueryError } from "@/lib/db/errors"; -import { assertObjectPathShape, containerDepth, type ObjectPathShapeEngine } from "@/lib/db/object-kinds"; +import { + assertContainerPathShape, + assertObjectPathShape, + containerDepth, + type ContainerPathShapeEngine, + type ObjectPathShapeEngine, +} from "@/lib/db/object-kinds"; + import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, @@ -123,6 +130,20 @@ import { DOCUMENT_KEY_EXPRESSION, unquoteIndexKey } from "./introspect"; import { COUCHBASE_DEFAULT_SCOPE } from "./keyspace"; import type { CouchbaseRow, Keyspace } from "./transport"; +/** + * Couchbase's identity for the shared container-path renderer. + * + * `shapes: "prefixes"`: every depth up to the declaration is a real address here, because a + * caller may name only the outer levels. A path longer than the declaration is still refused. + */ +const COUCHBASE_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "couchbase", + label: "A Couchbase", + shapeNames: "label", + shapes: "prefixes", + emptyShapes: "nothing: this declaration carries no container level", +}; + // ============================================================================ // The declaration // ============================================================================ @@ -393,24 +414,6 @@ function requiredSegment( return segment; } -/** Every prefix of the declared levels: a bucket alone, or a bucket and a scope. */ -function containerShapes(capabilities: ProviderCapabilities): readonly string[][] { - const names = declaredLevels(capabilities).map((level) => level.label.toLowerCase()); - return names.map((_, index) => names.slice(0, index + 1)); -} - -/** - * The shapes above, spelled for a message: `[bucket] or [bucket, scope]`. - * - * A declaration carrying no container level has no shape at all, and the empty join would - * print "a Couchbase container path is , received []", which reads as a formatting bug - * rather than as the fact it is. - */ -function shapeList(shapes: readonly string[][]): string { - if (shapes.length === 0) return "nothing: this declaration carries no container level"; - return shapes.map((shape) => `[${shape.join(", ")}]`).join(" or "); -} - /** * What one container path addresses: the bucket to bind, and the scope to filter to. * @@ -424,13 +427,7 @@ export interface ContainerRead { } export function containerRead(capabilities: ProviderCapabilities, container: readonly string[]): ContainerRead { - const shapes = containerShapes(capabilities); - if (!shapes.some((shape) => shape.length === container.length)) { - throw new QueryError( - `A Couchbase container path is ${shapeList(shapes)}, received ${JSON.stringify(container)}`, - "couchbase", - ); - } + assertContainerPathShape(capabilities, container, COUCHBASE_CONTAINER_PATH_ENGINE); const segments = containerSegments(capabilities, container); return { bucket: requiredSegment(segments, "catalog"), scope: segments.schema }; } diff --git a/src/lib/db/providers/document/mongodb.ts b/src/lib/db/providers/document/mongodb.ts index 1ec92cae0..cc8b9376e 100644 --- a/src/lib/db/providers/document/mongodb.ts +++ b/src/lib/db/providers/document/mongodb.ts @@ -47,19 +47,36 @@ import { } from "../../types"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, - type ObjectPathShapeEngine, callerBoundTruncationReason, containerDepth, declaredKinds, findKind, isCountUnavailable, + type ContainerPathShapeEngine, + type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; + import { comparePaths } from "@/lib/db/object-path"; import { DatabaseConfigError, ConnectionError, QueryError, mapDatabaseError } from "../../errors"; import { formatBytes } from "../../utils/pool-manager"; import { CACHE_HIT_RATIO_UNAVAILABLE, formatCacheHitRatio, measuredNumber } from "@/lib/monitoring-cache-ratio"; +/** + * MongoDB's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the caller either names every declared level or is refused, + * because a partial path would leave a level unbound and answer an empty folder. + */ +const MONGODB_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "mongodb", + label: "A MongoDB", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // Types // ============================================================================ @@ -431,14 +448,7 @@ function containerSegment( * way to report a caller mistake. */ function containerDatabase(capabilities: ProviderCapabilities, container: readonly string[]): string { - const levels = declaredLevels(capabilities); - if (container.length !== levels.length) { - throw new QueryError( - `A MongoDB container path is [${levels.map((level) => level.label.toLowerCase()).join(", ")}], ` + - `received ${JSON.stringify(container)}`, - "mongodb", - ); - } + assertContainerPathShape(capabilities, container, MONGODB_CONTAINER_PATH_ENGINE); return containerSegment(capabilities, container, "schema"); } diff --git a/src/lib/db/providers/embedded/libredb.ts b/src/lib/db/providers/embedded/libredb.ts index 48dfa665d..ef6c851a1 100644 --- a/src/lib/db/providers/embedded/libredb.ts +++ b/src/lib/db/providers/embedded/libredb.ts @@ -48,9 +48,11 @@ import { type ObjectKindSpec, } from "../../types"; import { + assertContainerPathShape, assertObjectPathShape, callerBoundTruncationReason, containerDepth, + type ContainerPathShapeEngine, declaredKinds, findKind, type ObjectPathShapeEngine, @@ -62,6 +64,20 @@ import { CACHE_HIT_RATIO_UNAVAILABLE } from "@/lib/monitoring-cache-ratio"; import * as fs from "fs"; import * as path from "path"; +/** + * LibreDB's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the only path this engine accepts is the declared depth, which here + * is the empty one, so any segment at all is a caller holding another engine's model. + */ +const LIBREDB_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "libredb", + label: "A LibreDB", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // Lazy package loader (mirrors sqlite.ts loading bun:sqlite) // ============================================================================ @@ -309,10 +325,7 @@ function declaredLevels(capabilities: ProviderCapabilities): readonly ContainerL * database holding nothing. */ function assertContainerPath(capabilities: ProviderCapabilities, container: readonly string[]): void { - const levels = declaredLevels(capabilities); - if (container.length === levels.length) return; - const shape = levels.length === 0 ? "empty" : `[${levels.map((level) => level.label.toLowerCase()).join(", ")}]`; - throw new QueryError(`A LibreDB container path is ${shape}, received ${JSON.stringify(container)}`, "libredb"); + assertContainerPathShape(capabilities, container, LIBREDB_CONTAINER_PATH_ENGINE); } /** One enumerated object, with the two things `describeObject` needs to describe it. */ diff --git a/src/lib/db/providers/keyvalue/redis.ts b/src/lib/db/providers/keyvalue/redis.ts index a843cee32..deb536b5b 100644 --- a/src/lib/db/providers/keyvalue/redis.ts +++ b/src/lib/db/providers/keyvalue/redis.ts @@ -19,10 +19,12 @@ import Redis, { type RedisOptions } from "ioredis"; import { BaseDatabaseProvider } from "../../base-provider"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, type ObjectPathShapeEngine, callerBoundTruncationReason, containerDepth, + type ContainerPathShapeEngine, declaredKinds, findKind, requireEditableKind, @@ -72,6 +74,20 @@ import { } from "../../types"; import { DatabaseConfigError, QueryError, ConnectionError } from "../../errors"; +/** + * Redis's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the caller either names every declared level or is refused, + * because a partial path would leave a level unbound and answer an empty folder. + */ +const REDIS_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "redis", + label: "A Redis", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + /** * The server's own words for "you asked me to discard and there is nothing queued". * @@ -317,14 +333,7 @@ function containerSegment( * limit of the deployment, which this function does not know without a second round trip. */ function containerDatabase(capabilities: ProviderCapabilities, container: readonly string[]): number { - const levels = declaredLevels(capabilities); - if (container.length !== levels.length) { - throw new QueryError( - `A Redis container path is [${levels.map((level) => level.label.toLowerCase()).join(", ")}], ` + - `received ${JSON.stringify(container)}`, - "redis", - ); - } + assertContainerPathShape(capabilities, container, REDIS_CONTAINER_PATH_ENGINE); const segment = containerSegment(capabilities, container, "schema"); if (!/^\d+$/.test(segment)) { throw new QueryError(`A Redis database is a number, received ${JSON.stringify(segment)}`, "redis"); diff --git a/src/lib/db/providers/sql/cassandra/objects.ts b/src/lib/db/providers/sql/cassandra/objects.ts index a59cdbade..9f91e91e4 100644 --- a/src/lib/db/providers/sql/cassandra/objects.ts +++ b/src/lib/db/providers/sql/cassandra/objects.ts @@ -97,14 +97,17 @@ import { QueryError } from "@/lib/db/errors"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, - type ObjectPathShapeEngine, callerBoundTruncationReason, containerDepth, declaredKinds, findKind, requireSourceKind, + type ContainerPathShapeEngine, + type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; + import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, @@ -127,6 +130,20 @@ import { CassandraTransportError, type CassandraRow, type CassandraTransport } f const PROVIDER = "cassandra" as const; +/** + * Cassandra's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the caller either names every declared level or is refused, + * because a partial path would leave a level unbound and answer an empty folder. + */ +const CASSANDRA_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: PROVIDER, + label: "A Cassandra", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // Declaration // ============================================================================ @@ -594,14 +611,7 @@ function containerSegment( * the worst way to report a caller mistake. */ function containerKeyspace(capabilities: ProviderCapabilities, container: readonly string[]): string { - const levels = declaredLevels(capabilities); - if (container.length !== levels.length) { - throw new QueryError( - `A Cassandra container path is [${levels.map((level) => level.label.toLowerCase()).join(", ")}], ` + - `received ${JSON.stringify(container)}`, - PROVIDER, - ); - } + assertContainerPathShape(capabilities, container, CASSANDRA_CONTAINER_PATH_ENGINE); return containerSegment(capabilities, container, "schema"); } diff --git a/src/lib/db/providers/sql/clickhouse/objects.ts b/src/lib/db/providers/sql/clickhouse/objects.ts index e916b224a..4bd4f90dd 100644 --- a/src/lib/db/providers/sql/clickhouse/objects.ts +++ b/src/lib/db/providers/sql/clickhouse/objects.ts @@ -61,14 +61,17 @@ import { QueryError } from "@/lib/db/errors"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, - type ObjectPathShapeEngine, callerBoundTruncationReason, containerDepth, declaredKinds, findKind, requireSourceKind, + type ContainerPathShapeEngine, + type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; + import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, @@ -98,6 +101,20 @@ import { type ClickHouseRow, type ClickHouseTransport, ClickHouseTransportError const PROVIDER = "clickhouse" as const; +/** + * ClickHouse's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the caller either names every declared level or is refused, + * because a partial path would leave a level unbound and answer an empty folder. + */ +const CLICKHOUSE_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: PROVIDER, + label: "A ClickHouse", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // Declaration // ============================================================================ @@ -549,14 +566,7 @@ function containerSegment( * which is the worst way to report a caller mistake. */ function containerDatabase(capabilities: ProviderCapabilities, container: readonly string[]): string { - const levels = declaredLevels(capabilities); - if (container.length !== levels.length) { - throw new QueryError( - `A ClickHouse container path is [${levels.map((level) => level.label.toLowerCase()).join(", ")}], ` + - `received ${JSON.stringify(container)}`, - PROVIDER, - ); - } + assertContainerPathShape(capabilities, container, CLICKHOUSE_CONTAINER_PATH_ENGINE); return containerSegment(capabilities, container, "schema"); } diff --git a/src/lib/db/providers/sql/druid/objects.ts b/src/lib/db/providers/sql/druid/objects.ts index 4898aa94e..764c07373 100644 --- a/src/lib/db/providers/sql/druid/objects.ts +++ b/src/lib/db/providers/sql/druid/objects.ts @@ -73,13 +73,16 @@ import { QueryError } from "@/lib/db/errors"; import { + assertContainerPathShape, assertObjectPathShape, callerBoundTruncationReason, containerDepth, declaredKinds, findKind, + type ContainerPathShapeEngine, type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; + import { comparePaths } from "@/lib/db/object-path"; import type { Container, @@ -97,6 +100,20 @@ import { DRUID_SCHEMA_NAME, DRUID_SYSTEM_READ_TIMEOUT_MS, readColumn, readIdenti const PROVIDER = "druid" as const; +/** + * Druid's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the caller either names every declared level or is refused, + * because a partial path would leave a level unbound and answer an empty folder. + */ +const DRUID_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: PROVIDER, + label: "A Druid", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + /** * Druid's identity for the shared path-shape renderer. * @@ -380,14 +397,7 @@ function containerSegment( * to report a caller mistake. */ function containerSchema(capabilities: ProviderCapabilities, container: readonly string[]): string { - const levels = declaredLevels(capabilities); - if (container.length !== levels.length) { - throw new QueryError( - `A Druid container path is [${levels.map((level) => level.label.toLowerCase()).join(", ")}], ` + - `received ${JSON.stringify(container)}`, - PROVIDER, - ); - } + assertContainerPathShape(capabilities, container, DRUID_CONTAINER_PATH_ENGINE); return containerSegment(capabilities, container, "schema"); } diff --git a/src/lib/db/providers/sql/duckdb/objects.ts b/src/lib/db/providers/sql/duckdb/objects.ts index 782cde3dc..a2cb62654 100644 --- a/src/lib/db/providers/sql/duckdb/objects.ts +++ b/src/lib/db/providers/sql/duckdb/objects.ts @@ -37,7 +37,7 @@ */ import { QueryError } from "../../../errors"; -import { containerDepth } from "../../../object-kinds"; +import { assertContainerPathShape, containerDepth, type ContainerPathShapeEngine } from "../../../object-kinds"; import { comparePaths } from "../../../object-path"; import { unquoteLiteral } from "@/lib/sql/values"; import { displayName } from "./introspect"; @@ -52,6 +52,20 @@ import type { ProviderCapabilities, } from "../../../types"; +/** + * DuckDB's identity for the shared container-path renderer. + * + * `shapes: "prefixes"`: every depth up to the declaration is a real address here, because a + * caller may name only the outer levels. A path longer than the declaration is still refused. + */ +const DUCKDB_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "duckdb", + label: "A DuckDB", + shapeNames: "label", + shapes: "prefixes", + emptyShapes: "nothing: this declaration carries no container level", +}; + // ============================================================================ // The macro vocabulary, derived from the ENGINE // ============================================================================ @@ -756,36 +770,8 @@ function requiredSegment( return segment; } -/** - * The container paths this engine accepts, outermost first, as segment NAMES. - * - * Every prefix of the declared levels, which at two levels means a catalog alone or a - * catalog and a schema. Both are real containers: the tree only draws folders at the - * deepest level (`src/components/object-tree/flatten.ts`), but `assertContainerDepth` in - * `src/lib/api/object-route.ts` admits any path down to the declared depth and - * `tests/helpers/object-surface-conformance.ts` reads counts at the OUTER one, so "how - * many tables does this whole catalog hold" is a question with a true answer rather than - * a caller mistake. SQL Server answered the same way for the same reason. - * - * The names in the message are the declared LABELS, the engine's own word for a person - * reading a refusal; the code addresses the same segments by `ContainerLevelSpec.id`. The - * depth behind both is `containerDepth()`, so the check and the sentence cannot disagree. - */ -function containerShapes(capabilities: ProviderCapabilities): readonly string[][] { - const names = declaredLevels(capabilities).map((level) => level.label.toLowerCase()); - return names.map((_, index) => names.slice(0, index + 1)); -} - -/** - * The shapes above, spelled for a message: `[database] or [database, schema]`. - * - * A declaration carrying no container level has no shape at all, and the empty join would - * print "a DuckDB container path is , received []", which reads as a formatting bug rather - * than as the fact it is. Reachable only through a declaration this engine does not have, - * and pinned by the test that hands the provider one. - */ +/** The one shape a DuckDB object path takes, spelled for the message in `objectRead()`. */ function shapeList(shapes: readonly string[][]): string { - if (shapes.length === 0) return "nothing: this declaration carries no container level"; return shapes.map((shape) => `[${shape.join(", ")}]`).join(" or "); } @@ -801,13 +787,7 @@ function containerTarget( capabilities: ProviderCapabilities, container: readonly string[], ): Partial> { - const shapes = containerShapes(capabilities); - if (!shapes.some((shape) => shape.length === container.length)) { - throw new QueryError( - `A DuckDB container path is ${shapeList(shapes)}, received ${JSON.stringify(container)}`, - "duckdb", - ); - } + assertContainerPathShape(capabilities, container, DUCKDB_CONTAINER_PATH_ENGINE); return containerSegments(capabilities, container); } diff --git a/src/lib/db/providers/sql/libsql/objects.ts b/src/lib/db/providers/sql/libsql/objects.ts index 3f87e4602..210e80885 100644 --- a/src/lib/db/providers/sql/libsql/objects.ts +++ b/src/lib/db/providers/sql/libsql/objects.ts @@ -30,7 +30,6 @@ import type { ColumnSchema, Container, - ContainerLevelSpec, DatabaseObject, ForeignKeySchema, IndexSchema, @@ -44,19 +43,35 @@ import type { import { QueryError } from "@/lib/db/errors"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, - type ObjectPathShapeEngine, callerBoundTruncationReason, - containerDepth, declaredKinds, findKind, requireSourceKind, + type ContainerPathShapeEngine, + type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; + import { comparePaths } from "@/lib/db/object-path"; import { unquoteLiteral } from "@/lib/sql/values"; import { readNumber, readText } from "./introspect"; import type { LibSQLBatchOutcome, LibSQLRow, LibSQLStatement, LibSQLTransport } from "./transport"; +/** + * libSQL's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the only path this engine accepts is the declared depth, which here + * is the empty one, so any segment at all is a caller holding another engine's model. + */ +const LIBSQL_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "libsql", + label: "A libSQL", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // The one schema, and the statements that read it // ============================================================================ @@ -398,21 +413,6 @@ export const LIBSQL_OBJECT_KINDS: readonly ObjectKindSpec[] = [ // Pure derivations // ============================================================================ -/** - * The container levels this provider declares, sliced to the depth `containerDepth()` - * reports. - * - * One reader for the whole file, so the depth and the level list can never be taken by two - * different rules. `containerDepth()` is what decides, never `containerLevels.length`: - * absent and empty are the same fact, and two callers reading the field by different rules - * is how the tree and the API route came to disagree about one engine. - * - * On libSQL this answers the empty array, which is the engine and not a degenerate case. - */ -function declaredLevels(capabilities: ProviderCapabilities): readonly ContainerLevelSpec[] { - return (capabilities.containerLevels ?? []).slice(0, containerDepth(capabilities)); -} - /** * Refuses a container path that is not the shape the DECLARATION describes. * @@ -427,10 +427,7 @@ function declaredLevels(capabilities: ProviderCapabilities): readonly ContainerL * like a database holding nothing is the worst way to report that. */ function assertContainerPath(capabilities: ProviderCapabilities, container: readonly string[]): void { - const levels = declaredLevels(capabilities); - if (container.length === levels.length) return; - const shape = levels.length === 0 ? "empty" : `[${levels.map((level) => level.label.toLowerCase()).join(", ")}]`; - throw new QueryError(`A libSQL container path is ${shape}, received ${JSON.stringify(container)}`, "libsql"); + assertContainerPathShape(capabilities, container, LIBSQL_CONTAINER_PATH_ENGINE); } /** diff --git a/src/lib/db/providers/sql/mssql.ts b/src/lib/db/providers/sql/mssql.ts index 8da99249d..abec9fce8 100644 --- a/src/lib/db/providers/sql/mssql.ts +++ b/src/lib/db/providers/sql/mssql.ts @@ -44,8 +44,10 @@ import { } from "../../types"; import { applySourceBound, + assertContainerPathShape, callerBoundTruncationReason, containerDepth, + type ContainerPathShapeEngine, declaredKinds, findKind, requireSourceKind, @@ -67,6 +69,20 @@ import { resolveSqlGrammar, type SqlGrammar } from "@/lib/sql/grammar"; import { readStatementEnd } from "@/lib/sql/statement-end"; import { CACHE_HIT_RATIO_UNAVAILABLE, formatCacheHitRatio, measuredNumber } from "@/lib/monitoring-cache-ratio"; +/** + * SQL Server's identity for the shared container-path renderer. + * + * `shapes: "prefixes"`: every depth up to the declaration is a real address here, because a + * caller may name only the outer levels. A path longer than the declaration is still refused. + */ +const MSSQL_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "mssql", + label: "A SQL Server", + shapeNames: "label", + shapes: "prefixes", + emptyShapes: "nothing: this declaration carries no container level", +}; + /** * `SELECT ... ` with `TOP n` spliced in where T-SQL wants it, or `null` when this * statement has no leading `SELECT` to splice after. @@ -1019,28 +1035,7 @@ function requiredSegment( return segment; } -/** - * The container paths this engine accepts, outermost first, as segment NAMES. - * - * Every prefix of the declared levels, which on a two-level engine means a database alone - * or a database and a schema. Both are real containers here: the tree only ever draws - * folders at the deepest level (`src/components/object-tree/flatten.ts`), but - * `assertContainerDepth` in `src/lib/api/object-route.ts` admits any path down to the - * declared depth and `assertObjectSurface` reads counts at the OUTER one, so a database - * holding twelve tables across three schemas is a question with a true answer rather than - * a caller mistake. - * - * The names in the message are the declared LABELS, which is the engine's own word for a - * person reading a refusal; the code addresses the same segments by `ContainerLevelSpec.id` - * through `containerSegments()`. The depth behind both is `containerDepth()`, so the check - * and the sentence it raises cannot disagree. - */ -function containerShapes(capabilities: ProviderCapabilities): readonly string[][] { - const names = declaredLevels(capabilities).map((level) => level.label.toLowerCase()); - return names.map((_, index) => names.slice(0, index + 1)); -} - -/** The shapes above, spelled for a message: `[database] or [database, schema]`. */ +/** A kind's path shapes, spelled for the message in `objectAddress()`. */ function shapeList(shapes: readonly string[][]): string { return shapes.map((shape) => `[${shape.join(", ")}]`).join(" or "); } @@ -1056,13 +1051,7 @@ function containerTarget( capabilities: ProviderCapabilities, container: readonly string[], ): Partial> { - const shapes = containerShapes(capabilities); - if (!shapes.some((shape) => shape.length === container.length)) { - throw new QueryError( - `A SQL Server container path is ${shapeList(shapes)}, received ${JSON.stringify(container)}`, - "mssql", - ); - } + assertContainerPathShape(capabilities, container, MSSQL_CONTAINER_PATH_ENGINE); return containerSegments(capabilities, container); } diff --git a/src/lib/db/providers/sql/mysql.ts b/src/lib/db/providers/sql/mysql.ts index b206ad193..065fa0b5e 100644 --- a/src/lib/db/providers/sql/mysql.ts +++ b/src/lib/db/providers/sql/mysql.ts @@ -43,10 +43,12 @@ import { import { DatabaseConfigError, ConnectionError, QueryError, mapDatabaseError } from "../../errors"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, type ObjectPathShapeEngine, callerBoundTruncationReason, containerDepth, + type ContainerPathShapeEngine, declaredKinds, findKind, requireSourceKind, @@ -57,6 +59,20 @@ import { measuredNullableAggregate } from "../../utils/measured-aggregate"; import { CACHE_HIT_RATIO_UNAVAILABLE, formatCacheHitRatio, measuredNumber } from "@/lib/monitoring-cache-ratio"; import { unquoteLiteral } from "@/lib/sql/values"; +/** + * MySQL's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the caller either names every declared level or is refused, + * because a partial path would leave a level unbound and answer an empty folder. + */ +const MYSQL_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "mysql", + label: "A MySQL", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + /** * mysql2 3.23 narrowed `execute`'s values parameter from `any` to a concrete * `ExecuteValues` union that excludes `undefined`. The provider interface every @@ -1315,14 +1331,7 @@ function containerSegment( * position holds the schema is read off the declaration rather than assumed. */ function containerSchema(capabilities: ProviderCapabilities, container: readonly string[]): string { - const levels = declaredLevels(capabilities); - if (container.length !== levels.length) { - throw new QueryError( - `A MySQL container path is [${levels.map((level) => level.label.toLowerCase()).join(", ")}], ` + - `received ${JSON.stringify(container)}`, - "mysql", - ); - } + assertContainerPathShape(capabilities, container, MYSQL_CONTAINER_PATH_ENGINE); return containerSegment(capabilities, container, "schema"); } diff --git a/src/lib/db/providers/sql/oracle.ts b/src/lib/db/providers/sql/oracle.ts index dc25990ed..a0b13e384 100644 --- a/src/lib/db/providers/sql/oracle.ts +++ b/src/lib/db/providers/sql/oracle.ts @@ -41,10 +41,12 @@ import { } from "../../types"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, type ObjectPathShapeEngine, callerBoundTruncationReason, containerDepth, + type ContainerPathShapeEngine, declaredKinds, findKind, } from "../../object-kinds"; @@ -63,6 +65,20 @@ import { resolveSqlGrammar } from "@/lib/sql/grammar"; import { readStatementEnd } from "@/lib/sql/statement-end"; import { CACHE_HIT_RATIO_UNAVAILABLE, formatCacheHitRatio, measuredNumber } from "@/lib/monitoring-cache-ratio"; +/** + * Oracle's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the caller either names every declared level or is refused, + * because a partial path would leave a level unbound and answer an empty folder. + */ +const ORACLE_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "oracle", + label: "An Oracle", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // SQL Statements // ============================================================================ @@ -936,14 +952,7 @@ function notableStatus(status: string): { status?: string } { * `CREATE USER "app"` is legal, so upper-casing here would make that owner unreachable. */ function containerOwner(capabilities: ProviderCapabilities, container: readonly string[]): string { - const levels = declaredLevels(capabilities); - if (container.length !== levels.length) { - throw new QueryError( - `An Oracle container path is [${levels.map((level) => level.label.toLowerCase()).join(", ")}], ` + - `received ${JSON.stringify(container)}`, - "oracle", - ); - } + assertContainerPathShape(capabilities, container, ORACLE_CONTAINER_PATH_ENGINE); return ownerSegment(capabilities, container); } diff --git a/src/lib/db/providers/sql/postgres.ts b/src/lib/db/providers/sql/postgres.ts index 6a7fef5c8..4a78f1cbd 100644 --- a/src/lib/db/providers/sql/postgres.ts +++ b/src/lib/db/providers/sql/postgres.ts @@ -47,10 +47,12 @@ import { } from "../../types"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, type ObjectPathShapeEngine, callerBoundTruncationReason, containerDepth, + type ContainerPathShapeEngine, declaredKinds, findKind, kindAcceptsSourceEdits, @@ -75,6 +77,21 @@ import { formatBytes } from "../../utils/pool-manager"; import { measuredNullableAggregate } from "../../utils/measured-aggregate"; import { CACHE_HIT_RATIO_UNAVAILABLE, formatCacheHitRatio, measuredNumber } from "@/lib/monitoring-cache-ratio"; +/** + * PostgreSQL's identity for the shared container-path renderer. + * + * `shapes: "exact"`: every declared level is named or the path is refused. Which level the + * readers need is not a field here: they look `schema` up rather than by position, so a + * declaration that names none is refused by `containerSchema` itself, where the reads are. + */ +const POSTGRES_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "postgres", + label: "A PostgreSQL", + shapeNames: "id", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // Type parsers // ============================================================================ @@ -1400,15 +1417,21 @@ function declaredLevels(capabilities: ProviderCapabilities): readonly ContainerL * A path of another depth is a caller that built it from another engine's shape, and it * raises rather than reading a segment and carrying on: `undefined` bound to `$1` would * answer an empty folder that looks exactly like a schema holding nothing. + * + * The same failure arrives through a declaration rather than a caller: a depth-matching one + * that names no `schema` level passes the shared shape check, so the lookup below refuses + * it. A rule only this engine's readers need cannot be seen by that check, which compares + * depths, so it lives here, next to the reads it protects. */ function containerSchema(capabilities: ProviderCapabilities, container: readonly string[]): string { + assertContainerPathShape(capabilities, container, POSTGRES_CONTAINER_PATH_ENGINE); const levels = declaredLevels(capabilities); const index = levels.findIndex((level) => level.id === "schema"); - const segment = container.length === levels.length && index >= 0 ? container[index] : undefined; + const segment = index < 0 ? undefined : container[index]; if (segment === undefined) { throw new QueryError( - `A PostgreSQL container path is [${levels.map((level) => level.id).join(", ")}], ` + - `received ${JSON.stringify(container)}`, + `A PostgreSQL path needs a "schema" container level and a segment for it; the declaration is ` + + `[${levels.map((level) => level.id).join(", ")}] and the path is ${JSON.stringify(container)}`, "postgres", ); } diff --git a/src/lib/db/providers/sql/sqlite.ts b/src/lib/db/providers/sql/sqlite.ts index 94c2a4325..f862dc874 100644 --- a/src/lib/db/providers/sql/sqlite.ts +++ b/src/lib/db/providers/sql/sqlite.ts @@ -13,7 +13,6 @@ import { SQLBaseProvider } from "./sql-base"; import { type Container, - type ContainerLevelSpec, type DatabaseObject, type DatabaseConnection, type KindCount, @@ -52,10 +51,11 @@ import { loadSQLiteDriver, type SQLiteDatabase } from "./sqlite-driver"; import { declaredColumnTypes } from "./column-types"; import { applySourceBound, + assertContainerPathShape, assertObjectPathShape, type ObjectPathShapeEngine, callerBoundTruncationReason, - containerDepth, + type ContainerPathShapeEngine, declaredKinds, findKind, requireSourceKind, @@ -67,6 +67,20 @@ import { logger } from "@/lib/logger"; import * as fs from "fs"; import * as path from "path"; +/** + * SQLite's identity for the shared container-path renderer. + * + * `shapes: "exact"`: the only path this engine accepts is the declared depth, which here + * is the empty one, so any segment at all is a caller holding another engine's model. + */ +const SQLITE_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: "sqlite", + label: "A SQLite", + shapeNames: "label", + shapes: "exact", + emptyShapes: "empty", +}; + // ============================================================================ // Type Definitions // ============================================================================ @@ -439,22 +453,6 @@ interface ObjectDetailRows { readonly foreignKeys: readonly ObjectForeignKeyRow[]; } -/** - * The container levels this provider declares, sliced to the depth `containerDepth()` - * reports. - * - * One reader for the whole file, so the depth and the level list can never be taken by - * two different rules. `containerDepth()` is what decides, never `containerLevels.length`: - * absent and empty are the same fact and two callers reading the field by different rules - * is how the tree and the API route came to disagree about one engine. - * - * On SQLite this answers the empty array, which is the point of the task and not a - * degenerate case. - */ -function declaredLevels(capabilities: ProviderCapabilities): readonly ContainerLevelSpec[] { - return (capabilities.containerLevels ?? []).slice(0, containerDepth(capabilities)); -} - /** * Refuses a container path that is not the shape the DECLARATION describes. * @@ -469,10 +467,7 @@ function declaredLevels(capabilities: ProviderCapabilities): readonly ContainerL * like a database holding nothing is the worst way to report that. */ function assertContainerPath(capabilities: ProviderCapabilities, container: readonly string[]): void { - const levels = declaredLevels(capabilities); - if (container.length === levels.length) return; - const shape = levels.length === 0 ? "empty" : `[${levels.map((level) => level.label.toLowerCase()).join(", ")}]`; - throw new QueryError(`A SQLite container path is ${shape}, received ${JSON.stringify(container)}`, "sqlite"); + assertContainerPathShape(capabilities, container, SQLITE_CONTAINER_PATH_ENGINE); } /** diff --git a/src/lib/db/providers/sql/trino/objects.ts b/src/lib/db/providers/sql/trino/objects.ts index a25a3e213..81c90abb6 100644 --- a/src/lib/db/providers/sql/trino/objects.ts +++ b/src/lib/db/providers/sql/trino/objects.ts @@ -64,7 +64,8 @@ */ import { QueryError } from "@/lib/db/errors"; -import { containerDepth } from "@/lib/db/object-kinds"; +import { assertContainerPathShape, containerDepth, type ContainerPathShapeEngine } from "@/lib/db/object-kinds"; + import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, @@ -81,6 +82,20 @@ import { TrinoTransportError, type TrinoRow } from "./transport"; /** The canonical type-id, for the errors raised here. */ const TYPE_ID = "trino"; +/** + * Trino's identity for the shared container-path renderer. + * + * `shapes: "prefixes"`: every depth up to the declaration is a real address here, because a + * caller may name only the outer levels. A path longer than the declaration is still refused. + */ +const TRINO_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = { + code: TYPE_ID, + label: "A Trino", + shapeNames: "id", + shapes: "prefixes", + emptyShapes: "nothing: this declaration carries no container level", +}; + // ============================================================================ // Quoting // ============================================================================ @@ -479,36 +494,8 @@ function requiredSegment( return segment; } -/** - * The container paths this engine accepts, outermost first, as segment NAMES. - * - * Every prefix of the declared levels, which at two levels means a catalog alone or a - * catalog and a schema. Both are real containers: the tree only draws folders at the - * deepest level (`src/components/object-tree/flatten.ts`), but `assertContainerDepth` in - * `src/lib/api/object-route.ts` admits any path down to the declared depth and - * `tests/helpers/object-surface-conformance.ts` reads counts at the OUTER one, so "how many - * tables does this whole catalog hold" is a question with a true answer rather than a - * caller mistake. SQL Server and DuckDB answered the same way for the same reason. - */ -function containerShapes(capabilities: ProviderCapabilities): readonly string[][] { - // `level.id` and NOT `level.label.toLowerCase()`: `id` is the field every read binds by - // (`containerSegments()` keys the record with it), so spelling the shape from `label` - // would describe a path shape no read accepts the moment a declaration's label is prose - // rather than its id capitalised. The two are the same word on Trino's own declaration, - // which is exactly why the divergence was invisible until a test varied the labels. - const names = declaredLevels(capabilities).map((level) => level.id); - return names.map((_, index) => names.slice(0, index + 1)); -} - -/** - * The shapes above, spelled for a message: `[catalog] or [catalog, schema]`. - * - * A declaration carrying no container level has no shape at all, and the empty join would - * print "a Trino container path is , received []", which reads as a formatting bug rather - * than as the fact it is. - */ +/** The one shape a Trino object path takes, spelled for the message in `objectRead()`. */ function shapeList(shapes: readonly string[][]): string { - if (shapes.length === 0) return "nothing: this declaration carries no container level"; return shapes.map((shape) => `[${shape.join(", ")}]`).join(" or "); } @@ -520,13 +507,7 @@ function shapeList(shapes: readonly string[][]): string { * that looks exactly like a schema holding nothing. */ export function containerRead(capabilities: ProviderCapabilities, container: readonly string[]): TrinoContainer { - const shapes = containerShapes(capabilities); - if (!shapes.some((shape) => shape.length === container.length)) { - throw new QueryError( - `A Trino container path is ${shapeList(shapes)}, received ${JSON.stringify(container)}`, - TYPE_ID, - ); - } + assertContainerPathShape(capabilities, container, TRINO_CONTAINER_PATH_ENGINE); const segments = containerSegments(capabilities, container); const catalog = requiredSegment(segments, "catalog"); const schema = segments.schema; @@ -562,7 +543,8 @@ export function objectRead( TYPE_ID, ); } - // `level.id`, the field the segments below are resolved by. See {@link containerShapes}. + // `level.id`, the field the segments below are resolved by. See the `shapeNames` note on + // {@link ContainerPathShapeEngine} for why the refusal spells the same field. const shape = [...declaredLevels(capabilities).map((level) => level.id), "name"]; if (path.length !== shape.length) { throw new QueryError( diff --git a/tests/integration/db/postgres-provider.test.ts b/tests/integration/db/postgres-provider.test.ts index aeee09c1e..c920d07e5 100644 --- a/tests/integration/db/postgres-provider.test.ts +++ b/tests/integration/db/postgres-provider.test.ts @@ -5107,6 +5107,36 @@ describe("PostgreSQL bulk column read", () => { await provider.disconnect(); }); + /** + * The schema level is REQUIRED by this engine's readers, not just the depth. + * + * A declaration whose one level is called `catalog` has the depth the shared check wants, + * so the renderer accepts `["shop"]` here and the refusal has to come from this engine. + * PostgreSQL's readers do not read their segment by position: they look the `schema` level + * up and bind what they find, so with no such level the read would bind `undefined` where + * `$1` belongs and answer an empty folder that looks exactly like a schema holding + * nothing. Nothing else in this file covers it, because every other fixture declares a + * `schema` level. + */ + test("a declaration with no schema level is refused even when the depth matches", async () => { + mockQueryFn = async () => ({ rows: [] }); + const provider = makeProvider(); + await provider.connect(); + const spy = spyOn(provider, "getCapabilities").mockReturnValue({ + ...provider.getCapabilities(), + containerLevels: [{ id: "catalog", label: "Catalog", labelPlural: "Catalogs" }], + }); + + try { + await expect(provider.describeObjects(["shop"], "table")).rejects.toThrow( + /A PostgreSQL path needs a "schema" container level and a segment for it; the declaration is \[catalog\] and the path is \["shop"\]/, + ); + } finally { + spy.mockRestore(); + } + await provider.disconnect(); + }); + test("a container path that is not one schema is refused, rather than read as empty", async () => { mockQueryFn = async () => ({ rows: [] }); const provider = makeProvider(); diff --git a/tests/unit/db/container-path-renderer.test.ts b/tests/unit/db/container-path-renderer.test.ts new file mode 100644 index 000000000..a4539d1e0 --- /dev/null +++ b/tests/unit/db/container-path-renderer.test.ts @@ -0,0 +1,170 @@ +import { describe, test, expect } from "bun:test"; +import * as fs from "fs"; +import * as path from "path"; + +/** + * The container-path sentence has one producer under `src/lib/db/providers`. + * + * #1023 hoisted the object-path checks so `assertObjectPathShape` was the only thing left + * rendering a `path is [...]` sentence. The container family was the same defect one layer + * over and was not hoisted with it: fifteen provider files rendered their own + * `container path is [...]`, eleven deriving the level list locally first and four carrying + * a private `shapeList()`, two of which had already drifted (SQL Server's had lost the + * guard for a declaration naming no level, so an empty one printed + * `A SQL Server container path is , received [...]`). + * + * This file is the mechanical half of #1065's acceptance criterion, the way + * `object-surface-conformance.test.ts` is for the object family. It reads the provider tree + * rather than exercising a provider, because the claim is about where the sentence is + * BUILT: a behavioural test can only show that one engine's message is right, and the + * defect was that fifteen engines each built their own. + */ + +// Anchored to this file rather than to `process.cwd()`, because a runner that launched this +// file from anywhere but the repository root would otherwise read nothing and pass. +const ROOT = path.resolve(import.meta.dir, "../../.."); +const PROVIDERS = path.join(ROOT, "src/lib/db/providers"); + +/** `path.relative` spells the separator by HOST; the pinned list below is POSIX-spelled. */ +const repoRelative = (file: string): string => path.relative(ROOT, file).split(path.sep).join("/"); + +function sourceFiles(dir: string): string[] { + const found: string[] = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) found.push(...sourceFiles(full)); + else if (entry.name.endsWith(".ts")) found.push(full); + } + return found; +} + +/** + * The source with comments removed, one entry per line, so line numbers survive. + * + * Prose quotes the phrase on purpose (a dozen docblocks explain why a path is refused), and + * a blanket grep would fail on the documentation that records the fix. So both comment + * forms are removed first: everything after a `//` on its line, and any block comment + * wherever it opens, including a docblock whose continuation lines carry no marker of their + * own. Only what remains is code. + */ +function codeLines(source: string): string[] { + const lines: string[] = []; + let inBlock = false; + for (const raw of source.split("\n")) { + let code = ""; + let index = 0; + while (index < raw.length) { + if (inBlock) { + const close = raw.indexOf("*/", index); + if (close < 0) { + index = raw.length; + continue; + } + inBlock = false; + index = close + 2; + continue; + } + const open = raw.indexOf("/*", index); + const line = raw.indexOf("//", index); + if (line >= 0 && (open < 0 || line < open)) { + code += raw.slice(index, line); + index = raw.length; + continue; + } + if (open < 0) { + code += raw.slice(index); + index = raw.length; + continue; + } + code += raw.slice(index, open); + inBlock = true; + index = open + 2; + } + lines.push(code); + } + return lines; +} + +/** + * Every line of CODE that RENDERS the sentence, rather than one that talks about it. + * + * Where the phrase survives the comment strip it can only sit inside a string literal, + * which is what a throw site looks like. Every quote style is matched rather than only the + * backtick: a `throw new QueryError("A Trino container path is ...")` written with double + * quotes is the same defect, and a backtick-only matcher reads it as clean. + */ +function renderingLines(source: string): string[] { + return codeLines(source) + .map((line, index) => ({ line, number: index + 1 })) + .filter(({ line }) => /["'`][^"'`]*container path is/.test(line)) + .map(({ line, number }) => `${number}: ${line.trim()}`); +} + +/** + * The fifteen provider files that reach the renderer, pinned by name. + * + * A floor (`>= 14`) stood here first, and a floor cannot see the thing it exists to notice: + * an engine that stops reaching the renderer takes the count DOWN, and 14 still passes for + * a fifteen-file population minus one. Naming the files makes a departure fail with the + * file in the message, and a sixteenth arriving as a deliberate edit here rather than as a + * silent count change. + */ +const ENGINES = [ + "src/lib/db/providers/document/couchbase/objects.ts", + "src/lib/db/providers/document/mongodb.ts", + "src/lib/db/providers/embedded/libredb.ts", + "src/lib/db/providers/keyvalue/redis.ts", + "src/lib/db/providers/sql/cassandra/objects.ts", + "src/lib/db/providers/sql/clickhouse/objects.ts", + "src/lib/db/providers/sql/druid/objects.ts", + "src/lib/db/providers/sql/duckdb/objects.ts", + "src/lib/db/providers/sql/libsql/objects.ts", + "src/lib/db/providers/sql/mssql.ts", + "src/lib/db/providers/sql/mysql.ts", + "src/lib/db/providers/sql/oracle.ts", + "src/lib/db/providers/sql/postgres.ts", + "src/lib/db/providers/sql/sqlite.ts", + "src/lib/db/providers/sql/trino/objects.ts", +]; + +describe("the container-path sentence has one producer under providers/", () => { + const files = sourceFiles(PROVIDERS); + + test("no provider builds the sentence itself", () => { + const offenders: string[] = []; + for (const file of files) { + const rendered = renderingLines(fs.readFileSync(file, "utf8")); + if (rendered.length > 0) offenders.push(`${repoRelative(file)}\n ${rendered.join("\n ")}`); + } + expect(offenders).toEqual([]); + }); + + test("the shared renderer is the producer, and every engine reaches it", () => { + const kinds = fs.readFileSync(path.join(ROOT, "src/lib/db/object-kinds.ts"), "utf8"); + expect(kinds).toContain("export function assertContainerPathShape("); + + // One descriptor per engine that renders the sentence, which is what keeps an engine's + // own opening words and accepted depths travelling through the shared renderer instead + // of being re-typed at a call site. A new provider that hand-rolls its own message + // fails the test above; one that reaches the renderer without a descriptor fails here. + const callers = files.filter((file) => fs.readFileSync(file, "utf8").includes("assertContainerPathShape(")); + expect(callers.map(repoRelative).sort()).toEqual([...ENGINES].sort()); + + for (const file of callers) { + expect(fs.readFileSync(file, "utf8")).toMatch(/\w+_CONTAINER_PATH_ENGINE: ContainerPathShapeEngine = \{/); + } + }); + + test("no provider keeps its own shape-list helper", () => { + // The four private `shapeList()` copies are the reason this issue exists; three of them + // also served an OBJECT-path message, which the shared renderer does not cover, so the + // helper may stay for that use. What must not stay is a helper whose only job is the + // container sentence, which is what a `containerShapes()` next to it means. + const offenders: string[] = []; + for (const file of files) { + const source = fs.readFileSync(file, "utf8"); + if (/function containerShapes\s*\(/.test(source)) offenders.push(repoRelative(file)); + } + expect(offenders).toEqual([]); + }); +}); From 4530fcf42143c7fe5cf0ba01ed5dc700fafa762d Mon Sep 17 00:00:00 2001 From: cevheri Date: Sun, 27 Sep 2026 04:42:20 +0300 Subject: [PATCH 2/2] test(db): read the container-path detector off the AST, tidy the imports The line scanner stripped `//` without knowing about strings, so a throw carrying a URL before the phrase read as clean. The detector now walks the string and template nodes of the parsed file, the way the seam guards read a provider, so comments never reach it. Also drops the blank line the descriptor move left inside seven import blocks, and a double space in Oracle's descriptor docblock. --- .../providers/document/couchbase/objects.ts | 1 - src/lib/db/providers/document/mongodb.ts | 1 - src/lib/db/providers/sql/cassandra/objects.ts | 1 - .../db/providers/sql/clickhouse/objects.ts | 1 - src/lib/db/providers/sql/druid/objects.ts | 1 - src/lib/db/providers/sql/libsql/objects.ts | 1 - src/lib/db/providers/sql/oracle.ts | 2 +- src/lib/db/providers/sql/trino/objects.ts | 1 - tests/unit/db/container-path-renderer.test.ts | 78 ++++++------------- 9 files changed, 24 insertions(+), 63 deletions(-) diff --git a/src/lib/db/providers/document/couchbase/objects.ts b/src/lib/db/providers/document/couchbase/objects.ts index 1480fca7b..1c7d4eed5 100644 --- a/src/lib/db/providers/document/couchbase/objects.ts +++ b/src/lib/db/providers/document/couchbase/objects.ts @@ -114,7 +114,6 @@ import { type ContainerPathShapeEngine, type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; - import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, diff --git a/src/lib/db/providers/document/mongodb.ts b/src/lib/db/providers/document/mongodb.ts index cc8b9376e..36222d4ae 100644 --- a/src/lib/db/providers/document/mongodb.ts +++ b/src/lib/db/providers/document/mongodb.ts @@ -57,7 +57,6 @@ import { type ContainerPathShapeEngine, type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; - import { comparePaths } from "@/lib/db/object-path"; import { DatabaseConfigError, ConnectionError, QueryError, mapDatabaseError } from "../../errors"; import { formatBytes } from "../../utils/pool-manager"; diff --git a/src/lib/db/providers/sql/cassandra/objects.ts b/src/lib/db/providers/sql/cassandra/objects.ts index 9f91e91e4..1cfdc164d 100644 --- a/src/lib/db/providers/sql/cassandra/objects.ts +++ b/src/lib/db/providers/sql/cassandra/objects.ts @@ -107,7 +107,6 @@ import { type ContainerPathShapeEngine, type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; - import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, diff --git a/src/lib/db/providers/sql/clickhouse/objects.ts b/src/lib/db/providers/sql/clickhouse/objects.ts index 4bd4f90dd..137bbb592 100644 --- a/src/lib/db/providers/sql/clickhouse/objects.ts +++ b/src/lib/db/providers/sql/clickhouse/objects.ts @@ -71,7 +71,6 @@ import { type ContainerPathShapeEngine, type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; - import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, diff --git a/src/lib/db/providers/sql/druid/objects.ts b/src/lib/db/providers/sql/druid/objects.ts index 764c07373..09e7e32c4 100644 --- a/src/lib/db/providers/sql/druid/objects.ts +++ b/src/lib/db/providers/sql/druid/objects.ts @@ -82,7 +82,6 @@ import { type ContainerPathShapeEngine, type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; - import { comparePaths } from "@/lib/db/object-path"; import type { Container, diff --git a/src/lib/db/providers/sql/libsql/objects.ts b/src/lib/db/providers/sql/libsql/objects.ts index 210e80885..af5a77fc0 100644 --- a/src/lib/db/providers/sql/libsql/objects.ts +++ b/src/lib/db/providers/sql/libsql/objects.ts @@ -52,7 +52,6 @@ import { type ContainerPathShapeEngine, type ObjectPathShapeEngine, } from "@/lib/db/object-kinds"; - import { comparePaths } from "@/lib/db/object-path"; import { unquoteLiteral } from "@/lib/sql/values"; import { readNumber, readText } from "./introspect"; diff --git a/src/lib/db/providers/sql/oracle.ts b/src/lib/db/providers/sql/oracle.ts index a0b13e384..cea0297ce 100644 --- a/src/lib/db/providers/sql/oracle.ts +++ b/src/lib/db/providers/sql/oracle.ts @@ -66,7 +66,7 @@ import { readStatementEnd } from "@/lib/sql/statement-end"; import { CACHE_HIT_RATIO_UNAVAILABLE, formatCacheHitRatio, measuredNumber } from "@/lib/monitoring-cache-ratio"; /** - * Oracle's identity for the shared container-path renderer. + * Oracle's identity for the shared container-path renderer. * * `shapes: "exact"`: the caller either names every declared level or is refused, * because a partial path would leave a level unbound and answer an empty folder. diff --git a/src/lib/db/providers/sql/trino/objects.ts b/src/lib/db/providers/sql/trino/objects.ts index 81c90abb6..5a52a51c0 100644 --- a/src/lib/db/providers/sql/trino/objects.ts +++ b/src/lib/db/providers/sql/trino/objects.ts @@ -65,7 +65,6 @@ import { QueryError } from "@/lib/db/errors"; import { assertContainerPathShape, containerDepth, type ContainerPathShapeEngine } from "@/lib/db/object-kinds"; - import { comparePaths } from "@/lib/db/object-path"; import type { ColumnSchema, diff --git a/tests/unit/db/container-path-renderer.test.ts b/tests/unit/db/container-path-renderer.test.ts index a4539d1e0..5afb87153 100644 --- a/tests/unit/db/container-path-renderer.test.ts +++ b/tests/unit/db/container-path-renderer.test.ts @@ -1,6 +1,7 @@ import { describe, test, expect } from "bun:test"; import * as fs from "fs"; import * as path from "path"; +import ts from "typescript"; /** * The container-path sentence has one producer under `src/lib/db/providers`. @@ -39,65 +40,32 @@ function sourceFiles(dir: string): string[] { } /** - * The source with comments removed, one entry per line, so line numbers survive. + * Every line of CODE that RENDERS the sentence, rather than one that talks about it. * * Prose quotes the phrase on purpose (a dozen docblocks explain why a path is refused), and - * a blanket grep would fail on the documentation that records the fix. So both comment - * forms are removed first: everything after a `//` on its line, and any block comment - * wherever it opens, including a docblock whose continuation lines carry no marker of their - * own. Only what remains is code. + * a blanket grep would fail on the documentation that records the fix. So the file is + * parsed rather than scanned, the way the seam guards read a provider: comments are trivia + * and never reach the walk, and every string the code carries is a node, whatever its quote + * style. A `throw new QueryError("A Trino container path is ...")` written with double + * quotes is the same defect as a template, and a line scanner that strips `//` without + * knowing about strings reads one that carries a URL before the phrase as clean. */ -function codeLines(source: string): string[] { - const lines: string[] = []; - let inBlock = false; - for (const raw of source.split("\n")) { - let code = ""; - let index = 0; - while (index < raw.length) { - if (inBlock) { - const close = raw.indexOf("*/", index); - if (close < 0) { - index = raw.length; - continue; - } - inBlock = false; - index = close + 2; - continue; - } - const open = raw.indexOf("/*", index); - const line = raw.indexOf("//", index); - if (line >= 0 && (open < 0 || line < open)) { - code += raw.slice(index, line); - index = raw.length; - continue; - } - if (open < 0) { - code += raw.slice(index); - index = raw.length; - continue; - } - code += raw.slice(index, open); - inBlock = true; - index = open + 2; +function renderingLines(file: string, source: string): string[] { + const sourceFile = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const lines = source.split("\n"); + // One entry per line: the two arms of a conditional that both carry the phrase would + // otherwise report their line twice, which reads like two throw sites. + const found = new Set(); + + const visit = (node: ts.Node): void => { + if ((ts.isStringLiteral(node) || ts.isTemplateLiteralToken(node)) && node.text.includes("container path is")) { + found.add(sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line); } - lines.push(code); - } - return lines; -} + ts.forEachChild(node, visit); + }; -/** - * Every line of CODE that RENDERS the sentence, rather than one that talks about it. - * - * Where the phrase survives the comment strip it can only sit inside a string literal, - * which is what a throw site looks like. Every quote style is matched rather than only the - * backtick: a `throw new QueryError("A Trino container path is ...")` written with double - * quotes is the same defect, and a backtick-only matcher reads it as clean. - */ -function renderingLines(source: string): string[] { - return codeLines(source) - .map((line, index) => ({ line, number: index + 1 })) - .filter(({ line }) => /["'`][^"'`]*container path is/.test(line)) - .map(({ line, number }) => `${number}: ${line.trim()}`); + visit(sourceFile); + return [...found].sort((a, b) => a - b).map((line) => `${line + 1}: ${lines[line].trim()}`); } /** @@ -133,7 +101,7 @@ describe("the container-path sentence has one producer under providers/", () => test("no provider builds the sentence itself", () => { const offenders: string[] = []; for (const file of files) { - const rendered = renderingLines(fs.readFileSync(file, "utf8")); + const rendered = renderingLines(file, fs.readFileSync(file, "utf8")); if (rendered.length > 0) offenders.push(`${repoRelative(file)}\n ${rendered.join("\n ")}`); } expect(offenders).toEqual([]);