diff --git a/.gitignore b/.gitignore index 9ad704285..6ca48cdfe 100644 --- a/.gitignore +++ b/.gitignore @@ -51,3 +51,4 @@ package-lock.json .vite-custom docs/superpowers pnpm-lock.yaml +.worktrees diff --git a/packages/core/src/stores/store.spec.ts b/packages/core/src/stores/store.spec.ts index 24ac91552..db151016a 100644 --- a/packages/core/src/stores/store.spec.ts +++ b/packages/core/src/stores/store.spec.ts @@ -1,12 +1,12 @@ -import { test } from "@webda/test"; +import { suite, test } from "@webda/test"; import * as assert from "assert"; import { stub } from "sinon"; import { randomUUID } from "crypto"; import { TestIdent } from "../test/objects.js"; -import { Ident, OperationContext, Store, User } from "../index.js"; +import { Ident, MemoryStore, OperationContext, Store, User } from "../index.js"; import { CoreModel } from "../models/coremodel.js"; import { WebdaApplicationTest } from "../test/application.js"; -import { StoreEvents, StoreNotFoundError, UpdateConditionFailError } from "./store.js"; +import { StoreEvents, StoreNotFoundError, StoreParameters, UpdateConditionFailError } from "./store.js"; import { UuidModel } from "@webda/models"; /** @@ -653,4 +653,101 @@ abstract class StoreTest> extends WebdaApplicationTest { } } +@suite +class StoreParametersTest { + @test + acceptsModelsArrayOnly() { + const p = new StoreParameters().load({ models: ["MyApp/User", "MyApp/Task"] }); + assert.deepStrictEqual(p.models, ["MyApp/User", "MyApp/Task"]); + } + + @test + mapsLegacyModelToModelsArray() { + const p = new StoreParameters().load({ model: "MyApp/User" }); + assert.deepStrictEqual(p.models, ["MyApp/User"]); + } + + @test + mapsModelPlusAdditionalToFlatArray() { + const p = new StoreParameters().load({ + model: "MyApp/User", + additionalModels: ["MyApp/Task", "MyApp/Order"] + }); + assert.deepStrictEqual(p.models, ["MyApp/User", "MyApp/Task", "MyApp/Order"]); + } + + @test + throwsWhenBothModelAndModelsSet() { + assert.throws( + () => new StoreParameters().load({ model: "MyApp/User", models: ["MyApp/User"] }), + /ambiguous/i + ); + } + + @test + defaultsToRegistryEntryWhenNeitherSet() { + const p = new StoreParameters().load({}); + assert.deepStrictEqual(p.models, ["Webda/RegistryEntry"]); + } + + @test + mapsAdditionalModelsAloneToRegistryFallback() { + const p = new StoreParameters().load({ additionalModels: ["MyApp/Task"] }); + assert.deepStrictEqual(p.models, ["Webda/RegistryEntry", "MyApp/Task"]); + } + + @test + throwsWhenModelsCombinedWithEmptyAdditionalModels() { + assert.throws( + () => new StoreParameters().load({ models: ["X"], additionalModels: [] }), + /ambiguous/i + ); + } +} + +@suite +class StoreFieldsMigrationTest extends WebdaApplicationTest { + @test + async populatesModelsArrayAndMetadatas() { + const store = new MemoryStore("multi", { models: ["Webda/Ident", "Webda/User"] }); + // filterParameters strips unknown fields from the pre-schema-regen module; set models directly + // to exercise computeParameters() via the canonical models[] path (not the legacy shim). + store.getParameters().models = ["Webda/Ident", "Webda/User"]; + store.resolve(); + assert.strictEqual((store as any)._models.length, 2); + assert.strictEqual((store as any)._modelMetadatas.size, 2); + assert.strictEqual((store as any)._modelsHierarchy["Webda/Ident"], 0); + assert.strictEqual((store as any)._modelsHierarchy["Webda/User"], 0); + assert.strictEqual(store.getModels().length, 2); + } + + @test + async getModelReturnsFirstForBackCompat() { + const store = new MemoryStore("primaryModel", { model: "Webda/User" }); + store.resolve(); + assert.strictEqual(store.getModel()?.name, "User"); + } + + @test + async walksSubclassHierarchyForNonStrictStore() { + // Webda/User has Webda/SimpleUser as a subclass (verified via webda.module.json). + // In non-strict mode the recursive walk should add the subclass at depth 1. + const store = new MemoryStore("hierarchical", { models: ["Webda/User"] }); + store.getParameters().models = ["Webda/User"]; + store.resolve(); + assert.strictEqual((store as any)._modelsHierarchy["Webda/User"], 0); + assert.strictEqual((store as any)._modelsHierarchy["Webda/SimpleUser"], 1); + } + + @test + async strictStoreSkipsSubclassWalk() { + // Same Webda/User parent, but strict: true should not add SimpleUser. + const store = new MemoryStore("strict", { models: ["Webda/User"], strict: true }); + store.getParameters().models = ["Webda/User"]; + store.resolve(); + assert.strictEqual((store as any)._modelsHierarchy["Webda/User"], 0); + assert.strictEqual((store as any)._modelsHierarchy["Webda/SimpleUser"], undefined); + } +} + export { StoreTest }; diff --git a/packages/core/src/stores/store.ts b/packages/core/src/stores/store.ts index e2c632b87..5bebeb710 100644 --- a/packages/core/src/stores/store.ts +++ b/packages/core/src/stores/store.ts @@ -218,17 +218,21 @@ export interface StoreInterface { */ export class StoreParameters extends ServiceParameters { /** - * Webda model to use within the Store + * Models managed by this Store. * - * @default "Webda/CoreModel" + * @default ["Webda/RegistryEntry"] + */ + models?: string[]; + + /** + * Webda model to use within the Store. + * @deprecated Use `models: [model]` instead. Will be removed in 5.x. */ model?: string; + /** - * Additional models - * - * Allow this store to manage other models - * - * @default [] + * Additional models managed by this Store. + * @deprecated Merge into `models[]` instead. Will be removed in 5.x. */ additionalModels?: string[]; @@ -281,13 +285,47 @@ export class StoreParameters extends ServiceParameters { } // END_REFACTOR super.load(params); - this.model ??= "Webda/RegistryEntry"; + + // Deprecation mapping: model + additionalModels → models[] + const hasModels = Array.isArray(params.models); + const hasModel = typeof params.model === "string"; + // Presence check (not length) used for the conflict guard so that + // `{ models: [...], additionalModels: [] }` is still detected as ambiguous. + const hasAdditionalKey = Array.isArray(params.additionalModels); + // Length check used for the deprecation branch: an empty additionalModels + // alongside a bare `model` is unambiguous and should not warn on its own. + const hasAdditional = hasAdditionalKey && params.additionalModels.length > 0; + + if (hasModels && (hasModel || hasAdditionalKey)) { + throw new Error( + "StoreParameters: `models` and `model`/`additionalModels` are mutually exclusive — ambiguous configuration" + ); + } + + if (hasModels) { + this.models = [...params.models]; + } else if (hasModel || hasAdditional) { + useLog( + "WARN", + `StoreParameters: \`model\` and \`additionalModels\` are deprecated. Use \`models: [...]\` instead.` + ); + this.models = [params.model ?? "Webda/RegistryEntry", ...(params.additionalModels ?? [])]; + } else { + this.models = ["Webda/RegistryEntry"]; + } + + // When the caller passed `models[]` (or nothing at all), backfill the deprecated + // fields from the canonical `models[]` so any code still reading `parameters.model` + // or `parameters.additionalModels` keeps working. In the legacy-input branch + // `super.load(params)` already set both via Object.assign, so the `??=` is a no-op. + this.model ??= this.models[0]; + this.additionalModels ??= this.models.slice(1); + this.strict ??= false; this.defaultModel ??= true; this.forceModel ??= false; this.slowQueryThreshold ??= 30000; this.modelAliases ??= {}; - this.additionalModels ??= []; return this; } } @@ -320,22 +358,12 @@ abstract class Store { - /** - * Contains the current model - */ - _model: ModelClass; - /** - * Contains the current model metadata - */ - _modelMetadata: ModelMetadata; - /** - * Store the manager hierarchy with their depth - */ + /** All model classes managed by this store. Index 0 is the legacy "primary" model. */ + _models: ModelClass[] = []; + /** Metadata for each model in `_models`, keyed by Identifier. */ + _modelMetadatas: Map = new Map(); + /** Store the manager hierarchy with their depth */ _modelsHierarchy: { [key: string]: number } = {}; - /** - * Contains the current model type - */ - _modelType: string; /** * Override the resolved model class for this store at runtime. Used by @@ -345,7 +373,7 @@ abstract class Store 0; - if (isDefaultModel && !hasAdditional) { + const ids = this.parameters.models ?? []; + // Empty / default RegistryEntry → leave hierarchy empty (preserves test-only / fallback behaviour). + const isOnlyDefault = ids.length === 1 && ids[0] === "Webda/RegistryEntry"; + if (ids.length === 0 || isOnlyDefault) { return; } - // Guard: useModel throws on undefined and may return undefined/null for - // unknown models. Catch both so test-only or misconfigured stores stay - // harmless. - try { - this._model = useModel(this.parameters.model); - } catch { - this._model = undefined; - } - if (!this._model) { - useLog("TRACE", `Store ${this.getName?.() ?? "unknown"}: model not found: ${this.parameters.model}`); - return; - } - this._modelMetadata = useModelMetadata(this._model); - if (!this._modelMetadata) { - useLog("WARN", `Store ${this.getName?.() ?? "unknown"}: model metadata not found for ${this.parameters.model}`); - return; - } - useLog("TRACE", "METADATA", this._modelMetadata); - this._modelType = this._modelMetadata.Identifier; - - // Recursively populate _modelsHierarchy for a model's subclass tree. - // Each subclass identifier in meta.Subclasses is a string that we resolve - // via useModel. We keep the minimum depth seen for each identifier. - const recursive = (subclassIds: string[], depth: number) => { - for (const id of subclassIds) { - this._modelsHierarchy[id] = Math.min(depth, this._modelsHierarchy[id] ?? depth); + this._models = []; + this._modelMetadatas = new Map(); + this._modelsHierarchy = {}; + + // Subclasses in ModelMetadata are resolved to ModelClass objects at runtime + // (see Application.setModelMetadata). Accept both string IDs and ModelClass + // references so the hierarchy is keyed by string identifier throughout. + const recursive = (subclasses: (string | ModelClass)[], depth: number) => { + for (const entry of subclasses) { let subModel: ModelClass | undefined; - try { - subModel = useModel(id); - } catch { - continue; + let subId: string | undefined; + if (typeof entry === "string") { + subId = entry; + try { subModel = useModel(entry); } catch { continue; } + } else { + subModel = entry as ModelClass; + subId = useModelId(subModel); } - if (!subModel) continue; + if (!subId || !subModel) continue; + this._modelsHierarchy[subId] = Math.min(depth, this._modelsHierarchy[subId] ?? depth); const subMeta = useModelMetadata(subModel); if (!subMeta) continue; recursive(subMeta.Subclasses ?? [], depth + 1); } }; - // Compute the hierarchy — reset first so re-resolve is idempotent - this._modelsHierarchy = {}; - this._modelsHierarchy[this._modelMetadata.Identifier] = 0; - // Strict Store only stores their exact model - if (!this.parameters.strict) { - recursive(this._modelMetadata.Subclasses ?? [], 1); - } - // Add additional models (each treated as depth-0 roots with their own subtree) - if ((this.parameters.additionalModels ?? []).length) { - if (this.parameters.strict) { - useLog("ERROR", "Cannot add additional models in strict mode"); - } else { - for (const modelType of this.parameters.additionalModels!) { - let addModel: ModelClass | undefined; - try { - addModel = useModel(modelType); - } catch { - continue; - } - if (!addModel) continue; - const addMeta = useModelMetadata(addModel); - if (!addMeta) continue; - this._modelsHierarchy[addMeta.Identifier] = 0; - recursive(addMeta.Subclasses ?? [], 1); - } + for (const id of ids) { + let model: ModelClass | undefined; + try { + model = useModel(id); + } catch { + model = undefined; + } + if (!model) { + useLog("TRACE", `Store ${this.getName?.() ?? "unknown"}: model not found: ${id}`); + continue; + } + const meta = useModelMetadata(model); + if (!meta) { + useLog("WARN", `Store ${this.getName?.() ?? "unknown"}: model metadata not found for ${id}`); + continue; + } + this._models.push(model); + this._modelMetadatas.set(meta.Identifier, meta); + this._modelsHierarchy[meta.Identifier] = 0; + if (!this.parameters.strict) { + recursive(meta.Subclasses ?? [], 1); } } } @@ -542,11 +548,20 @@ abstract class Store> { usersStore.computeParameters(); assert.ok(existsSync(usersStore.getParameters().folder)); } + + @test + async strictModeRejectsForeignType() { + const tmpFolder = join(tmpdir(), `webda-fs-strict-reject-${Date.now()}`); + mkdirSync(tmpFolder); + try { + const store: FileStore = await this.addService( + FileStore, + { folder: tmpFolder, models: ["Webda/Ident"], strict: true }, + "StrictRejectStore" + ); + // Write a file directly with a __type that is NOT the configured model. + const uid = "strict-reject-test-uid"; + writeFileSync(join(tmpFolder, `${uid}.json`), JSON.stringify({ __type: "Webda/User", uuid: uid })); + // _get should return undefined because Webda/User is not at depth 0 in _modelsHierarchy + const result = await store["_get"](uid, false); + assert.strictEqual(result, undefined, "strict mode should reject a file whose __type is not the configured model"); + } finally { + rmSync(tmpFolder, { recursive: true, force: true }); + } + } + + @test + async strictModeAcceptsConfiguredType() { + const tmpFolder = join(tmpdir(), `webda-fs-strict-accept-${Date.now()}`); + mkdirSync(tmpFolder); + try { + const store: FileStore = await this.addService( + FileStore, + { folder: tmpFolder, models: ["Webda/Ident"], strict: true }, + "StrictAcceptStore" + ); + // Write a file directly with the matching __type. + const uid = "strict-accept-test-uid"; + writeFileSync(join(tmpFolder, `${uid}.json`), JSON.stringify({ __type: "Webda/Ident", uuid: uid })); + // _get should return the data because Webda/Ident is at depth 0 in _modelsHierarchy + const result = await store["_get"](uid, false); + assert.ok(result !== undefined, "strict mode should accept a file whose __type matches the configured model"); + assert.strictEqual(result.__type, "Webda/Ident"); + } finally { + rmSync(tmpFolder, { recursive: true, force: true }); + } + } } export { FileStoreTest }; diff --git a/packages/fs/src/filestore.ts b/packages/fs/src/filestore.ts index d96f349ea..11c9ff3d1 100644 --- a/packages/fs/src/filestore.ts +++ b/packages/fs/src/filestore.ts @@ -202,7 +202,7 @@ export class FileStore exte .sort(); // Use the repository for this store's model to simulate a find - const repo = this.getRepository(this._model) as MemoryRepository; + const repo = this.getRepository(this._models[0]) as MemoryRepository; return MemoryRepository.simulateFind(query as any, files, repo as any); } @@ -389,7 +389,7 @@ export class FileStore exte const res = await this._exists(uid); if (res) { const data = JSON.parse(fs.readFileSync(this.file(uid)).toString()); - if (data.__type !== this._modelType && this.parameters.strict) { + if (this.parameters.strict && this._modelsHierarchy[data.__type] !== 0) { return undefined; } return data; diff --git a/packages/postgres/src/postgresstore.spec.ts b/packages/postgres/src/postgresstore.spec.ts index 5920ec53e..a4b73f7c7 100644 --- a/packages/postgres/src/postgresstore.spec.ts +++ b/packages/postgres/src/postgresstore.spec.ts @@ -2,6 +2,7 @@ import { suite, test } from "@webda/test"; import * as assert from "node:assert"; import pg from "pg"; import { WebdaApplicationTest } from "@webda/core/lib/test"; +import { useModel } from "@webda/core"; import PostgresStore from "./postgresstore.js"; const params = { @@ -147,4 +148,43 @@ export class PostgresStoreSmokeTest extends WebdaApplicationTest { this.store!.getParameters().views = [".*"]; this.store!.getParameters().viewPrefix = ""; } + +} + +/** + * Unit tests for resolveTable() that do not require a live PostgreSQL connection. + * Extends WebdaApplicationTest only for its model registry (useModel / useModelMetadata), + * but does NOT call addService() — so the DB-connect path is never triggered. + */ +@suite +export class PostgresStoreResolveTableTest extends WebdaApplicationTest { + @test + async resolveTableSingleModelUsesParametersTable() { + const store = new PostgresStore("singleTable", { models: ["Webda/Ident"], table: "idents" }); + // Bypass resolve()/init() — we only test the table-name resolution logic. + // Set parameters.models directly (schema workaround: Task 7 regenerates it). + store.getParameters().models = ["Webda/Ident"]; + assert.strictEqual(store.resolveTable(useModel("Webda/Ident")), "idents"); + } + + @test + async resolveTableMultiModelIgnoresParametersTable() { + const store = new PostgresStore("multiTable", { + models: ["Webda/Ident", "Webda/User"], + table: "idents" + }); + store.getParameters().models = ["Webda/Ident", "Webda/User"]; + assert.strictEqual(store.resolveTable(useModel("Webda/Ident")), "webda_ident"); + assert.strictEqual(store.resolveTable(useModel("Webda/User")), "webda_user"); + } + + @test + async resolveTableExplicitTablesMapWins() { + const store = new PostgresStore("explicitTable", { + models: ["Webda/User"], + tables: { "Webda/User": "users_v2" } + }); + store.getParameters().models = ["Webda/User"]; + assert.strictEqual(store.resolveTable(useModel("Webda/User")), "users_v2"); + } } diff --git a/packages/postgres/src/postgresstore.ts b/packages/postgres/src/postgresstore.ts index 3859a3a5e..a82006456 100644 --- a/packages/postgres/src/postgresstore.ts +++ b/packages/postgres/src/postgresstore.ts @@ -101,6 +101,12 @@ export class PostgresStore ex * @override */ async init(): Promise { + if (this.parameters.table && (this.parameters.models?.length ?? 0) > 1) { + useLog( + "WARN", + `PostgresStore "${this.getName()}": parameters.table is ignored when more than one model is configured. Use parameters.tables[id] instead.` + ); + } if (this.parameters.usePool) { this.client = acquirePool(this.parameters.postgresqlServer as PoolConfig); this.ownsPool = true; @@ -134,7 +140,7 @@ export class PostgresStore ex * * Resolution order: * 1. `parameters.tables[meta.Identifier]` — explicit per-model override - * 2. Primary model (matching `_modelMetadata.Identifier`) → `parameters.table` + * 2. Single-model back-compat: when only one model is configured, `parameters.table` maps to it * 3. Default — model identifier lowercased with "/" replaced by "_" * * @param model - the model class to resolve the table for @@ -149,11 +155,11 @@ export class PostgresStore ex if (this.parameters.tables?.[meta.Identifier]) { return this.parameters.tables[meta.Identifier]; } - // Primary model uses the configured table name - if (this._modelMetadata && meta.Identifier === this._modelMetadata.Identifier) { + // Single-model back-compat: parameters.table maps to the one model + if (this.parameters.models?.length === 1 && this.parameters.table) { return this.parameters.table; } - // Default: identifier lowercased, "/" → "_" + // Default derivation return meta.Identifier.toLowerCase().replace(/\//g, "_"); } diff --git a/packages/runtime/src/services/invitationservice.spec.ts b/packages/runtime/src/services/invitationservice.spec.ts index 194402968..f6d94b5dd 100644 --- a/packages/runtime/src/services/invitationservice.spec.ts +++ b/packages/runtime/src/services/invitationservice.spec.ts @@ -97,7 +97,7 @@ class InvitationTest extends WebdaInternalTest { await super.before(); this.store = this.webda.getService("Companies"); - this.store._model = MyCompany; + this.store.setModelDefinitionHelper(MyCompany); this.service = await this.addService( InvitationService,