From 8a7ba67b030a80aadae365aed77664797dfa4791 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 14 Sep 2026 11:41:05 +0000 Subject: [PATCH 1/4] feat(database): add Compatible indexes for Mongo and SQL dialects Make schema indexes first-class on Mongo, PostgreSQL, MySQL, MariaDB, and SQLite. Platform models use Compatible Ascending/Descending so the same declarations create live indexes on every dialect. Admin create/get/delete/import/export persist into _DeclaredSchema without a full schema rebuild. --- libraries/grpc-sdk/src/interfaces/Model.ts | 24 +- .../src/models/ActorIndex.schema.ts | 7 +- .../src/models/ObjectIndex.schema.ts | 23 +- .../src/models/Permission.schema.ts | 8 +- .../src/models/Relationship.schema.ts | 8 +- modules/chat/src/models/ChatRoom.schema.ts | 9 +- modules/chat/src/models/Message.schema.ts | 17 +- .../src/__tests__/no-old-pr-bugs.test.ts | 36 ++ .../__tests__/platform-models.indexes.test.ts | 33 ++ .../database/src/adapters/DatabaseAdapter.ts | 3 +- .../mongoose-adapter/SchemaConverter.ts | 65 +++- .../__tests__/SchemaConverter.indexes.test.ts | 66 ++++ .../__tests__/indexes.adapter.test.ts | 123 +++++++ .../src/adapters/mongoose-adapter/index.ts | 163 ++++++--- .../__tests__/indexes.adapter.test.ts | 128 +++++++ .../src/adapters/sequelize-adapter/index.ts | 183 +++++++--- .../postgres-adapter/PgSchemaConverter.ts | 4 +- .../sql-adapter/SqlSchemaConverter.ts | 9 +- .../adapters/utils/__tests__/indexes.test.ts | 198 +++++++++++ .../__tests__/sql-index-converters.test.ts | 125 +++++++ .../utils/database-transform-utils.ts | 158 ++++++--- modules/database/src/adapters/utils/index.ts | 1 + .../database/src/adapters/utils/indexes.ts | 329 ++++++++++++++++++ .../__tests__/schema.admin.indexes.test.ts | 129 +++++++ modules/database/src/admin/index.ts | 29 ++ modules/database/src/admin/schema.admin.ts | 93 ++++- .../validateModelOptions.indexes.test.ts | 45 +++ modules/database/src/utils/utilities.ts | 6 +- 28 files changed, 1811 insertions(+), 211 deletions(-) create mode 100644 modules/database/src/__tests__/no-old-pr-bugs.test.ts create mode 100644 modules/database/src/__tests__/platform-models.indexes.test.ts create mode 100644 modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts create mode 100644 modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts create mode 100644 modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts create mode 100644 modules/database/src/adapters/utils/__tests__/indexes.test.ts create mode 100644 modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts create mode 100644 modules/database/src/adapters/utils/indexes.ts create mode 100644 modules/database/src/admin/__tests__/schema.admin.indexes.test.ts create mode 100644 modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts diff --git a/libraries/grpc-sdk/src/interfaces/Model.ts b/libraries/grpc-sdk/src/interfaces/Model.ts index 6b7319a9b..f5600386c 100644 --- a/libraries/grpc-sdk/src/interfaces/Model.ts +++ b/libraries/grpc-sdk/src/interfaces/Model.ts @@ -41,6 +41,20 @@ export enum PostgresIndexType { BRIN = 'BRIN', } +/** + * Portable ascending/descending indexes that exist on MongoDB and every SQL dialect. + * Map to Mongo 1/-1 and SQL BTREE ASC/DESC. Optional uniqueness lives on index options. + */ +export enum CompatibleIndexType { + Ascending = 'Ascending', + Descending = 'Descending', +} + +export type IndexType = MongoIndexType | PostgresIndexType | CompatibleIndexType; + +export type ModelOptionsIndexTypes = + MongoIndexType[] | PostgresIndexType | CompatibleIndexType | CompatibleIndexType[]; + export type Array = any[]; export interface ConduitStringValidation { @@ -71,9 +85,7 @@ export interface ConduitArrayValidation { } export type ConduitValidationRules = - | ConduitStringValidation - | ConduitNumberValidation - | ConduitArrayValidation; + ConduitStringValidation | ConduitNumberValidation | ConduitArrayValidation; type BaseConduitModelField = { type?: TYPE | TYPE[] | ConduitModel | ArrayConduitModel[]; @@ -198,7 +210,7 @@ export interface ConduitSchemaOptions { } export interface SchemaFieldIndex { - type?: MongoIndexType | PostgresIndexType; + type?: IndexType; options?: MongoIndexOptions | PostgresIndexOptions; [field: string]: any; @@ -206,8 +218,10 @@ export interface SchemaFieldIndex { export interface ModelOptionsIndexes { fields: string[] | readonly string[]; - types?: MongoIndexType[] | PostgresIndexType; + types?: ModelOptionsIndexTypes; options?: MongoIndexOptions | PostgresIndexOptions; + /** Optional. Generated deterministically when omitted. */ + name?: string; [field: string]: any; } diff --git a/modules/authorization/src/models/ActorIndex.schema.ts b/modules/authorization/src/models/ActorIndex.schema.ts index 2663dee2b..30cce8a83 100644 --- a/modules/authorization/src/models/ActorIndex.schema.ts +++ b/modules/authorization/src/models/ActorIndex.schema.ts @@ -1,8 +1,8 @@ import { + CompatibleIndexType, ConduitModel, ConduitSchemaOptions, DatabaseProvider, - MongoIndexType, TYPE, } from '@conduitplatform/grpc-sdk'; import { ConduitActiveSchema } from '@conduitplatform/module-tools'; @@ -23,7 +23,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, subjectId: { @@ -41,7 +41,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, entityId: { @@ -69,6 +69,7 @@ const schemaOptions: ConduitSchemaOptions = { indexes: [ { fields: ['subject', 'entity'], + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], }, ], conduit: { diff --git a/modules/authorization/src/models/ObjectIndex.schema.ts b/modules/authorization/src/models/ObjectIndex.schema.ts index 7ef08abd1..0366dd08a 100644 --- a/modules/authorization/src/models/ObjectIndex.schema.ts +++ b/modules/authorization/src/models/ObjectIndex.schema.ts @@ -1,8 +1,8 @@ import { + CompatibleIndexType, ConduitModel, ConduitSchemaOptions, DatabaseProvider, - MongoIndexType, TYPE, } from '@conduitplatform/grpc-sdk'; import { ConduitActiveSchema } from '@conduitplatform/module-tools'; @@ -23,7 +23,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, subjectId: { @@ -47,7 +47,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, entityId: { @@ -75,7 +75,7 @@ const schema: ConduitModel = { type: [TYPE.String], default: [], index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, createdAt: TYPE.Date, @@ -86,12 +86,27 @@ const schemaOptions: ConduitSchemaOptions = { indexes: [ { fields: ['subjectType', 'subjectPermission', 'entity'], + types: [ + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + ], }, { fields: ['entity', 'subjectType', 'subjectPermission'], + types: [ + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + ], }, { fields: ['entityType', 'entityId', 'entityPermission'], + types: [ + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + ], }, ], conduit: { diff --git a/modules/authorization/src/models/Permission.schema.ts b/modules/authorization/src/models/Permission.schema.ts index 76dd2e3a5..ad1502649 100644 --- a/modules/authorization/src/models/Permission.schema.ts +++ b/modules/authorization/src/models/Permission.schema.ts @@ -1,7 +1,7 @@ import { + CompatibleIndexType, ConduitModel, DatabaseProvider, - MongoIndexType, TYPE, } from '@conduitplatform/grpc-sdk'; import { ConduitActiveSchema } from '@conduitplatform/module-tools'; @@ -18,7 +18,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, resourceId: { @@ -37,7 +37,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, subjectId: { @@ -61,7 +61,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, options: { unique: true, }, diff --git a/modules/authorization/src/models/Relationship.schema.ts b/modules/authorization/src/models/Relationship.schema.ts index 4961f5813..88cff5fc7 100644 --- a/modules/authorization/src/models/Relationship.schema.ts +++ b/modules/authorization/src/models/Relationship.schema.ts @@ -1,7 +1,7 @@ import { + CompatibleIndexType, ConduitModel, DatabaseProvider, - MongoIndexType, TYPE, } from '@conduitplatform/grpc-sdk'; import { ConduitActiveSchema } from '@conduitplatform/module-tools'; @@ -17,7 +17,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, resourceId: { @@ -36,7 +36,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, }, }, // user:1adasdas @@ -61,7 +61,7 @@ const schema: ConduitModel = { type: TYPE.String, required: true, index: { - type: MongoIndexType.Ascending, + type: CompatibleIndexType.Ascending, options: { unique: true, }, diff --git a/modules/chat/src/models/ChatRoom.schema.ts b/modules/chat/src/models/ChatRoom.schema.ts index 6a7c10f61..92eae55e7 100644 --- a/modules/chat/src/models/ChatRoom.schema.ts +++ b/modules/chat/src/models/ChatRoom.schema.ts @@ -1,4 +1,5 @@ import { + CompatibleIndexType, ConduitModel, ConduitSchemaOptions, DatabaseProvider, @@ -40,7 +41,13 @@ const schema: ConduitModel = { }; const modelOptions: ConduitSchemaOptions = { timestamps: true, - indexes: [{ fields: ['participants'] }, { fields: ['participants', 'deleted'] }], + indexes: [ + { fields: ['participants'], types: [CompatibleIndexType.Ascending] }, + { + fields: ['participants', 'deleted'], + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ], conduit: { permissions: { extendable: true, diff --git a/modules/chat/src/models/Message.schema.ts b/modules/chat/src/models/Message.schema.ts index 7f5bf5546..752928a49 100644 --- a/modules/chat/src/models/Message.schema.ts +++ b/modules/chat/src/models/Message.schema.ts @@ -1,8 +1,9 @@ import { + CompatibleIndexType, ConduitModel, + ConduitSchemaOptions, DatabaseProvider, TYPE, - ConduitSchemaOptions, } from '@conduitplatform/grpc-sdk'; import { ConduitActiveSchema } from '@conduitplatform/module-tools'; import { ChatRoom } from './ChatRoom.schema.js'; @@ -53,8 +54,18 @@ const schema: ConduitModel = { const modelOptions: ConduitSchemaOptions = { timestamps: true, indexes: [ - { fields: ['room', 'createdAt'] }, - { fields: ['room', 'deleted', 'createdAt'] }, + { + fields: ['room', 'createdAt'], + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + { + fields: ['room', 'deleted', 'createdAt'], + types: [ + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + ], + }, ], conduit: { permissions: { diff --git a/modules/database/src/__tests__/no-old-pr-bugs.test.ts b/modules/database/src/__tests__/no-old-pr-bugs.test.ts new file mode 100644 index 000000000..68a155907 --- /dev/null +++ b/modules/database/src/__tests__/no-old-pr-bugs.test.ts @@ -0,0 +1,36 @@ +import { readFileSync } from 'fs'; +import { resolve } from 'path'; +import { describe, expect, it } from '@jest/globals'; + +describe('do not port old PR #643 bugs', () => { + it("does not use `case 'mysql' || 'mariadb'`", () => { + const files = [ + resolve(process.cwd(), 'src/adapters/utils/indexes.ts'), + resolve(process.cwd(), 'src/adapters/sequelize-adapter/index.ts'), + resolve(process.cwd(), 'src/adapters/utils/database-transform-utils.ts'), + ]; + for (const file of files) { + const source = readFileSync(file, 'utf8'); + expect(source).not.toMatch(/case ['"]mysql['"]\s*\|\|/); + } + }); + + it('does not rename getDatabaseType PostgreSQL to postgres', () => { + const source = readFileSync( + resolve(process.cwd(), 'src/adapters/sequelize-adapter/index.ts'), + 'utf8', + ); + expect(source).toContain("return 'PostgreSQL'"); + }); + + it('does not use metadata-only getIndexes or createSchemaFromAdapter rebuild for indexes', () => { + const sequelize = readFileSync( + resolve(process.cwd(), 'src/adapters/sequelize-adapter/index.ts'), + 'utf8', + ); + expect(sequelize).toContain('showIndex'); + expect(sequelize).toContain('addIndex'); + expect(sequelize).not.toMatch(/createIndexes[\s\S]*createSchemaFromAdapter/); + expect(sequelize).not.toMatch(/await this\.models\[schemaName\]\.sync\(\)/); + }); +}); diff --git a/modules/database/src/__tests__/platform-models.indexes.test.ts b/modules/database/src/__tests__/platform-models.indexes.test.ts new file mode 100644 index 000000000..e5baea9a5 --- /dev/null +++ b/modules/database/src/__tests__/platform-models.indexes.test.ts @@ -0,0 +1,33 @@ +import { existsSync, readFileSync } from 'fs'; +import { resolve } from 'path'; +import { describe, expect, it } from '@jest/globals'; + +function repoFile(...parts: string[]) { + const candidates = [ + resolve(process.cwd(), '..', ...parts), + resolve(process.cwd(), ...parts), + resolve(process.cwd(), '../..', ...parts), + ]; + const found = candidates.find(existsSync); + if (!found) throw new Error(`Missing ${parts.join('/')}`); + return found; +} + +const files = [ + repoFile('authorization', 'src', 'models', 'Permission.schema.ts'), + repoFile('authorization', 'src', 'models', 'Relationship.schema.ts'), + repoFile('authorization', 'src', 'models', 'ActorIndex.schema.ts'), + repoFile('authorization', 'src', 'models', 'ObjectIndex.schema.ts'), + repoFile('chat', 'src', 'models', 'ChatRoom.schema.ts'), + repoFile('chat', 'src', 'models', 'Message.schema.ts'), +]; + +describe('platform models T7 CompatibleIndexType', () => { + it('authz + chat schemas declare Compatible indexes, not Mongo-only types', () => { + for (const file of files) { + const source = readFileSync(file, 'utf8'); + expect(source).toContain('CompatibleIndexType'); + expect(source).not.toContain('MongoIndexType'); + } + }); +}); diff --git a/modules/database/src/adapters/DatabaseAdapter.ts b/modules/database/src/adapters/DatabaseAdapter.ts index 7e5f898c9..346c15ab3 100644 --- a/modules/database/src/adapters/DatabaseAdapter.ts +++ b/modules/database/src/adapters/DatabaseAdapter.ts @@ -198,8 +198,9 @@ export abstract class DatabaseAdapter { abstract createIndexes( schemaName: string, - indexes: ModelOptionsIndexes[], + indexes: readonly ModelOptionsIndexes[], callerModule: string, + options?: { privileged?: boolean }, ): Promise; abstract getIndexes(schemaName: string): Promise; diff --git a/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts b/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts index 81a995253..d44524917 100644 --- a/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts +++ b/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts @@ -1,5 +1,6 @@ import { Schema } from 'mongoose'; import { + ConduitGrpcSdk, ConduitModelField, ConduitSchema, ModelOptionsIndexes, @@ -8,6 +9,12 @@ import { } from '@conduitplatform/grpc-sdk'; import { cloneDeep, isArray, isNil, isObject } from 'lodash-es'; import { checkIfMongoOptions } from './utils.js'; +import { + isCompatibleIndexType, + isMongoIndexType, + mapCompatibleToMongo, + mongoAllowsIndexType, +} from '../utils/indexes.js'; import * as deepdash from 'deepdash-es/standalone'; @@ -103,12 +110,23 @@ function convertSchemaFieldIndexes(copy: ConduitSchema) { if (!index) continue; const type = index.type; const options = index.options; - if (type && !Object.values(MongoIndexType).includes(type)) { - throw new Error('Incorrect index type for MongoDB'); + if (type && !mongoAllowsIndexType(type)) { + ConduitGrpcSdk.Logger.warn( + `Invalid index type for MongoDB found in '${copy.name}', ignoring index`, + ); + delete (field[1] as ConduitModelField).index; + continue; + } + if (type && isCompatibleIndexType(type)) { + index.type = mapCompatibleToMongo(type); } if (options) { if (!checkIfMongoOptions(options)) { - throw new Error('Incorrect index options for MongoDB'); + ConduitGrpcSdk.Logger.warn( + `Invalid index options for MongoDB found in '${copy.name}', ignoring index`, + ); + delete (field[1] as ConduitModelField).index; + continue; } for (const [option, optionValue] of Object.entries(options)) { index[option as keyof SchemaFieldIndex] = optionValue; @@ -122,31 +140,48 @@ function convertSchemaFieldIndexes(copy: ConduitSchema) { function convertModelOptionsIndexes(copy: ConduitSchema) { if (!copy.modelOptions.indexes?.length) return copy; const mutIndexes = copy.modelOptions.indexes as ModelOptionsIndexes[]; - for (const index of mutIndexes) { + for (const index of [...mutIndexes]) { + if (index.types) { + const types = isArray(index.types) + ? index.types + : index.fields.map(() => index.types); + if ( + types.some(type => !mongoAllowsIndexType(type)) || + (isArray(index.types) && index.fields.length !== index.types.length) + ) { + ConduitGrpcSdk.Logger.warn( + `Invalid index type for MongoDB found in '${copy.name}', ignoring index`, + ); + mutIndexes.splice(mutIndexes.indexOf(index), 1); + continue; + } + index.types = types.map(type => + isCompatibleIndexType(type) || isMongoIndexType(type) + ? mapCompatibleToMongo(type) + : (type as MongoIndexType), + ) as MongoIndexType[]; + } // compound indexes are maintained in modelOptions in order to be created after schema creation // single field index => add it to specified schema field if (index.fields.length !== 1) continue; const modelField = copy.fields[index.fields[0]] as ConduitModelField; if (!modelField) { - throw new Error(`Field ${modelField} in index definition doesn't exist`); + throw new Error(`Field ${index.fields[0]} in index definition doesn't exist`); } if (index.types) { - if ( - !isArray(index.types) || - !Object.values(MongoIndexType).includes(index.types[0]) || - index.fields.length !== index.types.length - ) { - throw new Error('Invalid index type for MongoDB'); - } - const type = index.types[0] as MongoIndexType; modelField.index = { - type: type, + type: (index.types as MongoIndexType[])[0], }; } if (index.options) { if (!checkIfMongoOptions(index.options)) { - throw new Error('Incorrect index options for MongoDB'); + ConduitGrpcSdk.Logger.warn( + `Invalid index options for MongoDB found in '${copy.name}', ignoring index`, + ); + mutIndexes.splice(mutIndexes.indexOf(index), 1); + continue; } + if (!modelField.index) modelField.index = {}; for (const [option, optionValue] of Object.entries(index.options)) { modelField.index![option as keyof SchemaFieldIndex] = optionValue; } diff --git a/modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts b/modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts new file mode 100644 index 000000000..daeb74055 --- /dev/null +++ b/modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts @@ -0,0 +1,66 @@ +import { beforeEach, describe, expect, it, jest } from '@jest/globals'; +import { + CompatibleIndexType, + ConduitGrpcSdk, + ConduitSchema, + MongoIndexType, + PostgresIndexType, + TYPE, +} from '@conduitplatform/grpc-sdk'; +import { schemaConverter } from '../SchemaConverter.js'; + +describe('mongoose SchemaConverter indexes T17 T23', () => { + beforeEach(() => { + jest.spyOn(ConduitGrpcSdk.Logger, 'warn').mockImplementation(() => {}); + }); + + it('T17 treats Compatible as Mongo 1/-1', () => { + const converted = schemaConverter( + new ConduitSchema( + 'User', + { + email: { + type: TYPE.String, + index: { type: CompatibleIndexType.Descending }, + }, + } as any, + {}, + ), + ); + expect((converted.fields.email as any).index.type).toBe(MongoIndexType.Descending); + }); + + it('T23 recover: postgres leftovers on Mongo are warned and skipped', () => { + const warn = ConduitGrpcSdk.Logger.warn as jest.Mock; + const converted = schemaConverter( + new ConduitSchema( + 'User', + { + email: { + type: TYPE.String, + index: { type: PostgresIndexType.GIN }, + }, + } as any, + {}, + ), + ); + expect((converted.fields.email as any).index).toBeUndefined(); + expect(warn).toHaveBeenCalled(); + }); + + it('does not treat the Mongo enum key "Ascending" as a valid Mongo type', () => { + const converted = schemaConverter( + new ConduitSchema( + 'User', + { + email: { + type: TYPE.String, + index: { type: 'Ascending' as any }, + }, + } as any, + {}, + ), + ); + expect((converted.fields.email as any).index.type).toBe(MongoIndexType.Ascending); + }); +}); diff --git a/modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts b/modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts new file mode 100644 index 000000000..681228b26 --- /dev/null +++ b/modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it, jest } from '@jest/globals'; +import { status } from '@grpc/grpc-js'; +import { CompatibleIndexType, MongoIndexType } from '@conduitplatform/grpc-sdk'; +import { MongooseAdapter } from '../index.js'; + +function makeAdapter(overrides: Record = {}) { + const createIndex = jest.fn().mockResolvedValue('email_1'); + const dropIndex = jest.fn().mockResolvedValue(undefined); + const indexes = jest.fn().mockResolvedValue([ + { v: 2, key: { _id: 1 }, name: '_id_' }, + { v: 2, key: { email: 1 }, name: 'cnd_idx_email_asc', unique: false }, + ]); + const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); + const findByIdAndUpdate = jest.fn().mockResolvedValue({}); + const adapter = Object.create(MongooseAdapter.prototype) as MongooseAdapter; + adapter.mongoose = { + model: () => ({ collection: { createIndex, dropIndex, indexes } }), + } as any; + const originalSchema = { + name: 'User', + ownerModule: 'chat', + collectionName: 'cnd_User', + fields: { email: { type: 'String' } }, + compiledFields: { email: { type: 'String' } }, + modelOptions: { indexes: [] as unknown[] }, + ...((overrides.originalSchema as object) ?? {}), + }; + adapter.models = { + User: { originalSchema }, + _DeclaredSchema: { findOne, findByIdAndUpdate }, + } as any; + return { adapter, createIndex, dropIndex, indexes, findByIdAndUpdate, originalSchema }; +} + +describe('mongoose adapter indexes T26–T29 T34 T38', () => { + it('T26 createIndex uses a single key spec object, not an array of objects', async () => { + const { adapter, createIndex } = makeAdapter(); + await adapter.createIndexes( + 'User', + [ + { + fields: ['email'], + types: [CompatibleIndexType.Ascending], + }, + ], + 'chat', + ); + expect(createIndex).toHaveBeenCalledTimes(1); + expect(createIndex.mock.calls[0][0]).toEqual({ email: MongoIndexType.Ascending }); + expect(Array.isArray(createIndex.mock.calls[0][0])).toBe(false); + }); + + it('T27 create persists metadata into _DeclaredSchema', async () => { + const { adapter, findByIdAndUpdate } = makeAdapter(); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'chat', + ); + expect(findByIdAndUpdate).toHaveBeenCalledTimes(1); + const update = findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes[0].name).toMatch(/email/); + }); + + it('T28 delete awaits dropIndex', async () => { + const { adapter, dropIndex } = makeAdapter(); + let resolveDrop: () => void = () => {}; + const dropped = new Promise(resolve => { + resolveDrop = resolve; + }); + dropIndex.mockImplementation(async () => { + resolveDrop(); + }); + await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + await dropped; + expect(dropIndex).toHaveBeenCalledWith('cnd_idx_email_asc'); + }); + + it('T29 delete persists removal', async () => { + const { adapter, findByIdAndUpdate, originalSchema } = makeAdapter({ + originalSchema: { + modelOptions: { indexes: [{ fields: ['email'], name: 'cnd_idx_email_asc' }] }, + }, + }); + await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + expect(originalSchema.modelOptions.indexes).toEqual([]); + expect(findByIdAndUpdate).toHaveBeenCalled(); + }); + + it('T34 getIndexes uses the live engine as source of truth', async () => { + const { adapter, indexes } = makeAdapter(); + const result = await adapter.getIndexes('User'); + expect(indexes).toHaveBeenCalled(); + expect(result.map(i => i.name)).toEqual(['_id_', 'cnd_idx_email_asc']); + expect(result[1].fields).toEqual(['email']); + }); + + it('T38 Admin-bound invalid types throw', async () => { + const { adapter } = makeAdapter(); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], types: ['GIST'] as any }], + 'database', + { privileged: true }, + ), + ).rejects.toMatchObject({ code: status.INVALID_ARGUMENT }); + }); + + it('T15 Admin privileged unique is allowed on a foreign-owned schema', async () => { + const { adapter } = makeAdapter(); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], options: { unique: true } }], + 'database', + { privileged: true }, + ), + ).resolves.toBe('Indexes created!'); + }); +}); diff --git a/modules/database/src/adapters/mongoose-adapter/index.ts b/modules/database/src/adapters/mongoose-adapter/index.ts index 7d5362f57..e14298eae 100644 --- a/modules/database/src/adapters/mongoose-adapter/index.ts +++ b/modules/database/src/adapters/mongoose-adapter/index.ts @@ -12,6 +12,22 @@ import { } from '@conduitplatform/grpc-sdk'; import { DatabaseAdapter } from '../DatabaseAdapter.js'; import { validateFieldChanges, validateFieldConstraints } from '../utils/index.js'; +import { + assertUniqueIndexPrivilege, + ensureIndexName, + isCompatibleIndexType, + isIndexAlreadyExistsError, + isMongoIndexType, + mapCompatibleToMongo, + mergeDeclaredIndexes, + mongoAllowsIndexType, + persistDeclaredSchemaIndexes, + removeDeclaredIndexes, + removeIndexFromSchemaFields, + resolveIndexName, + toMutableIndexes, + validateIndexFields, +} from '../utils/indexes.js'; import pluralize from '../../utils/pluralize.js'; import { mongoSchemaConverter } from '../../introspection/mongoose/utils.js'; import { status } from '@grpc/grpc-js'; @@ -622,22 +638,40 @@ export class MongooseAdapter extends DatabaseAdapter { schemaName: string, indexes: readonly ModelOptionsIndexes[], callerModule: string, + options?: { privileged?: boolean }, ): Promise { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); - this.checkIndexes(schemaName, indexes, callerModule); + const prepared = this.checkIndexes( + schemaName, + indexes, + callerModule, + options?.privileged, + ); const collection = this.mongoose.model(schemaName).collection; - for (const index of indexes) { - const indexSpecs = []; + for (const index of prepared) { + const spec: Record = {}; + const types = Array.isArray(index.types) ? index.types : undefined; for (let i = 0; i < index.fields.length; i++) { - const spec: any = {}; - spec[index.fields[i]] = index.types ? index.types[i] : 1; - indexSpecs.push(spec); + spec[index.fields[i]] = types + ? mapCompatibleToMongo(types[i]) + : MongoIndexType.Ascending; + } + try { + await collection.createIndex(spec, index.options); + } catch (e) { + if (isIndexAlreadyExistsError(e)) continue; + throw new GrpcError(status.INTERNAL, (e as Error).message); } - await collection.createIndex(indexSpecs, index.options).catch((e: Error) => { - throw new GrpcError(status.INTERNAL, e.message); - }); } + const original = this.models[schemaName].originalSchema; + const merged = mergeDeclaredIndexes(original.modelOptions.indexes, prepared); + await persistDeclaredSchemaIndexes({ + declaredSchemaModel: this.models['_DeclaredSchema'], + schemaName, + originalSchema: original, + indexes: merged, + }); return 'Indexes created!'; } @@ -683,29 +717,36 @@ export class MongooseAdapter extends DatabaseAdapter { throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); const collection = this.mongoose.model(schemaName).collection; const result = await collection.indexes(); - result.filter(index => { - index.options = {}; - for (const indexEntry of Object.entries(index)) { - if (indexEntry[0] === 'key' || indexEntry[0] === 'options') { - continue; - } - if (indexEntry[0] === 'v') { - delete index.v; - continue; - } - index.options[indexEntry[0]] = indexEntry[1]; - delete index[indexEntry[0]]; + const declared = this.models[schemaName].originalSchema.modelOptions.indexes ?? []; + const declaredByName = new Map( + declared.map((index: ModelOptionsIndexes) => [resolveIndexName(index), index]), + ); + return result.map(index => { + const options: Record = {}; + for (const [key, value] of Object.entries(index)) { + if (key === 'key' || key === 'options') continue; + if (key === 'v') continue; + options[key] = value; } - index.fields = []; - index.types = []; - for (const keyEntry of Object.entries(index.key)) { - index.fields.push(keyEntry[0]); - index.types.push(keyEntry[1]); - //@ts-expect-error - delete index.key; + const fields: string[] = []; + const types: MongoIndexType[] = []; + for (const [field, type] of Object.entries(index.key ?? {})) { + fields.push(field); + types.push(type as MongoIndexType); } + const name = (options.name as string | undefined) ?? index.name; + const declaredIndex = name ? declaredByName.get(name) : undefined; + return { + name, + fields, + types: declaredIndex?.types ?? types, + options: { + ...declaredIndex?.options, + ...options, + name, + }, + } as ModelOptionsIndexes; }); - return result as unknown as ModelOptionsIndexes[]; } async deleteIndexes(schemaName: string, indexNames: string[]): Promise { @@ -713,10 +754,23 @@ export class MongooseAdapter extends DatabaseAdapter { throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); const collection = this.mongoose.model(schemaName).collection; for (const name of indexNames) { - collection.dropIndex(name).catch(() => { + try { + await collection.dropIndex(name); + } catch { throw new GrpcError(status.INTERNAL, 'Unsuccessful index deletion'); - }); + } } + const original = this.models[schemaName].originalSchema; + for (const name of indexNames) { + removeIndexFromSchemaFields(original, name); + } + const remaining = removeDeclaredIndexes(original.modelOptions.indexes, indexNames); + await persistDeclaredSchemaIndexes({ + declaredSchemaModel: this.models['_DeclaredSchema'], + schemaName, + originalSchema: original, + indexes: remaining, + }); return 'Indexes deleted'; } @@ -854,35 +908,50 @@ export class MongooseAdapter extends DatabaseAdapter { schemaName: string, indexes: readonly ModelOptionsIndexes[], callerModule: string, - ) { - for (const index of indexes) { + privileged?: boolean, + ): ModelOptionsIndexes[] { + const schema = this.models[schemaName].originalSchema; + const prepared: ModelOptionsIndexes[] = []; + for (const raw of toMutableIndexes(indexes)) { + const index = ensureIndexName(raw); + validateIndexFields(schema, index); const options = index.options; const types = index.types; - if (!options && !types) continue; if (options) { if (!checkIfMongoOptions(options)) { - throw new GrpcError(status.INTERNAL, 'Invalid index options for mongoDB'); - } - if ( - Object.keys(options).includes('unique') && - this.models[schemaName].originalSchema.ownerModule !== callerModule - ) { throw new GrpcError( - status.PERMISSION_DENIED, - 'Not authorized to create unique index', + status.INVALID_ARGUMENT, + 'Invalid index options for mongoDB', ); } + assertUniqueIndexPrivilege({ + unique: options.unique === true, + schemaOwner: schema.ownerModule, + callerModule, + privileged, + }); } if (types) { - if (!Array.isArray(types) || types.length !== index.fields.length) { - throw new GrpcError(status.INTERNAL, 'Invalid index types format'); + const typeList = Array.isArray(types) ? types : index.fields.map(() => types); + if (Array.isArray(types) && typeList.length !== index.fields.length) { + throw new GrpcError(status.INVALID_ARGUMENT, 'Invalid index types format'); } - for (const type of types) { - if (!Object.values(MongoIndexType).includes(type)) { - throw new GrpcError(status.INTERNAL, 'Invalid index type for mongoDB'); + for (const type of typeList) { + if (!mongoAllowsIndexType(type)) { + throw new GrpcError( + status.INVALID_ARGUMENT, + 'Invalid index type for mongoDB', + ); } } + index.types = typeList.map(type => + isCompatibleIndexType(type) || isMongoIndexType(type) + ? mapCompatibleToMongo(type) + : type, + ) as MongoIndexType[]; } + prepared.push(index); } + return prepared; } } diff --git a/modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts b/modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts new file mode 100644 index 000000000..fe7cf5cd1 --- /dev/null +++ b/modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it, jest } from '@jest/globals'; +import { CompatibleIndexType, PostgresIndexType } from '@conduitplatform/grpc-sdk'; +import { SequelizeAdapter } from '../index.js'; + +class TestSequelizeAdapter extends SequelizeAdapter { + protected async hasLegacyCollections(): Promise { + return false; + } +} + +function makeAdapter(dialect: string = 'postgres') { + const addIndex = jest.fn().mockResolvedValue(undefined); + const removeIndex = jest.fn().mockResolvedValue(undefined); + const showIndex = jest.fn().mockResolvedValue([ + { + name: 'cnd_idx_email_asc', + unique: false, + fields: [{ attribute: 'email', order: 'ASC' }], + definition: 'CREATE INDEX cnd_idx_email_asc ON cnd_User USING btree (email)', + }, + ]); + const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); + const findByIdAndUpdate = jest.fn().mockResolvedValue({}); + const sync = jest.fn().mockResolvedValue(undefined); + const adapter = Object.create(TestSequelizeAdapter.prototype) as SequelizeAdapter; + adapter.sequelize = { + getDialect: () => dialect, + getQueryInterface: () => ({ addIndex, removeIndex, showIndex }), + } as any; + const originalSchema = { + name: 'User', + ownerModule: 'database', + collectionName: 'custom_users', + fields: { email: { type: 'String' } }, + compiledFields: { email: { type: 'String' } }, + modelOptions: { indexes: [] as unknown[] }, + }; + adapter.models = { + User: { originalSchema, sync }, + _DeclaredSchema: { findOne, findByIdAndUpdate }, + } as any; + return { + adapter, + addIndex, + removeIndex, + showIndex, + sync, + findByIdAndUpdate, + originalSchema, + }; +} + +describe('sequelize adapter indexes T24 T30–T33', () => { + it('T24 getDatabaseType still returns PostgreSQL, not postgres', () => { + const { adapter } = makeAdapter('postgres'); + expect(adapter.getDatabaseType()).toBe('PostgreSQL'); + }); + + it('T30 create/get/delete use originalSchema.collectionName, not a hardcoded cnd_ prefix', async () => { + const { adapter, addIndex, removeIndex, showIndex } = makeAdapter(); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'database', + { privileged: true }, + ); + expect(addIndex.mock.calls[0][0]).toBe('custom_users'); + expect(addIndex.mock.calls[0][0]).not.toBe('cnd_User'); + await adapter.getIndexes('User'); + expect(showIndex).toHaveBeenCalledWith('custom_users'); + await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + expect(removeIndex).toHaveBeenCalledWith('custom_users', 'cnd_idx_email_asc'); + }); + + it('T31 create does not rebuild/sync the schema', async () => { + const { adapter, sync } = makeAdapter(); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'database', + ); + expect(sync).not.toHaveBeenCalled(); + }); + + it('T32 getIndexes reads the live engine and overlays declared Compatible types', async () => { + const { adapter, showIndex, originalSchema } = makeAdapter(); + originalSchema.modelOptions.indexes = [ + { + fields: ['email'], + name: 'cnd_idx_email_asc', + types: [CompatibleIndexType.Ascending], + }, + ]; + const result = await adapter.getIndexes('User'); + expect(showIndex).toHaveBeenCalledWith('custom_users'); + expect(result[0].types).toEqual([CompatibleIndexType.Ascending]); + expect(result[0].fields).toEqual(['email']); + }); + + it('T33 delete awaits removeIndex and persists', async () => { + const { adapter, removeIndex, findByIdAndUpdate } = makeAdapter(); + await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + expect(removeIndex).toHaveBeenCalledTimes(1); + expect(findByIdAndUpdate).toHaveBeenCalled(); + }); + + it('mysql HASH is allowed; sqlite HASH throws on Admin create', async () => { + const mysql = makeAdapter('mysql'); + await expect( + mysql.adapter.createIndexes( + 'User', + [{ fields: ['email'], types: PostgresIndexType.HASH }], + 'database', + { privileged: true }, + ), + ).resolves.toBe('Indexes created!'); + + const sqlite = makeAdapter('sqlite'); + await expect( + sqlite.adapter.createIndexes( + 'User', + [{ fields: ['email'], types: PostgresIndexType.HASH }], + 'database', + { privileged: true }, + ), + ).rejects.toMatchObject({ message: expect.stringMatching(/sqlite/i) }); + }); +}); diff --git a/modules/database/src/adapters/sequelize-adapter/index.ts b/modules/database/src/adapters/sequelize-adapter/index.ts index 30df75904..43f485d4b 100644 --- a/modules/database/src/adapters/sequelize-adapter/index.ts +++ b/modules/database/src/adapters/sequelize-adapter/index.ts @@ -29,6 +29,21 @@ import { import { sqlSchemaConverter } from './sql-adapter/SqlSchemaConverter.js'; import { pgSchemaConverter } from './postgres-adapter/PgSchemaConverter.js'; import { isEqual, isNil } from 'lodash-es'; +import { + assertUniqueIndexPrivilege, + ensureIndexName, + inferSqlIndexType, + isIndexAlreadyExistsError, + mergeDeclaredIndexes, + persistDeclaredSchemaIndexes, + removeDeclaredIndexes, + removeIndexFromSchemaFields, + resolveIndexName, + sqlDialectAllowsIndexType, + sqlIndexFields, + toMutableIndexes, + validateIndexFields, +} from '../utils/indexes.js'; const sqlSchemaName = process.env.SQL_SCHEMA ?? 'public'; @@ -248,7 +263,7 @@ export abstract class SequelizeAdapter extends DatabaseAdapter const [newSchema, objectPaths, extractedRelations] = dialect === 'postgres' ? pgSchemaConverter(compiledSchema) - : sqlSchemaConverter(compiledSchema); + : sqlSchemaConverter(compiledSchema, dialect as 'mysql' | 'mariadb' | 'sqlite'); this.registeredSchemas.set( schema.name, Object.freeze(JSON.parse(JSON.stringify(schema))), @@ -356,69 +371,108 @@ export abstract class SequelizeAdapter extends DatabaseAdapter async createIndexes( schemaName: string, - indexes: ModelOptionsIndexes[], + indexes: readonly ModelOptionsIndexes[], callerModule: string, + options?: { privileged?: boolean }, ): Promise { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); - indexes = this.checkAndConvertIndexes(schemaName, indexes, callerModule); + const prepared = this.checkAndConvertIndexes( + schemaName, + indexes, + callerModule, + options?.privileged, + ); + const collectionName = this.models[schemaName].originalSchema.collectionName; const queryInterface = this.sequelize.getQueryInterface(); - for (const index of indexes) { - await queryInterface - .addIndex('cnd_' + schemaName, [...index.fields], index.options) - .catch(() => { - throw new GrpcError(status.INTERNAL, 'Unsuccessful index creation'); + for (const index of prepared) { + try { + await queryInterface.addIndex(collectionName, { + fields: sqlIndexFields(index), + ...index.options, }); + } catch (e) { + if (isIndexAlreadyExistsError(e)) continue; + throw new GrpcError(status.INTERNAL, 'Unsuccessful index creation'); + } } - await this.models[schemaName].sync(); + const original = this.models[schemaName].originalSchema; + const merged = mergeDeclaredIndexes(original.modelOptions.indexes, prepared); + await persistDeclaredSchemaIndexes({ + declaredSchemaModel: this.models['_DeclaredSchema'], + schemaName, + originalSchema: original, + indexes: merged, + }); return 'Indexes created!'; } async getIndexes(schemaName: string): Promise { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); + const collectionName = this.models[schemaName].originalSchema.collectionName; const queryInterface = this.sequelize.getQueryInterface(); - const result = (await queryInterface.showIndex('cnd_' + schemaName)) as UntypedArray; - result.filter(index => { - const fieldNames = []; - for (const field of index.fields) { - fieldNames.push(field.attribute); - } - index.fields = fieldNames; - // extract index type from index definition - let tmp = index.definition.split('USING '); - tmp = tmp[1].split(' '); - index.types = tmp[0]; - delete index.definition; - index.options = {}; - for (const indexEntry of Object.entries(index)) { + const result = (await queryInterface.showIndex(collectionName)) as UntypedArray; + const dialect = this.sequelize.getDialect(); + const declared = this.models[schemaName].originalSchema.modelOptions.indexes ?? []; + const declaredByName = new Map( + declared.map((index: ModelOptionsIndexes) => [resolveIndexName(index), index]), + ); + return result.map(row => { + const fields = (row.fields ?? []).map((field: unknown) => + typeof field === 'string' ? field : (field as { attribute?: string }).attribute, + ); + const name = row.name as string; + const declaredIndex = declaredByName.get(name); + const options: Record = { + name, + unique: !!row.unique, + ...(declaredIndex?.options ?? {}), + }; + for (const [key, value] of Object.entries(row)) { if ( - indexEntry[0] === 'options' || - indexEntry[0] === 'types' || - indexEntry[0] === 'fields' + key === 'options' || + key === 'types' || + key === 'fields' || + key === 'definition' || + key === 'indkey' ) { continue; } - if (indexEntry[0] === 'indkey') { - delete index.indkey; - continue; - } - index.options[indexEntry[0]] = indexEntry[1]; - delete index[indexEntry[0]]; + if (options[key] === undefined) options[key] = value; } + return { + name, + fields, + types: declaredIndex?.types ?? inferSqlIndexType(row, dialect), + options, + } as ModelOptionsIndexes; }); - return result; } async deleteIndexes(schemaName: string, indexNames: string[]): Promise { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); + const collectionName = this.models[schemaName].originalSchema.collectionName; const queryInterface = this.sequelize.getQueryInterface(); for (const name of indexNames) { - queryInterface.removeIndex('cnd_' + schemaName, name).catch(() => { + try { + await queryInterface.removeIndex(collectionName, name); + } catch { throw new GrpcError(status.INTERNAL, 'Unsuccessful index deletion'); - }); + } } + const original = this.models[schemaName].originalSchema; + for (const name of indexNames) { + removeIndexFromSchemaFields(original, name); + } + const remaining = removeDeclaredIndexes(original.modelOptions.indexes, indexNames); + await persistDeclaredSchemaIndexes({ + declaredSchemaModel: this.models['_DeclaredSchema'], + schemaName, + originalSchema: original, + indexes: remaining, + }); return 'Indexes deleted'; } @@ -466,42 +520,57 @@ export abstract class SequelizeAdapter extends DatabaseAdapter private checkAndConvertIndexes( schemaName: string, - indexes: ModelOptionsIndexes[], + indexes: readonly ModelOptionsIndexes[], callerModule: string, - ) { - for (const index of indexes) { - if (!index.types && !index.options) continue; + privileged?: boolean, + ): ModelOptionsIndexes[] { + const schema = this.models[schemaName].originalSchema; + const dialect = this.sequelize.getDialect(); + const prepared: ModelOptionsIndexes[] = []; + for (const raw of toMutableIndexes(indexes)) { + const index = ensureIndexName(raw); + validateIndexFields(schema, index); if (index.types) { - if ( - Array.isArray(index.types) || - !Object.values(PostgresIndexType).includes(index.types) - ) { + const types = Array.isArray(index.types) ? index.types : [index.types]; + if (types.some(type => !sqlDialectAllowsIndexType(dialect, type))) { throw new GrpcError( status.INVALID_ARGUMENT, - 'Invalid index type for PostgreSQL', + `Invalid index type for ${dialect}`, ); } - (index.options as PostgresIndexOptions).using = index.types; - delete index.types; + const first = types[0]; + if ( + typeof first === 'string' && + Object.values(PostgresIndexType).includes(first as PostgresIndexType) && + types.length === 1 + ) { + index.options = { + ...(index.options ?? {}), + using: first as PostgresIndexType, + } as PostgresIndexOptions; + } else { + index.options = { + ...(index.options ?? {}), + using: PostgresIndexType.BTREE, + } as PostgresIndexOptions; + } } if (index.options) { if (!checkIfPostgresOptions(index.options)) { throw new GrpcError( status.INVALID_ARGUMENT, - 'Invalid index options for PostgreSQL', - ); - } - if ( - Object.keys(index.options).includes('unique') && - this.models[schemaName].originalSchema.ownerModule !== callerModule - ) { - throw new GrpcError( - status.PERMISSION_DENIED, - 'Not authorized to create unique index', + `Invalid index options for ${dialect}`, ); } + assertUniqueIndexPrivilege({ + unique: index.options.unique === true, + schemaOwner: schema.ownerModule, + callerModule, + privileged, + }); } + prepared.push(index); } - return indexes; + return prepared; } } diff --git a/modules/database/src/adapters/sequelize-adapter/postgres-adapter/PgSchemaConverter.ts b/modules/database/src/adapters/sequelize-adapter/postgres-adapter/PgSchemaConverter.ts index 8c023fd7a..f04287326 100644 --- a/modules/database/src/adapters/sequelize-adapter/postgres-adapter/PgSchemaConverter.ts +++ b/modules/database/src/adapters/sequelize-adapter/postgres-adapter/PgSchemaConverter.ts @@ -37,13 +37,13 @@ export function pgSchemaConverter(jsonSchema: ConduitSchema): [ delete copy.fields['_id']; } if (copy.modelOptions.indexes) { - copy = convertModelOptionsIndexes(copy); + copy = convertModelOptionsIndexes(copy, 'postgres'); } const objectPaths: any = {}; convertObjectToDotNotation(jsonSchema.fields, copy.fields, objectPaths); const secondaryCopy = cloneDeep(copy.fields); const extractedRelations = extractRelations(secondaryCopy, copy.fields); - copy = convertSchemaFieldIndexes(copy); + copy = convertSchemaFieldIndexes(copy, 'postgres'); iterDeep(secondaryCopy, copy.fields); return [copy, objectPaths, extractedRelations]; } diff --git a/modules/database/src/adapters/sequelize-adapter/sql-adapter/SqlSchemaConverter.ts b/modules/database/src/adapters/sequelize-adapter/sql-adapter/SqlSchemaConverter.ts index fc869ca37..5e70084fa 100644 --- a/modules/database/src/adapters/sequelize-adapter/sql-adapter/SqlSchemaConverter.ts +++ b/modules/database/src/adapters/sequelize-adapter/sql-adapter/SqlSchemaConverter.ts @@ -23,7 +23,10 @@ import { * This function should take as an input a JSON schema and convert it to the sequelize equivalent * @param jsonSchema */ -export function sqlSchemaConverter(jsonSchema: ConduitSchema): [ +export function sqlSchemaConverter( + jsonSchema: ConduitSchema, + dialect: 'mysql' | 'mariadb' | 'sqlite' = 'mysql', +): [ ConduitSchema, { [key: string]: { parentKey: string; childKey: string }; @@ -35,13 +38,13 @@ export function sqlSchemaConverter(jsonSchema: ConduitSchema): [ delete copy.fields['_id']; } if (copy.modelOptions.indexes) { - copy = convertModelOptionsIndexes(copy); + copy = convertModelOptionsIndexes(copy, dialect); } const objectPaths: any = {}; convertObjectToDotNotation(jsonSchema.fields, copy.fields, objectPaths); const secondaryCopy = cloneDeep(copy.fields); const extractedRelations = extractRelations(secondaryCopy, copy.fields); - copy = convertSchemaFieldIndexes(copy); + copy = convertSchemaFieldIndexes(copy, dialect); iterDeep(secondaryCopy, copy.fields); return [copy, objectPaths, extractedRelations]; } diff --git a/modules/database/src/adapters/utils/__tests__/indexes.test.ts b/modules/database/src/adapters/utils/__tests__/indexes.test.ts new file mode 100644 index 000000000..c5dbc5335 --- /dev/null +++ b/modules/database/src/adapters/utils/__tests__/indexes.test.ts @@ -0,0 +1,198 @@ +import { describe, expect, it } from '@jest/globals'; +import { status } from '@grpc/grpc-js'; +import { + CompatibleIndexType, + MongoIndexType, + PostgresIndexType, +} from '@conduitplatform/grpc-sdk'; +import { + assertUniqueIndexPrivilege, + collectExistingIndexNames, + ensureIndexName, + generateIndexName, + isCompatibleIndexType, + isIndexAlreadyExistsError, + isMongoIndexType, + mapCompatibleToMongo, + mapCompatibleToSqlOrder, + mergeDeclaredIndexes, + mongoAllowsIndexType, + removeDeclaredIndexes, + removeIndexFromSchemaFields, + resolveIndexName, + sqlDialectAllowsIndexType, + sqlIndexFields, + validateIndexFields, +} from '../indexes.js'; + +describe('index helpers T1–T16', () => { + it('T1 CompatibleIndexType uses portable string values, not Mongo 1/-1', () => { + expect(CompatibleIndexType.Ascending).toBe('Ascending'); + expect(CompatibleIndexType.Descending).toBe('Descending'); + expect(CompatibleIndexType.Ascending).not.toBe(MongoIndexType.Ascending); + expect(isCompatibleIndexType(CompatibleIndexType.Ascending)).toBe(true); + expect(isCompatibleIndexType(1)).toBe(false); + expect(isMongoIndexType('Ascending')).toBe(false); + }); + + it('T4 generates a name when missing', () => { + const name = generateIndexName(['email'], [CompatibleIndexType.Ascending], false); + expect(name).toMatch(/^cnd_idx_email_asc$/); + }); + + it('T5 keeps a provided name', () => { + const named = ensureIndexName({ + fields: ['email'], + name: 'custom_email_idx', + }); + expect(resolveIndexName(named)).toBe('custom_email_idx'); + expect(named.options?.name).toBe('custom_email_idx'); + }); + + it('T6 is deterministic for the same input', () => { + const a = generateIndexName( + ['room', 'createdAt'], + [CompatibleIndexType.Ascending, CompatibleIndexType.Descending], + true, + ); + const b = generateIndexName( + ['room', 'createdAt'], + [CompatibleIndexType.Ascending, CompatibleIndexType.Descending], + true, + ); + expect(a).toBe(b); + expect(a).toMatch(/^cnd_uidx_/); + }); + + it('T7 maps Compatible to Mongo 1/-1', () => { + expect(mapCompatibleToMongo(CompatibleIndexType.Ascending)).toBe(1); + expect(mapCompatibleToMongo(CompatibleIndexType.Descending)).toBe(-1); + expect(mapCompatibleToMongo(undefined)).toBe(1); + }); + + it('T8 maps Compatible to SQL BTREE ASC/DESC field order', () => { + expect(mapCompatibleToSqlOrder(CompatibleIndexType.Ascending)).toBe('ASC'); + expect(mapCompatibleToSqlOrder(CompatibleIndexType.Descending)).toBe('DESC'); + const fields = sqlIndexFields({ + fields: ['createdAt', 'room'], + types: [CompatibleIndexType.Descending, CompatibleIndexType.Ascending], + }); + expect(fields).toEqual([ + { name: 'createdAt', order: 'DESC' }, + { name: 'room', order: 'ASC' }, + ]); + }); + + it('T9 preserves the unique option on generated names', () => { + const unique = ensureIndexName({ + fields: ['email'], + types: [CompatibleIndexType.Ascending], + options: { unique: true }, + }); + expect(unique.options?.unique).toBe(true); + expect(resolveIndexName(unique)).toMatch(/uidx/); + }); + + it('T10 persist helper merges incoming indexes by name and skips duplicates', () => { + const merged = mergeDeclaredIndexes( + [{ fields: ['a'], name: 'idx_a' }], + [ + { fields: ['a'], name: 'idx_a', options: { unique: true } }, + { fields: ['b'], name: 'idx_b' }, + ], + ); + expect(merged.map(i => i.name)).toEqual(['idx_a', 'idx_b']); + expect(merged[0].options?.unique).toBeUndefined(); + }); + + it('T11 persist helper removes indexes by name', () => { + const remaining = removeDeclaredIndexes( + [ + { fields: ['a'], name: 'idx_a' }, + { fields: ['b'], name: 'idx_b' }, + ], + ['idx_a'], + ); + expect(remaining).toEqual([{ fields: ['b'], name: 'idx_b' }]); + }); + + it('T12 validateIndexFields rejects unknown fields', () => { + expect(() => + validateIndexFields( + { compiledFields: { email: 'String' }, fields: {} }, + { + fields: ['missing'], + }, + ), + ).toThrow(/Invalid fields/); + }); + + it('T13 unique is denied for a non-owner, non-admin caller', () => { + expect(() => + assertUniqueIndexPrivilege({ + unique: true, + schemaOwner: 'chat', + callerModule: 'database', + privileged: false, + }), + ).toThrow(expect.objectContaining({ code: status.PERMISSION_DENIED })); + }); + + it('T14 unique is allowed for the schema owner', () => { + expect(() => + assertUniqueIndexPrivilege({ + unique: true, + schemaOwner: 'chat', + callerModule: 'chat', + privileged: false, + }), + ).not.toThrow(); + }); + + it('T15 unique is allowed for privileged Admin', () => { + expect(() => + assertUniqueIndexPrivilege({ + unique: true, + schemaOwner: 'chat', + callerModule: 'database', + privileged: true, + }), + ).not.toThrow(); + }); + + it('T16 import unique respects owner privilege (not Admin-privileged)', () => { + expect(() => + assertUniqueIndexPrivilege({ + unique: true, + schemaOwner: 'authorization', + callerModule: 'database', + privileged: false, + }), + ).toThrow(/Not authorized to create unique index/); + }); + + it('collects existing names and detects already-exists errors', () => { + expect( + collectExistingIndexNames([{ fields: ['a'], options: { name: 'idx_a' } }]).has( + 'idx_a', + ), + ).toBe(true); + expect(isIndexAlreadyExistsError(new Error('index already exists'))).toBe(true); + expect( + removeIndexFromSchemaFields({ fields: { a: { index: { name: 'x' } } } }, 'x'), + ).toBe(true); + }); + + it('T25 dialect checks use real switches, not `mysql || mariadb`', () => { + expect(sqlDialectAllowsIndexType('mysql', CompatibleIndexType.Ascending)).toBe(true); + expect(sqlDialectAllowsIndexType('mariadb', CompatibleIndexType.Descending)).toBe( + true, + ); + expect(sqlDialectAllowsIndexType('sqlite', PostgresIndexType.BTREE)).toBe(true); + expect(sqlDialectAllowsIndexType('sqlite', PostgresIndexType.HASH)).toBe(false); + expect(sqlDialectAllowsIndexType('mysql', PostgresIndexType.GIST)).toBe(false); + expect(sqlDialectAllowsIndexType('postgres', PostgresIndexType.GIN)).toBe(true); + expect(mongoAllowsIndexType(CompatibleIndexType.Ascending)).toBe(true); + expect(mongoAllowsIndexType(PostgresIndexType.BTREE)).toBe(false); + }); +}); diff --git a/modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts b/modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts new file mode 100644 index 000000000..95e061b0e --- /dev/null +++ b/modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts @@ -0,0 +1,125 @@ +import { beforeEach, describe, expect, it, jest } from '@jest/globals'; +import { + CompatibleIndexType, + ConduitGrpcSdk, + ConduitSchema, + MongoIndexType, + PostgresIndexType, + TYPE, +} from '@conduitplatform/grpc-sdk'; +import { + convertModelOptionsIndexes, + convertSchemaFieldIndexes, +} from '../database-transform-utils.js'; +import { sqlSchemaConverter } from '../../sequelize-adapter/sql-adapter/SqlSchemaConverter.js'; +import { pgSchemaConverter } from '../../sequelize-adapter/postgres-adapter/PgSchemaConverter.js'; + +function schemaWithIndexes( + indexes: ConduitSchema['modelOptions']['indexes'], + fields: ConduitSchema['fields'] = { email: { type: TYPE.String } }, +) { + return new ConduitSchema('User', fields as any, { indexes }); +} + +describe('SQL dialect-aware converters T17–T24', () => { + beforeEach(() => { + jest.spyOn(ConduitGrpcSdk.Logger, 'warn').mockImplementation(() => {}); + }); + + it('T18 postgres maps Compatible to BTREE + ASC/DESC', () => { + const copy = convertModelOptionsIndexes( + schemaWithIndexes([ + { + fields: ['email'], + types: [CompatibleIndexType.Descending], + }, + ]), + 'postgres', + ); + const index = copy.modelOptions.indexes![0] as any; + expect(index.using).toBe(PostgresIndexType.BTREE); + expect(index.fields[0]).toEqual({ name: 'email', order: 'DESC' }); + }); + + it('T19 mysql maps Compatible to BTREE + ASC', () => { + const copy = convertModelOptionsIndexes( + schemaWithIndexes([{ fields: ['email'], types: [CompatibleIndexType.Ascending] }]), + 'mysql', + ); + const index = copy.modelOptions.indexes![0] as any; + expect(index.using).toBe(PostgresIndexType.BTREE); + expect(index.fields[0]).toEqual({ name: 'email', order: 'ASC' }); + }); + + it('T20 sqlite maps Compatible to BTREE + ASC', () => { + const copy = convertModelOptionsIndexes( + schemaWithIndexes([{ fields: ['email'], types: CompatibleIndexType.Ascending }]), + 'sqlite', + ); + const index = copy.modelOptions.indexes![0] as any; + expect(index.using).toBe(PostgresIndexType.BTREE); + }); + + it('T21 recover: Mongo-only leftovers on SQL are warned and skipped', () => { + const warn = ConduitGrpcSdk.Logger.warn as jest.Mock; + const copy = convertModelOptionsIndexes( + schemaWithIndexes([ + { fields: ['loc'], types: [MongoIndexType.GeoSpatial2dSphere] }, + { fields: ['email'], types: [CompatibleIndexType.Ascending] }, + ]), + 'postgres', + ); + expect(copy.modelOptions.indexes).toHaveLength(1); + expect(warn).toHaveBeenCalled(); + }); + + it('T22 recover: postgres-only types on mysql/mariadb/sqlite are skipped', () => { + for (const dialect of ['mysql', 'mariadb', 'sqlite'] as const) { + const copy = convertModelOptionsIndexes( + schemaWithIndexes([{ fields: ['email'], types: PostgresIndexType.GIST }]), + dialect, + ); + expect(copy.modelOptions.indexes).toHaveLength(0); + } + }); + + it('converts field-level Compatible indexes into sequelize model indexes', () => { + const copy = convertSchemaFieldIndexes( + new ConduitSchema( + 'Perm', + { + resource: { + type: TYPE.String, + index: { type: CompatibleIndexType.Ascending }, + }, + } as any, + {}, + ), + 'mysql', + ); + expect(copy.modelOptions.indexes).toHaveLength(1); + expect((copy.fields.resource as any).index).toBeUndefined(); + }); + + it('sqlSchemaConverter is dialect-aware for mysql vs sqlite', () => { + const schema = new ConduitSchema('User', { email: { type: TYPE.String } } as any, { + indexes: [ + { fields: ['email'], types: [CompatibleIndexType.Descending] }, + { fields: ['email'], types: PostgresIndexType.HASH }, + ], + }); + const [mysql] = sqlSchemaConverter(schema, 'mysql'); + const [sqlite] = sqlSchemaConverter(schema, 'sqlite'); + expect(mysql.modelOptions.indexes).toHaveLength(2); + expect(sqlite.modelOptions.indexes).toHaveLength(1); + }); + + it('pgSchemaConverter keeps postgres-only types', () => { + const schema = new ConduitSchema('User', { email: { type: TYPE.String } } as any, { + indexes: [{ fields: ['email'], types: PostgresIndexType.GIN }], + }); + const [pg] = pgSchemaConverter(schema); + expect(pg.modelOptions.indexes).toHaveLength(1); + expect((pg.modelOptions.indexes![0] as any).using).toBe(PostgresIndexType.GIN); + }); +}); diff --git a/modules/database/src/adapters/utils/database-transform-utils.ts b/modules/database/src/adapters/utils/database-transform-utils.ts index 6ec5768d0..47ec7b093 100644 --- a/modules/database/src/adapters/utils/database-transform-utils.ts +++ b/modules/database/src/adapters/utils/database-transform-utils.ts @@ -1,13 +1,23 @@ -import { isArray, isBoolean, isNumber, isString } from 'lodash-es'; +import { isBoolean, isNumber, isString } from 'lodash-es'; import { + CompatibleIndexType, ConduitGrpcSdk, ConduitModelField, ConduitSchema, Indexable, + ModelOptionsIndexes, PostgresIndexOptions, PostgresIndexType, } from '@conduitplatform/grpc-sdk'; import { checkIfPostgresOptions } from '../sequelize-adapter/utils/index.js'; +import { + ensureIndexName, + isPortableDirection, + isPostgresIndexType, + mapCompatibleToSqlOrder, + normalizeIndexTypes, + sqlDialectAllowsIndexType, +} from './indexes.js'; export function checkDefaultValue(type: string, value: string) { switch (type) { @@ -28,72 +38,104 @@ export function checkDefaultValue(type: string, value: string) { } } -export function convertModelOptionsIndexes(copy: ConduitSchema) { - for (const index of copy.modelOptions.indexes!) { - if (index.types) { - if ( - isArray(index.types) || - !Object.values(PostgresIndexType).includes(index.types as PostgresIndexType) - ) { - // ignore index instead of error - ConduitGrpcSdk.Logger.warn('Invalid index type for PostgreSQL, ignoring index'); - continue; - // throw new Error('Incorrect index type for PostgreSQL'); - } - index.using = index.types as PostgresIndexType; - delete index.types; - } - if (index.options) { - if (!checkIfPostgresOptions(index.options)) { - // ignore index instead of error - ConduitGrpcSdk.Logger.warn( - 'Invalid index options for PostgreSQL, ignoring index', - ); - continue; - // throw new Error('Incorrect index options for PostgreSQL'); - } - for (const [option, value] of Object.entries(index.options)) { - index[option as keyof PostgresIndexOptions] = value; - } - delete index.options; - } +function flattenSqlIndexOptions(index: ModelOptionsIndexes, dialect: string): boolean { + if (!index.options) return true; + if (!checkIfPostgresOptions(index.options)) { + ConduitGrpcSdk.Logger.warn( + `Invalid index options for ${dialect} found in '${copyName(index)}', ignoring index`, + ); + return false; + } + for (const [option, value] of Object.entries(index.options)) { + index[option as keyof PostgresIndexOptions] = value; + } + delete index.options; + return true; +} + +function copyName(index: ModelOptionsIndexes): string { + return index.name ?? index.fields?.join(',') ?? 'unnamed'; +} + +function applySqlIndexTypes( + index: ModelOptionsIndexes, + dialect: string, + schemaName: string, +): boolean { + if (!index.types) { + index.using = PostgresIndexType.BTREE; + return true; + } + const types = normalizeIndexTypes(index.types, index.fields.length) ?? []; + if (types.some(type => !sqlDialectAllowsIndexType(dialect, type))) { + ConduitGrpcSdk.Logger.warn( + `Invalid index type for ${dialect} found in '${schemaName}', ignoring index`, + ); + return false; + } + if (types.some(isPortableDirection)) { + index.fields = index.fields.map((field, i) => ({ + name: field, + order: mapCompatibleToSqlOrder(types[i]), + })) as unknown as string[]; + index.using = PostgresIndexType.BTREE; + } else if (types.length === 1 && isPostgresIndexType(types[0])) { + index.using = types[0]; + } else { + ConduitGrpcSdk.Logger.warn( + `Invalid index type for ${dialect} found in '${schemaName}', ignoring index`, + ); + return false; } + delete index.types; + return true; +} + +export function convertModelOptionsIndexes(copy: ConduitSchema, dialect = 'postgres') { + const converted: ModelOptionsIndexes[] = []; + for (const raw of copy.modelOptions.indexes ?? []) { + const index = ensureIndexName({ ...raw, fields: [...raw.fields] }); + if (!applySqlIndexTypes(index, dialect, copy.name)) continue; + if (!flattenSqlIndexOptions(index, dialect)) continue; + if (!index.using) index.using = PostgresIndexType.BTREE; + converted.push(index); + } + copy.modelOptions.indexes = converted; return copy; } -export function convertSchemaFieldIndexes(copy: ConduitSchema) { - const indexes = []; - for (const field of Object.entries(copy.fields)) { - const fieldName = field[0]; - const index = (copy.fields[fieldName] as ConduitModelField).index; +export function convertSchemaFieldIndexes(copy: ConduitSchema, dialect = 'postgres') { + const indexes: ModelOptionsIndexes[] = []; + for (const [fieldName, fieldValue] of Object.entries(copy.fields)) { + const index = (fieldValue as ConduitModelField).index; if (!index) continue; - const newIndex: any = { + const newIndex = ensureIndexName({ fields: [fieldName], - }; - if (index.type) { - if (!Object.values(PostgresIndexType).includes(index.type as PostgresIndexType)) { - // ignore index instead of error - ConduitGrpcSdk.Logger.warn('Invalid index type for PostgreSQL, ignoring index'); - continue; - // throw new Error('Invalid index type for PostgreSQL'); - } - newIndex.using = index.type; + types: index.type + ? isPortableDirection(index.type) + ? [index.type as CompatibleIndexType] + : (index.type as PostgresIndexType) + : undefined, + options: index.options, + name: (index as { name?: string }).name, + }); + if (index.type && !sqlDialectAllowsIndexType(dialect, index.type)) { + ConduitGrpcSdk.Logger.warn( + `Invalid index type for ${dialect} found in '${copy.name}', ignoring index`, + ); + delete (copy.fields[fieldName] as ConduitModelField).index; + continue; + } + if (!applySqlIndexTypes(newIndex, dialect, copy.name)) { + delete (copy.fields[fieldName] as ConduitModelField).index; + continue; } - if (index.options) { - if (!checkIfPostgresOptions(index.options)) { - // ignore index instead of error - ConduitGrpcSdk.Logger.warn( - 'Invalid index options for PostgreSQL, ignoring index', - ); - continue; - // throw new Error('Invalid index options for PostgreSQL'); - } - for (const [option, value] of Object.entries(index.options)) { - newIndex[option] = value; - } + if (!flattenSqlIndexOptions(newIndex, dialect)) { + delete (copy.fields[fieldName] as ConduitModelField).index; + continue; } indexes.push(newIndex); - delete copy.fields[fieldName]; + delete (copy.fields[fieldName] as ConduitModelField).index; } if (copy.modelOptions.indexes) { copy.modelOptions.indexes = [...copy.modelOptions.indexes, ...indexes]; diff --git a/modules/database/src/adapters/utils/index.ts b/modules/database/src/adapters/utils/index.ts index ec8998dd4..4d05366eb 100644 --- a/modules/database/src/adapters/utils/index.ts +++ b/modules/database/src/adapters/utils/index.ts @@ -2,3 +2,4 @@ export * from './validateFieldChanges.js'; export * from './validateFieldConstraints.js'; export * from './database-transform-utils.js'; export * from './extensions.js'; +export * from './indexes.js'; diff --git a/modules/database/src/adapters/utils/indexes.ts b/modules/database/src/adapters/utils/indexes.ts new file mode 100644 index 000000000..2c1a2776a --- /dev/null +++ b/modules/database/src/adapters/utils/indexes.ts @@ -0,0 +1,329 @@ +import { createHash } from 'crypto'; +import { + CompatibleIndexType, + ConduitModelField, + GrpcError, + ModelOptionsIndexes, + MongoIndexType, + PostgresIndexType, +} from '@conduitplatform/grpc-sdk'; +import { status } from '@grpc/grpc-js'; +import { ConduitDatabaseSchema } from '../../interfaces/index.js'; + +export const ADMIN_INDEX_CALLER = 'database'; + +export const MONGO_INDEX_TYPE_VALUES: ReadonlyArray = [ + MongoIndexType.Ascending, + MongoIndexType.Descending, + MongoIndexType.GeoSpatial2d, + MongoIndexType.GeoSpatial2dSphere, + MongoIndexType.GeoHaystack, + MongoIndexType.Hashed, + MongoIndexType.Text, +]; + +const SQL_IDENTIFIER_MAX_LEN = 63; + +export function isCompatibleIndexType(value: unknown): value is CompatibleIndexType { + return ( + value === CompatibleIndexType.Ascending || value === CompatibleIndexType.Descending + ); +} + +export function isMongoIndexType(value: unknown): value is MongoIndexType { + return (MONGO_INDEX_TYPE_VALUES as readonly unknown[]).includes(value); +} + +export function isPostgresIndexType(value: unknown): value is PostgresIndexType { + return Object.values(PostgresIndexType).includes(value as PostgresIndexType); +} + +export function resolveIndexName(index: ModelOptionsIndexes): string | undefined { + if (typeof index.name === 'string' && index.name.length > 0) return index.name; + const optionsName = index.options?.name; + if (typeof optionsName === 'string' && optionsName.length > 0) return optionsName; + return undefined; +} + +export function isUniqueIndex(index: ModelOptionsIndexes): boolean { + return index.options?.unique === true; +} + +export function normalizeIndexTypes( + types: ModelOptionsIndexes['types'], + fieldCount: number, +): unknown[] | undefined { + if (types === undefined) return undefined; + if (Array.isArray(types)) { + return [...types]; + } + return Array.from({ length: fieldCount }, () => types); +} + +function typeToken(type: unknown): string { + if (type === undefined || type === CompatibleIndexType.Ascending) return 'asc'; + if (type === CompatibleIndexType.Descending) return 'desc'; + if (type === MongoIndexType.Ascending || type === 1) return 'asc'; + if (type === MongoIndexType.Descending || type === -1) return 'desc'; + if (typeof type === 'string') return type.toLowerCase().replace(/[^a-z0-9]+/g, ''); + return String(type); +} + +export function generateIndexName( + fields: readonly string[], + types?: ModelOptionsIndexes['types'], + unique = false, +): string { + const tokens = ( + normalizeIndexTypes(types, fields.length) ?? fields.map(() => undefined) + ) + .map(typeToken) + .join('_'); + const prefix = unique ? 'cnd_uidx' : 'cnd_idx'; + const raw = `${prefix}_${fields.join('_')}_${tokens}`.replace(/[^A-Za-z0-9_]+/g, '_'); + const sanitized = raw.replace(/_+/g, '_').replace(/^_|_$/g, ''); + if (sanitized.length <= SQL_IDENTIFIER_MAX_LEN) return sanitized; + const hash = createHash('sha1').update(sanitized).digest('hex').slice(0, 8); + return `${sanitized.slice(0, SQL_IDENTIFIER_MAX_LEN - 9)}_${hash}`; +} + +export function ensureIndexName(index: ModelOptionsIndexes): ModelOptionsIndexes { + const existing = resolveIndexName(index); + if (existing) { + return { + ...index, + name: existing, + options: { ...index.options, name: existing }, + }; + } + const name = generateIndexName(index.fields, index.types, isUniqueIndex(index)); + return { + ...index, + name, + options: { ...index.options, name }, + }; +} + +export function mapCompatibleToMongo(type: unknown): MongoIndexType { + if (type === CompatibleIndexType.Descending) return MongoIndexType.Descending; + if (type === CompatibleIndexType.Ascending || type === undefined) { + return MongoIndexType.Ascending; + } + if (isMongoIndexType(type)) return type; + throw new GrpcError(status.INVALID_ARGUMENT, `Invalid index type for MongoDB: ${type}`); +} + +export function mapCompatibleToSqlOrder(type: unknown): 'ASC' | 'DESC' { + if (type === CompatibleIndexType.Descending || type === MongoIndexType.Descending) { + return 'DESC'; + } + return 'ASC'; +} + +export function isPortableDirection(type: unknown): boolean { + return ( + isCompatibleIndexType(type) || + type === MongoIndexType.Ascending || + type === MongoIndexType.Descending + ); +} + +export function sqlDialectAllowsIndexType(dialect: string, type: unknown): boolean { + if (type === undefined || isPortableDirection(type)) return true; + if (type === PostgresIndexType.BTREE) return true; + if (type === PostgresIndexType.HASH) { + return dialect === 'postgres' || dialect === 'mysql' || dialect === 'mariadb'; + } + if (isPostgresIndexType(type)) return dialect === 'postgres'; + return false; +} + +export function mongoAllowsIndexType(type: unknown): boolean { + return type === undefined || isCompatibleIndexType(type) || isMongoIndexType(type); +} + +export function mergeDeclaredIndexes( + existing: readonly ModelOptionsIndexes[] | undefined, + incoming: readonly ModelOptionsIndexes[], +): ModelOptionsIndexes[] { + const merged = new Map(); + for (const index of existing ?? []) { + const named = ensureIndexName(index); + merged.set(resolveIndexName(named)!, named); + } + for (const index of incoming) { + const named = ensureIndexName(index); + const name = resolveIndexName(named)!; + if (!merged.has(name)) { + merged.set(name, named); + } + } + return [...merged.values()]; +} + +export function removeDeclaredIndexes( + existing: readonly ModelOptionsIndexes[] | undefined, + names: readonly string[], +): ModelOptionsIndexes[] { + const drop = new Set(names); + return (existing ?? []).filter(index => { + const name = resolveIndexName(index); + return !name || !drop.has(name); + }); +} + +export function removeIndexFromSchemaFields( + schema: { fields?: Record; compiledFields?: Record }, + indexName: string, +): boolean { + let removed = false; + for (const bag of [schema.fields, schema.compiledFields]) { + if (!bag) continue; + for (const value of Object.values(bag)) { + if (!value || typeof value !== 'object') continue; + const field = value as ConduitModelField; + const name = field.index?.options?.name ?? (field.index as { name?: string })?.name; + if (name === indexName) { + delete field.index; + removed = true; + } + } + } + return removed; +} + +export function validateIndexFields( + schema: Pick, + index: ModelOptionsIndexes, +) { + if (!index.fields || index.fields.length === 0) { + throw new GrpcError( + status.INVALID_ARGUMENT, + 'Index fields must be a non-empty array', + ); + } + const available = new Set([ + ...Object.keys(schema.compiledFields ?? {}), + ...Object.keys(schema.fields ?? {}), + ]); + const missing = index.fields.filter(field => !available.has(field)); + if (missing.length > 0) { + throw new GrpcError( + status.INVALID_ARGUMENT, + `Invalid fields for index creation: ${missing.join(', ')}`, + ); + } +} + +export function assertUniqueIndexPrivilege(args: { + unique: boolean; + schemaOwner: string; + callerModule: string; + privileged?: boolean; +}) { + if (!args.unique) return; + if (args.privileged) return; + if (args.schemaOwner === args.callerModule) return; + throw new GrpcError(status.PERMISSION_DENIED, 'Not authorized to create unique index'); +} + +export function isIndexAlreadyExistsError(error: unknown): boolean { + const err = error as { message?: string; code?: number | string; name?: string }; + const message = (err.message ?? '').toLowerCase(); + if ( + message.includes('already exists') || + message.includes('already exist') || + message.includes('duplicate key name') || + message.includes('index already exists') + ) { + return true; + } + return ( + err.code === 85 || + err.code === '42P07' || + err.name === 'SequelizeUniqueConstraintError' + ); +} + +export async function persistDeclaredSchemaIndexes(args: { + declaredSchemaModel: { + findOne: (query: Record) => Promise<{ _id: string } | null>; + findByIdAndUpdate: (id: string, update: Record) => Promise; + }; + schemaName: string; + originalSchema: { + modelOptions: { indexes?: ModelOptionsIndexes[] | readonly ModelOptionsIndexes[] }; + fields?: Record; + compiledFields?: Record; + }; + indexes: ModelOptionsIndexes[]; +}): Promise { + args.originalSchema.modelOptions.indexes = args.indexes; + const found = await args.declaredSchemaModel.findOne({ name: args.schemaName }); + if (!found) return; + await args.declaredSchemaModel.findByIdAndUpdate(found._id, { + modelOptions: args.originalSchema.modelOptions, + fields: args.originalSchema.fields, + compiledFields: args.originalSchema.compiledFields, + }); +} + +export function collectExistingIndexNames( + indexes: readonly ModelOptionsIndexes[], +): Set { + const names = new Set(); + for (const index of indexes) { + const name = resolveIndexName(index); + if (name) names.add(name); + } + return names; +} + +export function toMutableIndexes( + indexes: readonly ModelOptionsIndexes[], +): ModelOptionsIndexes[] { + return indexes.map(index => ({ + ...index, + fields: [...index.fields], + types: Array.isArray(index.types) ? [...index.types] : index.types, + options: index.options ? { ...index.options } : index.options, + })); +} + +export function sqlIndexFields( + index: ModelOptionsIndexes, +): Array { + const types = normalizeIndexTypes(index.types, index.fields.length); + if (!types || !types.some(isCompatibleIndexType)) { + return [...index.fields]; + } + return index.fields.map((field, i) => ({ + name: field, + order: mapCompatibleToSqlOrder(types[i]), + })); +} + +export function inferSqlIndexType( + row: { type?: string; definition?: string }, + dialect: string, +): PostgresIndexType | undefined { + if (typeof row.type === 'string' && row.type.length > 0) { + const upper = row.type.toUpperCase(); + if (isPostgresIndexType(upper)) return upper; + } + if (typeof row.definition === 'string') { + const match = /USING\s+(\w+)/i.exec(row.definition); + if (match && isPostgresIndexType(match[1].toUpperCase())) { + return match[1].toUpperCase() as PostgresIndexType; + } + } + if ( + dialect === 'postgres' || + dialect === 'mysql' || + dialect === 'mariadb' || + dialect === 'sqlite' + ) { + return PostgresIndexType.BTREE; + } + return undefined; +} diff --git a/modules/database/src/admin/__tests__/schema.admin.indexes.test.ts b/modules/database/src/admin/__tests__/schema.admin.indexes.test.ts new file mode 100644 index 000000000..86627f6d5 --- /dev/null +++ b/modules/database/src/admin/__tests__/schema.admin.indexes.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it, jest } from '@jest/globals'; +import { status } from '@grpc/grpc-js'; +import { ConduitGrpcSdk, ParsedRouterRequest } from '@conduitplatform/grpc-sdk'; +import { SchemaAdmin } from '../schema.admin.js'; +import { DatabaseAdapter } from '../../adapters/DatabaseAdapter.js'; +import { MongooseSchema } from '../../adapters/mongoose-adapter/MongooseSchema.js'; +import { SequelizeSchema } from '../../adapters/sequelize-adapter/SequelizeSchema.js'; +import { SchemaController } from '../../controllers/cms/schema.controller.js'; +import { CustomEndpointController } from '../../controllers/customEndpoints/customEndpoint.controller.js'; +import { ADMIN_INDEX_CALLER } from '../../adapters/utils/indexes.js'; + +function makeCall(params: Record): ParsedRouterRequest { + return { request: { params } } as unknown as ParsedRouterRequest; +} + +function setup() { + const findOne = jest.fn().mockResolvedValue({ + _id: 'schema-1', + name: 'User', + ownerModule: 'database', + }); + const findMany = jest.fn().mockResolvedValue([{ name: 'User' }, { name: 'ChatRoom' }]); + const countDocuments = jest.fn().mockResolvedValue(2); + const createIndexes = jest.fn().mockResolvedValue('Indexes created!'); + const getIndexes = jest + .fn() + .mockResolvedValue([{ name: 'cnd_idx_email_asc', fields: ['email'] }]); + const deleteIndexes = jest.fn().mockResolvedValue('Indexes deleted'); + const getSchemaModel = jest.fn().mockReturnValue({ + model: { findOne, findMany, countDocuments }, + }); + const database = { + getSchemaModel, + createIndexes, + getIndexes, + deleteIndexes, + systemSchemas: ['_DeclaredSchema'], + models: { + User: { originalSchema: { ownerModule: 'database' } }, + ChatRoom: { originalSchema: { ownerModule: 'chat' } }, + }, + } as unknown as DatabaseAdapter; + const admin = new SchemaAdmin( + {} as ConduitGrpcSdk, + database, + {} as SchemaController, + {} as CustomEndpointController, + ); + return { admin, createIndexes, getIndexes, findMany, countDocuments, findOne }; +} + +describe('SchemaAdmin indexes T35–T42', () => { + it('T37 Admin createIndexes is privileged', async () => { + const { admin, createIndexes } = setup(); + await admin.createIndexes( + makeCall({ + id: 'schema-1', + indexes: [{ fields: ['email'], options: { unique: true } }], + }), + ); + expect(createIndexes).toHaveBeenCalledWith( + 'User', + [{ fields: ['email'], options: { unique: true } }], + ADMIN_INDEX_CALLER, + { privileged: true }, + ); + }); + + it('T39 exportIndexes is paginated (skip/limit, no unbounded findMany)', async () => { + const { admin, findMany, countDocuments, getIndexes } = setup(); + const result = (await admin.exportIndexes(makeCall({ skip: 10, limit: 5 }))) as { + indexes: unknown[]; + count: number; + }; + expect(findMany).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ skip: 10, limit: 5 }), + ); + expect(countDocuments).toHaveBeenCalled(); + expect(getIndexes).toHaveBeenCalled(); + expect(result.count).toBe(2); + expect(result.indexes.every(index => 'schemaName' in (index as object))).toBe(true); + }); + + it('T40 importIndexes skips same-name indexes', async () => { + const { admin, createIndexes, getIndexes } = setup(); + getIndexes.mockResolvedValue([{ name: 'keep_me', fields: ['email'] }]); + await admin.importIndexes( + makeCall({ + indexes: [ + { schemaName: 'User', fields: ['email'], name: 'keep_me' }, + { schemaName: 'User', fields: ['name'], name: 'new_name' }, + ], + }), + ); + expect(createIndexes).toHaveBeenCalledTimes(1); + const created = createIndexes.mock.calls[0][1] as { name?: string }[]; + expect(created.map(i => i.name)).toEqual(['new_name']); + }); + + it('T41 import unique is not Admin-privileged (respects owner)', async () => { + const { admin, createIndexes } = setup(); + await admin.importIndexes( + makeCall({ + indexes: [ + { + schemaName: 'ChatRoom', + fields: ['name'], + name: 'chat_name', + options: { unique: true }, + }, + ], + }), + ); + expect(createIndexes).toHaveBeenCalledWith( + 'ChatRoom', + expect.any(Array), + ADMIN_INDEX_CALLER, + { privileged: false }, + ); + }); + + it('T42 import/export are Admin handlers and reject empty import', async () => { + const { admin } = setup(); + await expect(admin.importIndexes(makeCall({ indexes: [] }))).rejects.toMatchObject({ + code: status.INVALID_ARGUMENT, + }); + }); +}); diff --git a/modules/database/src/admin/index.ts b/modules/database/src/admin/index.ts index a4c1be91f..a3de8cb45 100644 --- a/modules/database/src/admin/index.ts +++ b/modules/database/src/admin/index.ts @@ -633,6 +633,35 @@ export class AdminHandlers { }), this.customEndpointsAdmin.schemaDetailsForOperation.bind(this.customEndpointsAdmin), ); + this.routingManager.route( + { + path: '/indexes/export', + action: ConduitRouteActions.GET, + description: `Exports schema indexes. Admin-only and paginated.`, + queryParams: { + skip: ConduitNumber.OptionalWith({ min: 0, integer: true }), + limit: ConduitNumber.OptionalWith({ min: 1, max: 1000, integer: true }), + }, + }, + new ConduitRouteReturnDefinition('ExportSchemaIndexes', { + indexes: [ConduitJson.Required], + count: ConduitNumber.Required, + }), + this.schemaAdmin.exportIndexes.bind(this.schemaAdmin), + ); + this.routingManager.route( + { + path: '/indexes/import', + action: ConduitRouteActions.POST, + description: `Imports schema indexes. Skips indexes that already exist by name. Unique indexes respect schema owner privilege.`, + mcp: false, + bodyParams: { + indexes: { type: [TYPE.JSON], required: true }, + }, + }, + new ConduitRouteReturnDefinition('ImportSchemaIndexes', 'String'), + this.schemaAdmin.importIndexes.bind(this.schemaAdmin), + ); this.routingManager.route( { path: '/schemas/:id/indexes', diff --git a/modules/database/src/admin/schema.admin.ts b/modules/database/src/admin/schema.admin.ts index 9c149d6db..b676f11a5 100644 --- a/modules/database/src/admin/schema.admin.ts +++ b/modules/database/src/admin/schema.admin.ts @@ -3,6 +3,7 @@ import { ConduitSchema, GrpcError, Indexable, + ModelOptionsIndexes, ParsedRouterRequest, UnparsedRouterResponse, } from '@conduitplatform/grpc-sdk'; @@ -23,6 +24,12 @@ import { import { SchemaConverter } from '../utils/SchemaConverter.js'; import { parseSortParam } from '../handlers/utils.js'; import escapeStringRegexp from 'escape-string-regexp'; +import { + ADMIN_INDEX_CALLER, + collectExistingIndexNames, + ensureIndexName, + resolveIndexName, +} from '../adapters/utils/indexes.js'; type ExportedCmsSchema = Pick< ConduitDatabaseSchema, @@ -692,7 +699,12 @@ export class SchemaAdmin { if (isNil(requestedSchema)) { throw new GrpcError(status.NOT_FOUND, 'Schema does not exist'); } - return await this.database.createIndexes(requestedSchema.name, indexes, 'database'); + return await this.database.createIndexes( + requestedSchema.name, + indexes, + ADMIN_INDEX_CALLER, + { privileged: true }, + ); } async getIndexes(call: ParsedRouterRequest): Promise { @@ -703,7 +715,8 @@ export class SchemaAdmin { if (isNil(requestedSchema)) { throw new GrpcError(status.NOT_FOUND, 'Schema does not exist'); } - return this.database.getIndexes(requestedSchema.name); + const indexes = await this.database.getIndexes(requestedSchema.name); + return { indexes }; } async deleteIndexes(call: ParsedRouterRequest): Promise { @@ -723,6 +736,82 @@ export class SchemaAdmin { return this.database.deleteIndexes(requestedSchema.name, indexNames); } + async exportIndexes(call: ParsedRouterRequest): Promise { + const skip = call.request.params.skip ?? 0; + const limit = call.request.params.limit ?? 25; + const query: Indexable = { + name: { $nin: this.database.systemSchemas }, + $or: [ + { parentSchema: { $exists: false } }, + { parentSchema: { $eq: null } }, + { parentSchema: { $eq: '' } }, + ], + }; + const schemaAdapter = this.database.getSchemaModel('_DeclaredSchema'); + const [schemas, count] = await Promise.all([ + schemaAdapter.model.findMany(query, { + skip, + limit, + select: 'name', + sort: { name: 1 }, + }), + schemaAdapter.model.countDocuments(query), + ]); + const indexes: Array = []; + for (const schema of schemas) { + if (!this.database.models[schema.name]) continue; + const schemaIndexes = await this.database.getIndexes(schema.name); + if (isNil(schemaIndexes) || isEmpty(schemaIndexes)) continue; + indexes.push( + ...schemaIndexes.map(index => ({ ...index, schemaName: schema.name })), + ); + } + return { indexes, count }; + } + + async importIndexes(call: ParsedRouterRequest): Promise { + const { indexes } = call.request.params as { + indexes: Array; + }; + if (!Array.isArray(indexes) || indexes.length === 0) { + throw new GrpcError(status.INVALID_ARGUMENT, 'indexes must be a non-empty array'); + } + const bySchema = new Map(); + for (const entry of indexes) { + const { schemaName, ...rest } = entry; + if (!schemaName) { + throw new GrpcError( + status.INVALID_ARGUMENT, + 'Each imported index needs schemaName', + ); + } + const bucket = bySchema.get(schemaName) ?? []; + bucket.push(rest); + bySchema.set(schemaName, bucket); + } + for (const [schemaName, schemaIndexes] of bySchema) { + if (!this.database.models[schemaName]) { + throw new GrpcError( + status.NOT_FOUND, + `Requested schema not found: ${schemaName}`, + ); + } + const existing = await this.database.getIndexes(schemaName); + const existingNames = collectExistingIndexNames(existing); + const toCreate = schemaIndexes + .map(index => ensureIndexName(index)) + .filter(index => { + const name = resolveIndexName(index); + return !name || !existingNames.has(name); + }); + if (toCreate.length === 0) continue; + await this.database.createIndexes(schemaName, toCreate, ADMIN_INDEX_CALLER, { + privileged: false, + }); + } + return 'Indexes imported successfully'; + } + async checkRequestedSchema(id: string) { const requestedSchema = await this.database .getSchemaModel('_DeclaredSchema') diff --git a/modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts b/modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts new file mode 100644 index 000000000..a2d6e52fa --- /dev/null +++ b/modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from '@jest/globals'; +import { CompatibleIndexType } from '@conduitplatform/grpc-sdk'; +import { validateSchemaInput } from '../utilities.js'; + +describe('validateModelOptions T35 T36', () => { + it('T35 accepts modelOptions.indexes', () => { + expect(() => + validateSchemaInput( + 'User', + { email: 'String' }, + { + timestamps: true, + indexes: [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + }, + ), + ).not.toThrow(); + }); + + it('T36 keeps conduit.readPreference while accepting indexes', () => { + expect(() => + validateSchemaInput( + 'User', + { email: 'String' }, + { + timestamps: true, + indexes: [{ fields: ['email'] }], + conduit: { readPreference: 'secondaryPreferred' }, + }, + ), + ).not.toThrow(); + }); + + it('still rejects unknown conduit keys and unknown model option keys', () => { + expect(() => + validateSchemaInput('User', { email: 'String' }, { + unknown: true, + } as any), + ).toThrow(/indexes/); + expect(() => + validateSchemaInput('User', { email: 'String' }, { + conduit: { notARealKey: true }, + } as any), + ).toThrow(/readPreference/); + }); +}); diff --git a/modules/database/src/utils/utilities.ts b/modules/database/src/utils/utilities.ts index ce6cd7328..b44be8998 100644 --- a/modules/database/src/utils/utilities.ts +++ b/modules/database/src/utils/utilities.ts @@ -156,10 +156,12 @@ const ALLOWED_CONDUIT_READ_PREFERENCES = [ function validateModelOptions(modelOptions: ConduitSchemaOptions) { if (!isPlainObject(modelOptions)) throw new Error('Model options must be an object'); Object.keys(modelOptions).forEach(key => { - if (key !== 'conduit' && key !== 'timestamps') - throw new Error("Only 'conduit' and 'timestamps' options allowed"); + if (key !== 'conduit' && key !== 'timestamps' && key !== 'indexes') + throw new Error("Only 'conduit', 'timestamps', and 'indexes' options allowed"); else if (key === 'timestamps' && !isBoolean(modelOptions.timestamps)) throw new Error("Option 'timestamps' must be of type Boolean"); + else if (key === 'indexes' && !isArray(modelOptions.indexes)) + throw new Error("Option 'indexes' must be of type Array"); else if (key === 'conduit') { if (!isObject(modelOptions.conduit)) throw new Error("Option 'conduit' must be of type Object"); From 54ce232f560f1fa169f1a4c8161db8200ae26f13 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 14 Sep 2026 11:57:25 +0000 Subject: [PATCH 2/4] fix(database): typecheck dialect indexes and group tests Repair the ModelOptionsIndexTypes union that failed tsc, stop stuffing Sequelize field objects into Conduit index types, and move the index suites into src/__tests__/indexes. --- libraries/grpc-sdk/src/interfaces/Model.ts | 3 +- .../src/__tests__/indexes/adapters.test.ts | 242 ++++++++++++++++++ .../indexes/admin.test.ts} | 52 +++- .../src/__tests__/indexes/converters.test.ts | 195 ++++++++++++++ .../indexes/helpers.test.ts} | 116 ++++----- .../regressions.test.ts} | 31 ++- .../__tests__/platform-models.indexes.test.ts | 33 --- .../mongoose-adapter/SchemaConverter.ts | 55 ++-- .../__tests__/SchemaConverter.indexes.test.ts | 66 ----- .../__tests__/indexes.adapter.test.ts | 123 --------- .../src/adapters/mongoose-adapter/index.ts | 43 ++-- .../__tests__/indexes.adapter.test.ts | 128 --------- .../src/adapters/sequelize-adapter/index.ts | 64 ++--- .../__tests__/sql-index-converters.test.ts | 125 --------- .../utils/database-transform-utils.ts | 160 ++++++------ .../database/src/adapters/utils/indexes.ts | 108 ++++---- modules/database/src/admin/schema.admin.ts | 32 +-- .../validateModelOptions.indexes.test.ts | 45 ---- 18 files changed, 766 insertions(+), 855 deletions(-) create mode 100644 modules/database/src/__tests__/indexes/adapters.test.ts rename modules/database/src/{admin/__tests__/schema.admin.indexes.test.ts => __tests__/indexes/admin.test.ts} (71%) create mode 100644 modules/database/src/__tests__/indexes/converters.test.ts rename modules/database/src/{adapters/utils/__tests__/indexes.test.ts => __tests__/indexes/helpers.test.ts} (70%) rename modules/database/src/__tests__/{no-old-pr-bugs.test.ts => indexes/regressions.test.ts} (53%) delete mode 100644 modules/database/src/__tests__/platform-models.indexes.test.ts delete mode 100644 modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts delete mode 100644 modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts delete mode 100644 modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts delete mode 100644 modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts delete mode 100644 modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts diff --git a/libraries/grpc-sdk/src/interfaces/Model.ts b/libraries/grpc-sdk/src/interfaces/Model.ts index f5600386c..b3451372b 100644 --- a/libraries/grpc-sdk/src/interfaces/Model.ts +++ b/libraries/grpc-sdk/src/interfaces/Model.ts @@ -52,8 +52,7 @@ export enum CompatibleIndexType { export type IndexType = MongoIndexType | PostgresIndexType | CompatibleIndexType; -export type ModelOptionsIndexTypes = - MongoIndexType[] | PostgresIndexType | CompatibleIndexType | CompatibleIndexType[]; +export type ModelOptionsIndexTypes = IndexType | readonly IndexType[]; export type Array = any[]; diff --git a/modules/database/src/__tests__/indexes/adapters.test.ts b/modules/database/src/__tests__/indexes/adapters.test.ts new file mode 100644 index 000000000..cb0810497 --- /dev/null +++ b/modules/database/src/__tests__/indexes/adapters.test.ts @@ -0,0 +1,242 @@ +import { describe, expect, it, jest } from '@jest/globals'; +import { status } from '@grpc/grpc-js'; +import { + CompatibleIndexType, + MongoIndexType, + PostgresIndexType, +} from '@conduitplatform/grpc-sdk'; +import { MongooseAdapter } from '../../adapters/mongoose-adapter/index.js'; +import { SequelizeAdapter } from '../../adapters/sequelize-adapter/index.js'; + +function makeMongooseAdapter(overrides: Record = {}) { + const createIndex = jest.fn().mockResolvedValue('email_1'); + const dropIndex = jest.fn().mockResolvedValue(undefined); + const indexes = jest.fn().mockResolvedValue([ + { v: 2, key: { _id: 1 }, name: '_id_' }, + { v: 2, key: { email: 1 }, name: 'cnd_idx_email_asc', unique: false }, + ]); + const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); + const findByIdAndUpdate = jest.fn().mockResolvedValue({}); + const adapter = Object.create(MongooseAdapter.prototype) as MongooseAdapter; + adapter.mongoose = { + model: () => ({ collection: { createIndex, dropIndex, indexes } }), + } as MongooseAdapter['mongoose']; + const originalSchema = { + name: 'User', + ownerModule: 'chat', + collectionName: 'cnd_User', + fields: { email: { type: 'String' } }, + compiledFields: { email: { type: 'String' } }, + modelOptions: { indexes: [] as unknown[] }, + ...((overrides.originalSchema as object) ?? {}), + }; + adapter.models = { + User: { originalSchema }, + _DeclaredSchema: { findOne, findByIdAndUpdate }, + } as MongooseAdapter['models']; + return { adapter, createIndex, dropIndex, indexes, findByIdAndUpdate, originalSchema }; +} + +class TestSequelizeAdapter extends SequelizeAdapter { + protected async hasLegacyCollections(): Promise { + return false; + } +} + +function makeSequelizeAdapter(dialect = 'postgres') { + const addIndex = jest.fn().mockResolvedValue(undefined); + const removeIndex = jest.fn().mockResolvedValue(undefined); + const showIndex = jest.fn().mockResolvedValue([ + { + name: 'cnd_idx_email_asc', + unique: false, + fields: [{ attribute: 'email', order: 'ASC' }], + definition: 'CREATE INDEX cnd_idx_email_asc ON cnd_User USING btree (email)', + }, + ]); + const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); + const findByIdAndUpdate = jest.fn().mockResolvedValue({}); + const sync = jest.fn().mockResolvedValue(undefined); + const adapter = Object.create(TestSequelizeAdapter.prototype) as SequelizeAdapter; + adapter.sequelize = { + getDialect: () => dialect, + getQueryInterface: () => ({ addIndex, removeIndex, showIndex }), + } as SequelizeAdapter['sequelize']; + const originalSchema = { + name: 'User', + ownerModule: 'database', + collectionName: 'custom_users', + fields: { email: { type: 'String' } }, + compiledFields: { email: { type: 'String' } }, + modelOptions: { indexes: [] as unknown[] }, + }; + adapter.models = { + User: { originalSchema, sync }, + _DeclaredSchema: { findOne, findByIdAndUpdate }, + } as SequelizeAdapter['models']; + return { + adapter, + addIndex, + removeIndex, + showIndex, + sync, + findByIdAndUpdate, + originalSchema, + }; +} + +describe('mongoose adapter indexes', () => { + it('creates a single key spec object, not an array of objects', async () => { + const { adapter, createIndex } = makeMongooseAdapter(); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'chat', + ); + expect(createIndex).toHaveBeenCalledTimes(1); + expect(createIndex.mock.calls[0][0]).toEqual({ email: MongoIndexType.Ascending }); + expect(Array.isArray(createIndex.mock.calls[0][0])).toBe(false); + }); + + it('persists created and deleted indexes on _DeclaredSchema', async () => { + const created = makeMongooseAdapter(); + await created.adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'chat', + ); + expect(created.findByIdAndUpdate).toHaveBeenCalledTimes(1); + const update = created.findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes[0].name).toMatch(/email/); + + const deleted = makeMongooseAdapter({ + originalSchema: { + modelOptions: { indexes: [{ fields: ['email'], name: 'cnd_idx_email_asc' }] }, + }, + }); + await deleted.adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + expect(deleted.originalSchema.modelOptions.indexes).toEqual([]); + expect(deleted.findByIdAndUpdate).toHaveBeenCalled(); + }); + + it('awaits dropIndex', async () => { + const { adapter, dropIndex } = makeMongooseAdapter(); + let resolveDrop: () => void = () => undefined; + const dropped = new Promise(resolve => { + resolveDrop = resolve; + }); + dropIndex.mockImplementation(async () => { + resolveDrop(); + }); + await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + await dropped; + expect(dropIndex).toHaveBeenCalledWith('cnd_idx_email_asc'); + }); + + it('reads live engine indexes', async () => { + const { adapter, indexes } = makeMongooseAdapter(); + const result = await adapter.getIndexes('User'); + expect(indexes).toHaveBeenCalled(); + expect(result.map(index => index.name)).toEqual(['_id_', 'cnd_idx_email_asc']); + expect(result[1].fields).toEqual(['email']); + }); + + it('throws on Admin-bound invalid types and allows privileged unique', async () => { + const { adapter } = makeMongooseAdapter(); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [PostgresIndexType.GIST] }], + 'database', + { privileged: true }, + ), + ).rejects.toMatchObject({ code: status.INVALID_ARGUMENT }); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], options: { unique: true } }], + 'database', + { privileged: true }, + ), + ).resolves.toBe('Indexes created!'); + }); +}); + +describe('sequelize adapter indexes', () => { + it('keeps getDatabaseType as PostgreSQL', () => { + const { adapter } = makeSequelizeAdapter('postgres'); + expect(adapter.getDatabaseType()).toBe('PostgreSQL'); + }); + + it('uses originalSchema.collectionName instead of a hardcoded cnd_ prefix', async () => { + const { adapter, addIndex, removeIndex, showIndex } = makeSequelizeAdapter(); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'database', + { privileged: true }, + ); + expect(addIndex.mock.calls[0][0]).toBe('custom_users'); + expect(addIndex.mock.calls[0][0]).not.toBe('cnd_User'); + await adapter.getIndexes('User'); + expect(showIndex).toHaveBeenCalledWith('custom_users'); + await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + expect(removeIndex).toHaveBeenCalledWith('custom_users', 'cnd_idx_email_asc'); + }); + + it('does not rebuild or sync the schema when creating indexes', async () => { + const { adapter, sync } = makeSequelizeAdapter(); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'database', + ); + expect(sync).not.toHaveBeenCalled(); + }); + + it('reads the live engine and overlays declared Compatible types', async () => { + const { adapter, showIndex, originalSchema } = makeSequelizeAdapter(); + originalSchema.modelOptions.indexes = [ + { + fields: ['email'], + name: 'cnd_idx_email_asc', + types: [CompatibleIndexType.Ascending], + }, + ]; + const result = await adapter.getIndexes('User'); + expect(showIndex).toHaveBeenCalledWith('custom_users'); + expect(result[0].types).toEqual([CompatibleIndexType.Ascending]); + expect(result[0].fields).toEqual(['email']); + }); + + it('awaits removeIndex and persists the deletion', async () => { + const { adapter, removeIndex, findByIdAndUpdate } = makeSequelizeAdapter(); + await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); + expect(removeIndex).toHaveBeenCalledTimes(1); + expect(findByIdAndUpdate).toHaveBeenCalled(); + }); + + it('allows HASH on mysql and rejects it on sqlite', async () => { + const mysql = makeSequelizeAdapter('mysql'); + await expect( + mysql.adapter.createIndexes( + 'User', + [{ fields: ['email'], types: PostgresIndexType.HASH }], + 'database', + { privileged: true }, + ), + ).resolves.toBe('Indexes created!'); + + const sqlite = makeSequelizeAdapter('sqlite'); + await expect( + sqlite.adapter.createIndexes( + 'User', + [{ fields: ['email'], types: PostgresIndexType.HASH }], + 'database', + { privileged: true }, + ), + ).rejects.toMatchObject({ message: expect.stringMatching(/sqlite/i) }); + }); +}); diff --git a/modules/database/src/admin/__tests__/schema.admin.indexes.test.ts b/modules/database/src/__tests__/indexes/admin.test.ts similarity index 71% rename from modules/database/src/admin/__tests__/schema.admin.indexes.test.ts rename to modules/database/src/__tests__/indexes/admin.test.ts index 86627f6d5..757954193 100644 --- a/modules/database/src/admin/__tests__/schema.admin.indexes.test.ts +++ b/modules/database/src/__tests__/indexes/admin.test.ts @@ -1,13 +1,18 @@ import { describe, expect, it, jest } from '@jest/globals'; import { status } from '@grpc/grpc-js'; -import { ConduitGrpcSdk, ParsedRouterRequest } from '@conduitplatform/grpc-sdk'; -import { SchemaAdmin } from '../schema.admin.js'; +import { + CompatibleIndexType, + ConduitGrpcSdk, + ParsedRouterRequest, +} from '@conduitplatform/grpc-sdk'; +import { SchemaAdmin } from '../../admin/schema.admin.js'; import { DatabaseAdapter } from '../../adapters/DatabaseAdapter.js'; import { MongooseSchema } from '../../adapters/mongoose-adapter/MongooseSchema.js'; import { SequelizeSchema } from '../../adapters/sequelize-adapter/SequelizeSchema.js'; import { SchemaController } from '../../controllers/cms/schema.controller.js'; import { CustomEndpointController } from '../../controllers/customEndpoints/customEndpoint.controller.js'; import { ADMIN_INDEX_CALLER } from '../../adapters/utils/indexes.js'; +import { validateSchemaInput } from '../../utils/utilities.js'; function makeCall(params: Record): ParsedRouterRequest { return { request: { params } } as unknown as ParsedRouterRequest; @@ -49,8 +54,8 @@ function setup() { return { admin, createIndexes, getIndexes, findMany, countDocuments, findOne }; } -describe('SchemaAdmin indexes T35–T42', () => { - it('T37 Admin createIndexes is privileged', async () => { +describe('SchemaAdmin indexes', () => { + it('creates indexes as a privileged Admin caller', async () => { const { admin, createIndexes } = setup(); await admin.createIndexes( makeCall({ @@ -66,7 +71,7 @@ describe('SchemaAdmin indexes T35–T42', () => { ); }); - it('T39 exportIndexes is paginated (skip/limit, no unbounded findMany)', async () => { + it('exports indexes with skip/limit pagination', async () => { const { admin, findMany, countDocuments, getIndexes } = setup(); const result = (await admin.exportIndexes(makeCall({ skip: 10, limit: 5 }))) as { indexes: unknown[]; @@ -82,7 +87,7 @@ describe('SchemaAdmin indexes T35–T42', () => { expect(result.indexes.every(index => 'schemaName' in (index as object))).toBe(true); }); - it('T40 importIndexes skips same-name indexes', async () => { + it('skips same-name indexes on import', async () => { const { admin, createIndexes, getIndexes } = setup(); getIndexes.mockResolvedValue([{ name: 'keep_me', fields: ['email'] }]); await admin.importIndexes( @@ -95,10 +100,10 @@ describe('SchemaAdmin indexes T35–T42', () => { ); expect(createIndexes).toHaveBeenCalledTimes(1); const created = createIndexes.mock.calls[0][1] as { name?: string }[]; - expect(created.map(i => i.name)).toEqual(['new_name']); + expect(created.map(index => index.name)).toEqual(['new_name']); }); - it('T41 import unique is not Admin-privileged (respects owner)', async () => { + it('imports unique indexes without Admin privilege so owner rules apply', async () => { const { admin, createIndexes } = setup(); await admin.importIndexes( makeCall({ @@ -120,10 +125,39 @@ describe('SchemaAdmin indexes T35–T42', () => { ); }); - it('T42 import/export are Admin handlers and reject empty import', async () => { + it('rejects an empty import payload', async () => { const { admin } = setup(); await expect(admin.importIndexes(makeCall({ indexes: [] }))).rejects.toMatchObject({ code: status.INVALID_ARGUMENT, }); }); }); + +describe('validateModelOptions indexes', () => { + it('accepts modelOptions.indexes and conduit.readPreference together', () => { + expect(() => + validateSchemaInput( + 'User', + { email: 'String' }, + { + timestamps: true, + indexes: [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + conduit: { readPreference: 'secondaryPreferred' }, + }, + ), + ).not.toThrow(); + }); + + it('still rejects unknown conduit keys and unknown model option keys', () => { + expect(() => + validateSchemaInput('User', { email: 'String' }, { unknown: true } as Parameters< + typeof validateSchemaInput + >[2]), + ).toThrow(/indexes/); + expect(() => + validateSchemaInput('User', { email: 'String' }, { + conduit: { notARealKey: true }, + } as Parameters[2]), + ).toThrow(/readPreference/); + }); +}); diff --git a/modules/database/src/__tests__/indexes/converters.test.ts b/modules/database/src/__tests__/indexes/converters.test.ts new file mode 100644 index 000000000..88843de97 --- /dev/null +++ b/modules/database/src/__tests__/indexes/converters.test.ts @@ -0,0 +1,195 @@ +import { beforeEach, describe, expect, it, jest } from '@jest/globals'; +import { + CompatibleIndexType, + ConduitGrpcSdk, + ConduitSchema, + MongoIndexType, + PostgresIndexType, + TYPE, +} from '@conduitplatform/grpc-sdk'; +import { + convertModelOptionsIndexes, + convertSchemaFieldIndexes, +} from '../../adapters/utils/database-transform-utils.js'; +import { sqlSchemaConverter } from '../../adapters/sequelize-adapter/sql-adapter/SqlSchemaConverter.js'; +import { pgSchemaConverter } from '../../adapters/sequelize-adapter/postgres-adapter/PgSchemaConverter.js'; +import { schemaConverter } from '../../adapters/mongoose-adapter/SchemaConverter.js'; + +type ConvertedSqlIndex = { + fields: Array; + using?: PostgresIndexType; +}; + +function schemaWithIndexes( + indexes: ConduitSchema['modelOptions']['indexes'], + fields: ConduitSchema['fields'] = { email: { type: TYPE.String } }, +) { + return new ConduitSchema('User', fields, { indexes }); +} + +describe('SQL index converters', () => { + beforeEach(() => { + jest.spyOn(ConduitGrpcSdk.Logger, 'warn').mockImplementation(() => undefined); + }); + + it('maps Compatible types to BTREE plus ASC/DESC on postgres and mysql', () => { + const postgres = convertModelOptionsIndexes( + schemaWithIndexes([{ fields: ['email'], types: [CompatibleIndexType.Descending] }]), + 'postgres', + ); + const pgIndex = postgres.modelOptions.indexes![0] as ConvertedSqlIndex; + expect(pgIndex.using).toBe(PostgresIndexType.BTREE); + expect(pgIndex.fields[0]).toEqual({ name: 'email', order: 'DESC' }); + + const mysql = convertModelOptionsIndexes( + schemaWithIndexes([{ fields: ['email'], types: [CompatibleIndexType.Ascending] }]), + 'mysql', + ); + const mysqlIndex = mysql.modelOptions.indexes![0] as ConvertedSqlIndex; + expect(mysqlIndex.using).toBe(PostgresIndexType.BTREE); + expect(mysqlIndex.fields[0]).toEqual({ name: 'email', order: 'ASC' }); + }); + + it('maps Compatible types to BTREE on sqlite', () => { + const copy = convertModelOptionsIndexes( + schemaWithIndexes([{ fields: ['email'], types: CompatibleIndexType.Ascending }]), + 'sqlite', + ); + expect((copy.modelOptions.indexes![0] as ConvertedSqlIndex).using).toBe( + PostgresIndexType.BTREE, + ); + }); + + it('warns and skips Mongo-only leftovers on SQL', () => { + const warn = ConduitGrpcSdk.Logger.warn as jest.Mock; + const copy = convertModelOptionsIndexes( + schemaWithIndexes([ + { fields: ['loc'], types: [MongoIndexType.GeoSpatial2dSphere] }, + { fields: ['email'], types: [CompatibleIndexType.Ascending] }, + ]), + 'postgres', + ); + expect(copy.modelOptions.indexes).toHaveLength(1); + expect(warn).toHaveBeenCalled(); + }); + + it('skips postgres-only types on mysql, mariadb, and sqlite', () => { + for (const dialect of ['mysql', 'mariadb', 'sqlite'] as const) { + const copy = convertModelOptionsIndexes( + schemaWithIndexes([{ fields: ['email'], types: PostgresIndexType.GIST }]), + dialect, + ); + expect(copy.modelOptions.indexes).toHaveLength(0); + } + }); + + it('converts field-level Compatible indexes into sequelize model indexes', () => { + const copy = convertSchemaFieldIndexes( + new ConduitSchema( + 'Perm', + { + resource: { + type: TYPE.String, + index: { type: CompatibleIndexType.Ascending }, + }, + }, + {}, + ), + 'mysql', + ); + expect(copy.modelOptions.indexes).toHaveLength(1); + expect((copy.fields.resource as { index?: unknown }).index).toBeUndefined(); + }); + + it('keeps HASH on mysql and drops it on sqlite', () => { + const schema = new ConduitSchema( + 'User', + { email: { type: TYPE.String } }, + { + indexes: [ + { fields: ['email'], types: [CompatibleIndexType.Descending] }, + { fields: ['email'], types: PostgresIndexType.HASH }, + ], + }, + ); + const [mysql] = sqlSchemaConverter(schema, 'mysql'); + const [sqlite] = sqlSchemaConverter(schema, 'sqlite'); + expect(mysql.modelOptions.indexes).toHaveLength(2); + expect(sqlite.modelOptions.indexes).toHaveLength(1); + }); + + it('keeps postgres-only types in the pg converter', () => { + const schema = new ConduitSchema( + 'User', + { email: { type: TYPE.String } }, + { + indexes: [{ fields: ['email'], types: PostgresIndexType.GIN }], + }, + ); + const [pg] = pgSchemaConverter(schema); + expect(pg.modelOptions.indexes).toHaveLength(1); + expect((pg.modelOptions.indexes![0] as ConvertedSqlIndex).using).toBe( + PostgresIndexType.GIN, + ); + }); +}); + +describe('mongoose SchemaConverter indexes', () => { + beforeEach(() => { + jest.spyOn(ConduitGrpcSdk.Logger, 'warn').mockImplementation(() => undefined); + }); + + it('treats Compatible types as Mongo 1/-1', () => { + const converted = schemaConverter( + new ConduitSchema( + 'User', + { + email: { + type: TYPE.String, + index: { type: CompatibleIndexType.Descending }, + }, + }, + {}, + ), + ); + expect( + (converted.fields.email as { index: { type: MongoIndexType } }).index.type, + ).toBe(MongoIndexType.Descending); + }); + + it('warns and skips postgres leftovers on Mongo', () => { + const warn = ConduitGrpcSdk.Logger.warn as jest.Mock; + const converted = schemaConverter( + new ConduitSchema( + 'User', + { + email: { + type: TYPE.String, + index: { type: PostgresIndexType.GIN }, + }, + }, + {}, + ), + ); + expect((converted.fields.email as { index?: unknown }).index).toBeUndefined(); + expect(warn).toHaveBeenCalled(); + }); + + it('maps the Compatible string Ascending, not a Mongo enum key', () => { + const converted = schemaConverter( + new ConduitSchema( + 'User', + { + email: { + type: TYPE.String, + index: { type: CompatibleIndexType.Ascending }, + }, + }, + {}, + ), + ); + expect( + (converted.fields.email as { index: { type: MongoIndexType } }).index.type, + ).toBe(MongoIndexType.Ascending); + }); +}); diff --git a/modules/database/src/adapters/utils/__tests__/indexes.test.ts b/modules/database/src/__tests__/indexes/helpers.test.ts similarity index 70% rename from modules/database/src/adapters/utils/__tests__/indexes.test.ts rename to modules/database/src/__tests__/indexes/helpers.test.ts index c5dbc5335..74be105b6 100644 --- a/modules/database/src/adapters/utils/__tests__/indexes.test.ts +++ b/modules/database/src/__tests__/indexes/helpers.test.ts @@ -23,10 +23,10 @@ import { sqlDialectAllowsIndexType, sqlIndexFields, validateIndexFields, -} from '../indexes.js'; +} from '../../adapters/utils/indexes.js'; -describe('index helpers T1–T16', () => { - it('T1 CompatibleIndexType uses portable string values, not Mongo 1/-1', () => { +describe('index helpers', () => { + it('keeps CompatibleIndexType as portable strings, not Mongo 1/-1', () => { expect(CompatibleIndexType.Ascending).toBe('Ascending'); expect(CompatibleIndexType.Descending).toBe('Descending'); expect(CompatibleIndexType.Ascending).not.toBe(MongoIndexType.Ascending); @@ -35,12 +35,25 @@ describe('index helpers T1–T16', () => { expect(isMongoIndexType('Ascending')).toBe(false); }); - it('T4 generates a name when missing', () => { + it('generates a deterministic name when one is missing', () => { const name = generateIndexName(['email'], [CompatibleIndexType.Ascending], false); - expect(name).toMatch(/^cnd_idx_email_asc$/); + expect(name).toBe('cnd_idx_email_asc'); + const unique = generateIndexName( + ['room', 'createdAt'], + [CompatibleIndexType.Ascending, CompatibleIndexType.Descending], + true, + ); + expect(unique).toBe( + generateIndexName( + ['room', 'createdAt'], + [CompatibleIndexType.Ascending, CompatibleIndexType.Descending], + true, + ), + ); + expect(unique).toMatch(/^cnd_uidx_/); }); - it('T5 keeps a provided name', () => { + it('keeps a provided name on the index and options', () => { const named = ensureIndexName({ fields: ['email'], name: 'custom_email_idx', @@ -49,41 +62,34 @@ describe('index helpers T1–T16', () => { expect(named.options?.name).toBe('custom_email_idx'); }); - it('T6 is deterministic for the same input', () => { - const a = generateIndexName( - ['room', 'createdAt'], - [CompatibleIndexType.Ascending, CompatibleIndexType.Descending], - true, - ); - const b = generateIndexName( - ['room', 'createdAt'], - [CompatibleIndexType.Ascending, CompatibleIndexType.Descending], - true, - ); - expect(a).toBe(b); - expect(a).toMatch(/^cnd_uidx_/); - }); - - it('T7 maps Compatible to Mongo 1/-1', () => { + it('maps Compatible and Mongo directions to engine types', () => { expect(mapCompatibleToMongo(CompatibleIndexType.Ascending)).toBe(1); expect(mapCompatibleToMongo(CompatibleIndexType.Descending)).toBe(-1); expect(mapCompatibleToMongo(undefined)).toBe(1); - }); - - it('T8 maps Compatible to SQL BTREE ASC/DESC field order', () => { expect(mapCompatibleToSqlOrder(CompatibleIndexType.Ascending)).toBe('ASC'); expect(mapCompatibleToSqlOrder(CompatibleIndexType.Descending)).toBe('DESC'); - const fields = sqlIndexFields({ - fields: ['createdAt', 'room'], - types: [CompatibleIndexType.Descending, CompatibleIndexType.Ascending], - }); - expect(fields).toEqual([ + expect(sqlIndexFields({ fields: ['createdAt', 'room'] })).toEqual([ + 'createdAt', + 'room', + ]); + expect( + sqlIndexFields({ + fields: ['createdAt', 'room'], + types: [CompatibleIndexType.Descending, CompatibleIndexType.Ascending], + }), + ).toEqual([ { name: 'createdAt', order: 'DESC' }, { name: 'room', order: 'ASC' }, ]); + expect( + sqlIndexFields({ + fields: ['createdAt'], + types: [MongoIndexType.Descending], + }), + ).toEqual([{ name: 'createdAt', order: 'DESC' }]); }); - it('T9 preserves the unique option on generated names', () => { + it('preserves unique when generating a name', () => { const unique = ensureIndexName({ fields: ['email'], types: [CompatibleIndexType.Ascending], @@ -93,7 +99,7 @@ describe('index helpers T1–T16', () => { expect(resolveIndexName(unique)).toMatch(/uidx/); }); - it('T10 persist helper merges incoming indexes by name and skips duplicates', () => { + it('merges declared indexes by name without overwriting the first', () => { const merged = mergeDeclaredIndexes( [{ fields: ['a'], name: 'idx_a' }], [ @@ -101,33 +107,35 @@ describe('index helpers T1–T16', () => { { fields: ['b'], name: 'idx_b' }, ], ); - expect(merged.map(i => i.name)).toEqual(['idx_a', 'idx_b']); + expect(merged.map(index => index.name)).toEqual(['idx_a', 'idx_b']); expect(merged[0].options?.unique).toBeUndefined(); }); - it('T11 persist helper removes indexes by name', () => { - const remaining = removeDeclaredIndexes( - [ - { fields: ['a'], name: 'idx_a' }, - { fields: ['b'], name: 'idx_b' }, - ], - ['idx_a'], - ); - expect(remaining).toEqual([{ fields: ['b'], name: 'idx_b' }]); + it('removes declared indexes and field-level index metadata by name', () => { + expect( + removeDeclaredIndexes( + [ + { fields: ['a'], name: 'idx_a' }, + { fields: ['b'], name: 'idx_b' }, + ], + ['idx_a'], + ), + ).toEqual([{ fields: ['b'], name: 'idx_b' }]); + expect( + removeIndexFromSchemaFields({ fields: { a: { index: { name: 'x' } } } }, 'x'), + ).toBe(true); }); - it('T12 validateIndexFields rejects unknown fields', () => { + it('rejects unknown index fields', () => { expect(() => validateIndexFields( { compiledFields: { email: 'String' }, fields: {} }, - { - fields: ['missing'], - }, + { fields: ['missing'] }, ), ).toThrow(/Invalid fields/); }); - it('T13 unique is denied for a non-owner, non-admin caller', () => { + it('enforces unique-index privilege for owner, Admin, and import', () => { expect(() => assertUniqueIndexPrivilege({ unique: true, @@ -136,9 +144,6 @@ describe('index helpers T1–T16', () => { privileged: false, }), ).toThrow(expect.objectContaining({ code: status.PERMISSION_DENIED })); - }); - - it('T14 unique is allowed for the schema owner', () => { expect(() => assertUniqueIndexPrivilege({ unique: true, @@ -147,9 +152,6 @@ describe('index helpers T1–T16', () => { privileged: false, }), ).not.toThrow(); - }); - - it('T15 unique is allowed for privileged Admin', () => { expect(() => assertUniqueIndexPrivilege({ unique: true, @@ -158,9 +160,6 @@ describe('index helpers T1–T16', () => { privileged: true, }), ).not.toThrow(); - }); - - it('T16 import unique respects owner privilege (not Admin-privileged)', () => { expect(() => assertUniqueIndexPrivilege({ unique: true, @@ -178,12 +177,9 @@ describe('index helpers T1–T16', () => { ), ).toBe(true); expect(isIndexAlreadyExistsError(new Error('index already exists'))).toBe(true); - expect( - removeIndexFromSchemaFields({ fields: { a: { index: { name: 'x' } } } }, 'x'), - ).toBe(true); }); - it('T25 dialect checks use real switches, not `mysql || mariadb`', () => { + it('allows dialect-native types and rejects foreign leftovers', () => { expect(sqlDialectAllowsIndexType('mysql', CompatibleIndexType.Ascending)).toBe(true); expect(sqlDialectAllowsIndexType('mariadb', CompatibleIndexType.Descending)).toBe( true, diff --git a/modules/database/src/__tests__/no-old-pr-bugs.test.ts b/modules/database/src/__tests__/indexes/regressions.test.ts similarity index 53% rename from modules/database/src/__tests__/no-old-pr-bugs.test.ts rename to modules/database/src/__tests__/indexes/regressions.test.ts index 68a155907..5a13612c8 100644 --- a/modules/database/src/__tests__/no-old-pr-bugs.test.ts +++ b/modules/database/src/__tests__/indexes/regressions.test.ts @@ -1,7 +1,36 @@ -import { readFileSync } from 'fs'; +import { existsSync, readFileSync } from 'fs'; import { resolve } from 'path'; import { describe, expect, it } from '@jest/globals'; +function repoFile(...parts: string[]) { + const candidates = [ + resolve(process.cwd(), '..', ...parts), + resolve(process.cwd(), ...parts), + resolve(process.cwd(), '../..', ...parts), + ]; + const found = candidates.find(existsSync); + if (!found) throw new Error(`Missing ${parts.join('/')}`); + return found; +} + +describe('platform models use CompatibleIndexType', () => { + it('authz and chat schemas declare Compatible indexes, not Mongo-only types', () => { + const files = [ + repoFile('authorization', 'src', 'models', 'Permission.schema.ts'), + repoFile('authorization', 'src', 'models', 'Relationship.schema.ts'), + repoFile('authorization', 'src', 'models', 'ActorIndex.schema.ts'), + repoFile('authorization', 'src', 'models', 'ObjectIndex.schema.ts'), + repoFile('chat', 'src', 'models', 'ChatRoom.schema.ts'), + repoFile('chat', 'src', 'models', 'Message.schema.ts'), + ]; + for (const file of files) { + const source = readFileSync(file, 'utf8'); + expect(source).toContain('CompatibleIndexType'); + expect(source).not.toContain('MongoIndexType'); + } + }); +}); + describe('do not port old PR #643 bugs', () => { it("does not use `case 'mysql' || 'mariadb'`", () => { const files = [ diff --git a/modules/database/src/__tests__/platform-models.indexes.test.ts b/modules/database/src/__tests__/platform-models.indexes.test.ts deleted file mode 100644 index e5baea9a5..000000000 --- a/modules/database/src/__tests__/platform-models.indexes.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { existsSync, readFileSync } from 'fs'; -import { resolve } from 'path'; -import { describe, expect, it } from '@jest/globals'; - -function repoFile(...parts: string[]) { - const candidates = [ - resolve(process.cwd(), '..', ...parts), - resolve(process.cwd(), ...parts), - resolve(process.cwd(), '../..', ...parts), - ]; - const found = candidates.find(existsSync); - if (!found) throw new Error(`Missing ${parts.join('/')}`); - return found; -} - -const files = [ - repoFile('authorization', 'src', 'models', 'Permission.schema.ts'), - repoFile('authorization', 'src', 'models', 'Relationship.schema.ts'), - repoFile('authorization', 'src', 'models', 'ActorIndex.schema.ts'), - repoFile('authorization', 'src', 'models', 'ObjectIndex.schema.ts'), - repoFile('chat', 'src', 'models', 'ChatRoom.schema.ts'), - repoFile('chat', 'src', 'models', 'Message.schema.ts'), -]; - -describe('platform models T7 CompatibleIndexType', () => { - it('authz + chat schemas declare Compatible indexes, not Mongo-only types', () => { - for (const file of files) { - const source = readFileSync(file, 'utf8'); - expect(source).toContain('CompatibleIndexType'); - expect(source).not.toContain('MongoIndexType'); - } - }); -}); diff --git a/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts b/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts index d44524917..ea90330d5 100644 --- a/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts +++ b/modules/database/src/adapters/mongoose-adapter/SchemaConverter.ts @@ -11,9 +11,9 @@ import { cloneDeep, isArray, isNil, isObject } from 'lodash-es'; import { checkIfMongoOptions } from './utils.js'; import { isCompatibleIndexType, - isMongoIndexType, mapCompatibleToMongo, mongoAllowsIndexType, + normalizeIndexTypes, } from '../utils/indexes.js'; import * as deepdash from 'deepdash-es/standalone'; @@ -139,12 +139,11 @@ function convertSchemaFieldIndexes(copy: ConduitSchema) { function convertModelOptionsIndexes(copy: ConduitSchema) { if (!copy.modelOptions.indexes?.length) return copy; - const mutIndexes = copy.modelOptions.indexes as ModelOptionsIndexes[]; - for (const index of [...mutIndexes]) { + const remaining: ModelOptionsIndexes[] = []; + for (const index of copy.modelOptions.indexes) { + let mappedTypes: MongoIndexType[] | undefined; if (index.types) { - const types = isArray(index.types) - ? index.types - : index.fields.map(() => index.types); + const types = normalizeIndexTypes(index.types, index.fields.length) ?? []; if ( types.some(type => !mongoAllowsIndexType(type)) || (isArray(index.types) && index.fields.length !== index.types.length) @@ -152,41 +151,31 @@ function convertModelOptionsIndexes(copy: ConduitSchema) { ConduitGrpcSdk.Logger.warn( `Invalid index type for MongoDB found in '${copy.name}', ignoring index`, ); - mutIndexes.splice(mutIndexes.indexOf(index), 1); continue; } - index.types = types.map(type => - isCompatibleIndexType(type) || isMongoIndexType(type) - ? mapCompatibleToMongo(type) - : (type as MongoIndexType), - ) as MongoIndexType[]; + mappedTypes = types.map(mapCompatibleToMongo); + index.types = mappedTypes; + } + // compound indexes stay on modelOptions and are created after schema creation + if (index.fields.length !== 1) { + remaining.push(index); + continue; } - // compound indexes are maintained in modelOptions in order to be created after schema creation - // single field index => add it to specified schema field - if (index.fields.length !== 1) continue; const modelField = copy.fields[index.fields[0]] as ConduitModelField; if (!modelField) { throw new Error(`Field ${index.fields[0]} in index definition doesn't exist`); } - if (index.types) { - modelField.index = { - type: (index.types as MongoIndexType[])[0], - }; - } - if (index.options) { - if (!checkIfMongoOptions(index.options)) { - ConduitGrpcSdk.Logger.warn( - `Invalid index options for MongoDB found in '${copy.name}', ignoring index`, - ); - mutIndexes.splice(mutIndexes.indexOf(index), 1); - continue; - } - if (!modelField.index) modelField.index = {}; - for (const [option, optionValue] of Object.entries(index.options)) { - modelField.index![option as keyof SchemaFieldIndex] = optionValue; - } + if (index.options && !checkIfMongoOptions(index.options)) { + ConduitGrpcSdk.Logger.warn( + `Invalid index options for MongoDB found in '${copy.name}', ignoring index`, + ); + continue; } - mutIndexes.splice(mutIndexes.indexOf(index), 1); + modelField.index = { + ...(mappedTypes ? { type: mappedTypes[0] } : {}), + ...index.options, + }; } + copy.modelOptions.indexes = remaining; return copy; } diff --git a/modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts b/modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts deleted file mode 100644 index daeb74055..000000000 --- a/modules/database/src/adapters/mongoose-adapter/__tests__/SchemaConverter.indexes.test.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { beforeEach, describe, expect, it, jest } from '@jest/globals'; -import { - CompatibleIndexType, - ConduitGrpcSdk, - ConduitSchema, - MongoIndexType, - PostgresIndexType, - TYPE, -} from '@conduitplatform/grpc-sdk'; -import { schemaConverter } from '../SchemaConverter.js'; - -describe('mongoose SchemaConverter indexes T17 T23', () => { - beforeEach(() => { - jest.spyOn(ConduitGrpcSdk.Logger, 'warn').mockImplementation(() => {}); - }); - - it('T17 treats Compatible as Mongo 1/-1', () => { - const converted = schemaConverter( - new ConduitSchema( - 'User', - { - email: { - type: TYPE.String, - index: { type: CompatibleIndexType.Descending }, - }, - } as any, - {}, - ), - ); - expect((converted.fields.email as any).index.type).toBe(MongoIndexType.Descending); - }); - - it('T23 recover: postgres leftovers on Mongo are warned and skipped', () => { - const warn = ConduitGrpcSdk.Logger.warn as jest.Mock; - const converted = schemaConverter( - new ConduitSchema( - 'User', - { - email: { - type: TYPE.String, - index: { type: PostgresIndexType.GIN }, - }, - } as any, - {}, - ), - ); - expect((converted.fields.email as any).index).toBeUndefined(); - expect(warn).toHaveBeenCalled(); - }); - - it('does not treat the Mongo enum key "Ascending" as a valid Mongo type', () => { - const converted = schemaConverter( - new ConduitSchema( - 'User', - { - email: { - type: TYPE.String, - index: { type: 'Ascending' as any }, - }, - } as any, - {}, - ), - ); - expect((converted.fields.email as any).index.type).toBe(MongoIndexType.Ascending); - }); -}); diff --git a/modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts b/modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts deleted file mode 100644 index 681228b26..000000000 --- a/modules/database/src/adapters/mongoose-adapter/__tests__/indexes.adapter.test.ts +++ /dev/null @@ -1,123 +0,0 @@ -import { describe, expect, it, jest } from '@jest/globals'; -import { status } from '@grpc/grpc-js'; -import { CompatibleIndexType, MongoIndexType } from '@conduitplatform/grpc-sdk'; -import { MongooseAdapter } from '../index.js'; - -function makeAdapter(overrides: Record = {}) { - const createIndex = jest.fn().mockResolvedValue('email_1'); - const dropIndex = jest.fn().mockResolvedValue(undefined); - const indexes = jest.fn().mockResolvedValue([ - { v: 2, key: { _id: 1 }, name: '_id_' }, - { v: 2, key: { email: 1 }, name: 'cnd_idx_email_asc', unique: false }, - ]); - const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); - const findByIdAndUpdate = jest.fn().mockResolvedValue({}); - const adapter = Object.create(MongooseAdapter.prototype) as MongooseAdapter; - adapter.mongoose = { - model: () => ({ collection: { createIndex, dropIndex, indexes } }), - } as any; - const originalSchema = { - name: 'User', - ownerModule: 'chat', - collectionName: 'cnd_User', - fields: { email: { type: 'String' } }, - compiledFields: { email: { type: 'String' } }, - modelOptions: { indexes: [] as unknown[] }, - ...((overrides.originalSchema as object) ?? {}), - }; - adapter.models = { - User: { originalSchema }, - _DeclaredSchema: { findOne, findByIdAndUpdate }, - } as any; - return { adapter, createIndex, dropIndex, indexes, findByIdAndUpdate, originalSchema }; -} - -describe('mongoose adapter indexes T26–T29 T34 T38', () => { - it('T26 createIndex uses a single key spec object, not an array of objects', async () => { - const { adapter, createIndex } = makeAdapter(); - await adapter.createIndexes( - 'User', - [ - { - fields: ['email'], - types: [CompatibleIndexType.Ascending], - }, - ], - 'chat', - ); - expect(createIndex).toHaveBeenCalledTimes(1); - expect(createIndex.mock.calls[0][0]).toEqual({ email: MongoIndexType.Ascending }); - expect(Array.isArray(createIndex.mock.calls[0][0])).toBe(false); - }); - - it('T27 create persists metadata into _DeclaredSchema', async () => { - const { adapter, findByIdAndUpdate } = makeAdapter(); - await adapter.createIndexes( - 'User', - [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], - 'chat', - ); - expect(findByIdAndUpdate).toHaveBeenCalledTimes(1); - const update = findByIdAndUpdate.mock.calls[0][1] as { - modelOptions: { indexes: { name?: string }[] }; - }; - expect(update.modelOptions.indexes[0].name).toMatch(/email/); - }); - - it('T28 delete awaits dropIndex', async () => { - const { adapter, dropIndex } = makeAdapter(); - let resolveDrop: () => void = () => {}; - const dropped = new Promise(resolve => { - resolveDrop = resolve; - }); - dropIndex.mockImplementation(async () => { - resolveDrop(); - }); - await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); - await dropped; - expect(dropIndex).toHaveBeenCalledWith('cnd_idx_email_asc'); - }); - - it('T29 delete persists removal', async () => { - const { adapter, findByIdAndUpdate, originalSchema } = makeAdapter({ - originalSchema: { - modelOptions: { indexes: [{ fields: ['email'], name: 'cnd_idx_email_asc' }] }, - }, - }); - await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); - expect(originalSchema.modelOptions.indexes).toEqual([]); - expect(findByIdAndUpdate).toHaveBeenCalled(); - }); - - it('T34 getIndexes uses the live engine as source of truth', async () => { - const { adapter, indexes } = makeAdapter(); - const result = await adapter.getIndexes('User'); - expect(indexes).toHaveBeenCalled(); - expect(result.map(i => i.name)).toEqual(['_id_', 'cnd_idx_email_asc']); - expect(result[1].fields).toEqual(['email']); - }); - - it('T38 Admin-bound invalid types throw', async () => { - const { adapter } = makeAdapter(); - await expect( - adapter.createIndexes( - 'User', - [{ fields: ['email'], types: ['GIST'] as any }], - 'database', - { privileged: true }, - ), - ).rejects.toMatchObject({ code: status.INVALID_ARGUMENT }); - }); - - it('T15 Admin privileged unique is allowed on a foreign-owned schema', async () => { - const { adapter } = makeAdapter(); - await expect( - adapter.createIndexes( - 'User', - [{ fields: ['email'], options: { unique: true } }], - 'database', - { privileged: true }, - ), - ).resolves.toBe('Indexes created!'); - }); -}); diff --git a/modules/database/src/adapters/mongoose-adapter/index.ts b/modules/database/src/adapters/mongoose-adapter/index.ts index e14298eae..aeaa53a2e 100644 --- a/modules/database/src/adapters/mongoose-adapter/index.ts +++ b/modules/database/src/adapters/mongoose-adapter/index.ts @@ -14,17 +14,16 @@ import { DatabaseAdapter } from '../DatabaseAdapter.js'; import { validateFieldChanges, validateFieldConstraints } from '../utils/index.js'; import { assertUniqueIndexPrivilege, + declaredIndexMap, ensureIndexName, - isCompatibleIndexType, isIndexAlreadyExistsError, - isMongoIndexType, mapCompatibleToMongo, mergeDeclaredIndexes, mongoAllowsIndexType, + normalizeIndexTypes, persistDeclaredSchemaIndexes, removeDeclaredIndexes, removeIndexFromSchemaFields, - resolveIndexName, toMutableIndexes, validateIndexFields, } from '../utils/indexes.js'; @@ -651,7 +650,7 @@ export class MongooseAdapter extends DatabaseAdapter { const collection = this.mongoose.model(schemaName).collection; for (const index of prepared) { const spec: Record = {}; - const types = Array.isArray(index.types) ? index.types : undefined; + const types = normalizeIndexTypes(index.types, index.fields.length); for (let i = 0; i < index.fields.length; i++) { spec[index.fields[i]] = types ? mapCompatibleToMongo(types[i]) @@ -717,15 +716,13 @@ export class MongooseAdapter extends DatabaseAdapter { throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); const collection = this.mongoose.model(schemaName).collection; const result = await collection.indexes(); - const declared = this.models[schemaName].originalSchema.modelOptions.indexes ?? []; - const declaredByName = new Map( - declared.map((index: ModelOptionsIndexes) => [resolveIndexName(index), index]), + const declaredByName = declaredIndexMap( + this.models[schemaName].originalSchema.modelOptions.indexes, ); return result.map(index => { const options: Record = {}; for (const [key, value] of Object.entries(index)) { - if (key === 'key' || key === 'options') continue; - if (key === 'v') continue; + if (key === 'key' || key === 'options' || key === 'v') continue; options[key] = value; } const fields: string[] = []; @@ -734,18 +731,20 @@ export class MongooseAdapter extends DatabaseAdapter { fields.push(field); types.push(type as MongoIndexType); } - const name = (options.name as string | undefined) ?? index.name; + const name = typeof options.name === 'string' ? options.name : index.name; const declaredIndex = name ? declaredByName.get(name) : undefined; - return { + const live: ModelOptionsIndexes = { name, fields, - types: declaredIndex?.types ?? types, - options: { - ...declaredIndex?.options, - ...options, - name, - }, - } as ModelOptionsIndexes; + types, + options: { ...options, name }, + }; + if (!declaredIndex) return live; + return { + ...live, + types: declaredIndex.types ?? live.types, + options: { ...declaredIndex.options, ...live.options, name }, + }; }); } @@ -932,7 +931,7 @@ export class MongooseAdapter extends DatabaseAdapter { }); } if (types) { - const typeList = Array.isArray(types) ? types : index.fields.map(() => types); + const typeList = normalizeIndexTypes(types, index.fields.length) ?? []; if (Array.isArray(types) && typeList.length !== index.fields.length) { throw new GrpcError(status.INVALID_ARGUMENT, 'Invalid index types format'); } @@ -944,11 +943,7 @@ export class MongooseAdapter extends DatabaseAdapter { ); } } - index.types = typeList.map(type => - isCompatibleIndexType(type) || isMongoIndexType(type) - ? mapCompatibleToMongo(type) - : type, - ) as MongoIndexType[]; + index.types = typeList.map(mapCompatibleToMongo); } prepared.push(index); } diff --git a/modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts b/modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts deleted file mode 100644 index fe7cf5cd1..000000000 --- a/modules/database/src/adapters/sequelize-adapter/__tests__/indexes.adapter.test.ts +++ /dev/null @@ -1,128 +0,0 @@ -import { describe, expect, it, jest } from '@jest/globals'; -import { CompatibleIndexType, PostgresIndexType } from '@conduitplatform/grpc-sdk'; -import { SequelizeAdapter } from '../index.js'; - -class TestSequelizeAdapter extends SequelizeAdapter { - protected async hasLegacyCollections(): Promise { - return false; - } -} - -function makeAdapter(dialect: string = 'postgres') { - const addIndex = jest.fn().mockResolvedValue(undefined); - const removeIndex = jest.fn().mockResolvedValue(undefined); - const showIndex = jest.fn().mockResolvedValue([ - { - name: 'cnd_idx_email_asc', - unique: false, - fields: [{ attribute: 'email', order: 'ASC' }], - definition: 'CREATE INDEX cnd_idx_email_asc ON cnd_User USING btree (email)', - }, - ]); - const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); - const findByIdAndUpdate = jest.fn().mockResolvedValue({}); - const sync = jest.fn().mockResolvedValue(undefined); - const adapter = Object.create(TestSequelizeAdapter.prototype) as SequelizeAdapter; - adapter.sequelize = { - getDialect: () => dialect, - getQueryInterface: () => ({ addIndex, removeIndex, showIndex }), - } as any; - const originalSchema = { - name: 'User', - ownerModule: 'database', - collectionName: 'custom_users', - fields: { email: { type: 'String' } }, - compiledFields: { email: { type: 'String' } }, - modelOptions: { indexes: [] as unknown[] }, - }; - adapter.models = { - User: { originalSchema, sync }, - _DeclaredSchema: { findOne, findByIdAndUpdate }, - } as any; - return { - adapter, - addIndex, - removeIndex, - showIndex, - sync, - findByIdAndUpdate, - originalSchema, - }; -} - -describe('sequelize adapter indexes T24 T30–T33', () => { - it('T24 getDatabaseType still returns PostgreSQL, not postgres', () => { - const { adapter } = makeAdapter('postgres'); - expect(adapter.getDatabaseType()).toBe('PostgreSQL'); - }); - - it('T30 create/get/delete use originalSchema.collectionName, not a hardcoded cnd_ prefix', async () => { - const { adapter, addIndex, removeIndex, showIndex } = makeAdapter(); - await adapter.createIndexes( - 'User', - [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], - 'database', - { privileged: true }, - ); - expect(addIndex.mock.calls[0][0]).toBe('custom_users'); - expect(addIndex.mock.calls[0][0]).not.toBe('cnd_User'); - await adapter.getIndexes('User'); - expect(showIndex).toHaveBeenCalledWith('custom_users'); - await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); - expect(removeIndex).toHaveBeenCalledWith('custom_users', 'cnd_idx_email_asc'); - }); - - it('T31 create does not rebuild/sync the schema', async () => { - const { adapter, sync } = makeAdapter(); - await adapter.createIndexes( - 'User', - [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], - 'database', - ); - expect(sync).not.toHaveBeenCalled(); - }); - - it('T32 getIndexes reads the live engine and overlays declared Compatible types', async () => { - const { adapter, showIndex, originalSchema } = makeAdapter(); - originalSchema.modelOptions.indexes = [ - { - fields: ['email'], - name: 'cnd_idx_email_asc', - types: [CompatibleIndexType.Ascending], - }, - ]; - const result = await adapter.getIndexes('User'); - expect(showIndex).toHaveBeenCalledWith('custom_users'); - expect(result[0].types).toEqual([CompatibleIndexType.Ascending]); - expect(result[0].fields).toEqual(['email']); - }); - - it('T33 delete awaits removeIndex and persists', async () => { - const { adapter, removeIndex, findByIdAndUpdate } = makeAdapter(); - await adapter.deleteIndexes('User', ['cnd_idx_email_asc']); - expect(removeIndex).toHaveBeenCalledTimes(1); - expect(findByIdAndUpdate).toHaveBeenCalled(); - }); - - it('mysql HASH is allowed; sqlite HASH throws on Admin create', async () => { - const mysql = makeAdapter('mysql'); - await expect( - mysql.adapter.createIndexes( - 'User', - [{ fields: ['email'], types: PostgresIndexType.HASH }], - 'database', - { privileged: true }, - ), - ).resolves.toBe('Indexes created!'); - - const sqlite = makeAdapter('sqlite'); - await expect( - sqlite.adapter.createIndexes( - 'User', - [{ fields: ['email'], types: PostgresIndexType.HASH }], - 'database', - { privileged: true }, - ), - ).rejects.toMatchObject({ message: expect.stringMatching(/sqlite/i) }); - }); -}); diff --git a/modules/database/src/adapters/sequelize-adapter/index.ts b/modules/database/src/adapters/sequelize-adapter/index.ts index 43f485d4b..4e3c1128f 100644 --- a/modules/database/src/adapters/sequelize-adapter/index.ts +++ b/modules/database/src/adapters/sequelize-adapter/index.ts @@ -6,7 +6,6 @@ import { GrpcError, Indexable, ModelOptionsIndexes, - PostgresIndexOptions, PostgresIndexType, RawSQLQuery, UntypedArray, @@ -31,14 +30,16 @@ import { pgSchemaConverter } from './postgres-adapter/PgSchemaConverter.js'; import { isEqual, isNil } from 'lodash-es'; import { assertUniqueIndexPrivilege, + declaredIndexMap, ensureIndexName, inferSqlIndexType, isIndexAlreadyExistsError, + isPostgresIndexType, mergeDeclaredIndexes, + normalizeIndexTypes, persistDeclaredSchemaIndexes, removeDeclaredIndexes, removeIndexFromSchemaFields, - resolveIndexName, sqlDialectAllowsIndexType, sqlIndexFields, toMutableIndexes, @@ -414,9 +415,8 @@ export abstract class SequelizeAdapter extends DatabaseAdapter const queryInterface = this.sequelize.getQueryInterface(); const result = (await queryInterface.showIndex(collectionName)) as UntypedArray; const dialect = this.sequelize.getDialect(); - const declared = this.models[schemaName].originalSchema.modelOptions.indexes ?? []; - const declaredByName = new Map( - declared.map((index: ModelOptionsIndexes) => [resolveIndexName(index), index]), + const declaredByName = declaredIndexMap( + this.models[schemaName].originalSchema.modelOptions.indexes, ); return result.map(row => { const fields = (row.fields ?? []).map((field: unknown) => @@ -424,29 +424,18 @@ export abstract class SequelizeAdapter extends DatabaseAdapter ); const name = row.name as string; const declaredIndex = declaredByName.get(name); - const options: Record = { + const live: ModelOptionsIndexes = { name, - unique: !!row.unique, - ...(declaredIndex?.options ?? {}), + fields, + types: inferSqlIndexType(row, dialect), + options: { name, unique: !!row.unique }, }; - for (const [key, value] of Object.entries(row)) { - if ( - key === 'options' || - key === 'types' || - key === 'fields' || - key === 'definition' || - key === 'indkey' - ) { - continue; - } - if (options[key] === undefined) options[key] = value; - } + if (!declaredIndex) return live; return { - name, - fields, - types: declaredIndex?.types ?? inferSqlIndexType(row, dialect), - options, - } as ModelOptionsIndexes; + ...live, + types: declaredIndex.types ?? live.types, + options: { ...declaredIndex.options, ...live.options, name }, + }; }); } @@ -531,7 +520,7 @@ export abstract class SequelizeAdapter extends DatabaseAdapter const index = ensureIndexName(raw); validateIndexFields(schema, index); if (index.types) { - const types = Array.isArray(index.types) ? index.types : [index.types]; + const types = normalizeIndexTypes(index.types, index.fields.length) ?? []; if (types.some(type => !sqlDialectAllowsIndexType(dialect, type))) { throw new GrpcError( status.INVALID_ARGUMENT, @@ -539,21 +528,14 @@ export abstract class SequelizeAdapter extends DatabaseAdapter ); } const first = types[0]; - if ( - typeof first === 'string' && - Object.values(PostgresIndexType).includes(first as PostgresIndexType) && - types.length === 1 - ) { - index.options = { - ...(index.options ?? {}), - using: first as PostgresIndexType, - } as PostgresIndexOptions; - } else { - index.options = { - ...(index.options ?? {}), - using: PostgresIndexType.BTREE, - } as PostgresIndexOptions; - } + const using = + types.length === 1 && isPostgresIndexType(first) + ? first + : PostgresIndexType.BTREE; + index.options = { + ...(index.options ?? {}), + using, + }; } if (index.options) { if (!checkIfPostgresOptions(index.options)) { diff --git a/modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts b/modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts deleted file mode 100644 index 95e061b0e..000000000 --- a/modules/database/src/adapters/utils/__tests__/sql-index-converters.test.ts +++ /dev/null @@ -1,125 +0,0 @@ -import { beforeEach, describe, expect, it, jest } from '@jest/globals'; -import { - CompatibleIndexType, - ConduitGrpcSdk, - ConduitSchema, - MongoIndexType, - PostgresIndexType, - TYPE, -} from '@conduitplatform/grpc-sdk'; -import { - convertModelOptionsIndexes, - convertSchemaFieldIndexes, -} from '../database-transform-utils.js'; -import { sqlSchemaConverter } from '../../sequelize-adapter/sql-adapter/SqlSchemaConverter.js'; -import { pgSchemaConverter } from '../../sequelize-adapter/postgres-adapter/PgSchemaConverter.js'; - -function schemaWithIndexes( - indexes: ConduitSchema['modelOptions']['indexes'], - fields: ConduitSchema['fields'] = { email: { type: TYPE.String } }, -) { - return new ConduitSchema('User', fields as any, { indexes }); -} - -describe('SQL dialect-aware converters T17–T24', () => { - beforeEach(() => { - jest.spyOn(ConduitGrpcSdk.Logger, 'warn').mockImplementation(() => {}); - }); - - it('T18 postgres maps Compatible to BTREE + ASC/DESC', () => { - const copy = convertModelOptionsIndexes( - schemaWithIndexes([ - { - fields: ['email'], - types: [CompatibleIndexType.Descending], - }, - ]), - 'postgres', - ); - const index = copy.modelOptions.indexes![0] as any; - expect(index.using).toBe(PostgresIndexType.BTREE); - expect(index.fields[0]).toEqual({ name: 'email', order: 'DESC' }); - }); - - it('T19 mysql maps Compatible to BTREE + ASC', () => { - const copy = convertModelOptionsIndexes( - schemaWithIndexes([{ fields: ['email'], types: [CompatibleIndexType.Ascending] }]), - 'mysql', - ); - const index = copy.modelOptions.indexes![0] as any; - expect(index.using).toBe(PostgresIndexType.BTREE); - expect(index.fields[0]).toEqual({ name: 'email', order: 'ASC' }); - }); - - it('T20 sqlite maps Compatible to BTREE + ASC', () => { - const copy = convertModelOptionsIndexes( - schemaWithIndexes([{ fields: ['email'], types: CompatibleIndexType.Ascending }]), - 'sqlite', - ); - const index = copy.modelOptions.indexes![0] as any; - expect(index.using).toBe(PostgresIndexType.BTREE); - }); - - it('T21 recover: Mongo-only leftovers on SQL are warned and skipped', () => { - const warn = ConduitGrpcSdk.Logger.warn as jest.Mock; - const copy = convertModelOptionsIndexes( - schemaWithIndexes([ - { fields: ['loc'], types: [MongoIndexType.GeoSpatial2dSphere] }, - { fields: ['email'], types: [CompatibleIndexType.Ascending] }, - ]), - 'postgres', - ); - expect(copy.modelOptions.indexes).toHaveLength(1); - expect(warn).toHaveBeenCalled(); - }); - - it('T22 recover: postgres-only types on mysql/mariadb/sqlite are skipped', () => { - for (const dialect of ['mysql', 'mariadb', 'sqlite'] as const) { - const copy = convertModelOptionsIndexes( - schemaWithIndexes([{ fields: ['email'], types: PostgresIndexType.GIST }]), - dialect, - ); - expect(copy.modelOptions.indexes).toHaveLength(0); - } - }); - - it('converts field-level Compatible indexes into sequelize model indexes', () => { - const copy = convertSchemaFieldIndexes( - new ConduitSchema( - 'Perm', - { - resource: { - type: TYPE.String, - index: { type: CompatibleIndexType.Ascending }, - }, - } as any, - {}, - ), - 'mysql', - ); - expect(copy.modelOptions.indexes).toHaveLength(1); - expect((copy.fields.resource as any).index).toBeUndefined(); - }); - - it('sqlSchemaConverter is dialect-aware for mysql vs sqlite', () => { - const schema = new ConduitSchema('User', { email: { type: TYPE.String } } as any, { - indexes: [ - { fields: ['email'], types: [CompatibleIndexType.Descending] }, - { fields: ['email'], types: PostgresIndexType.HASH }, - ], - }); - const [mysql] = sqlSchemaConverter(schema, 'mysql'); - const [sqlite] = sqlSchemaConverter(schema, 'sqlite'); - expect(mysql.modelOptions.indexes).toHaveLength(2); - expect(sqlite.modelOptions.indexes).toHaveLength(1); - }); - - it('pgSchemaConverter keeps postgres-only types', () => { - const schema = new ConduitSchema('User', { email: { type: TYPE.String } } as any, { - indexes: [{ fields: ['email'], types: PostgresIndexType.GIN }], - }); - const [pg] = pgSchemaConverter(schema); - expect(pg.modelOptions.indexes).toHaveLength(1); - expect((pg.modelOptions.indexes![0] as any).using).toBe(PostgresIndexType.GIN); - }); -}); diff --git a/modules/database/src/adapters/utils/database-transform-utils.ts b/modules/database/src/adapters/utils/database-transform-utils.ts index 47ec7b093..b23c6d7b6 100644 --- a/modules/database/src/adapters/utils/database-transform-utils.ts +++ b/modules/database/src/adapters/utils/database-transform-utils.ts @@ -1,12 +1,10 @@ import { isBoolean, isNumber, isString } from 'lodash-es'; import { - CompatibleIndexType, ConduitGrpcSdk, ConduitModelField, ConduitSchema, Indexable, ModelOptionsIndexes, - PostgresIndexOptions, PostgresIndexType, } from '@conduitplatform/grpc-sdk'; import { checkIfPostgresOptions } from '../sequelize-adapter/utils/index.js'; @@ -17,8 +15,19 @@ import { mapCompatibleToSqlOrder, normalizeIndexTypes, sqlDialectAllowsIndexType, + type SqlIndexField, } from './indexes.js'; +type SqlEngineIndex = Omit & { + fields: SqlIndexField[]; + using?: PostgresIndexType; +}; + +function setSqlEngineIndexes(copy: ConduitSchema, indexes: SqlEngineIndex[]) { + // Sequelize reads these through define(..., schema.modelOptions). + copy.modelOptions.indexes = indexes as ModelOptionsIndexes[]; +} + export function checkDefaultValue(type: string, value: string) { switch (type) { case 'String': @@ -38,110 +47,91 @@ export function checkDefaultValue(type: string, value: string) { } } -function flattenSqlIndexOptions(index: ModelOptionsIndexes, dialect: string): boolean { - if (!index.options) return true; - if (!checkIfPostgresOptions(index.options)) { - ConduitGrpcSdk.Logger.warn( - `Invalid index options for ${dialect} found in '${copyName(index)}', ignoring index`, - ); - return false; - } - for (const [option, value] of Object.entries(index.options)) { - index[option as keyof PostgresIndexOptions] = value; - } - delete index.options; - return true; +function skipIndex(schemaName: string, dialect: string, reason: string) { + ConduitGrpcSdk.Logger.warn( + `Invalid index ${reason} for ${dialect} found in '${schemaName}', ignoring index`, + ); } -function copyName(index: ModelOptionsIndexes): string { - return index.name ?? index.fields?.join(',') ?? 'unnamed'; -} - -function applySqlIndexTypes( - index: ModelOptionsIndexes, +function toSqlEngineIndex( + raw: ModelOptionsIndexes, dialect: string, schemaName: string, -): boolean { - if (!index.types) { - index.using = PostgresIndexType.BTREE; - return true; +): SqlEngineIndex | null { + const index = ensureIndexName({ ...raw, fields: [...raw.fields] }); + if (index.options && !checkIfPostgresOptions(index.options)) { + skipIndex(schemaName, dialect, 'options'); + return null; } - const types = normalizeIndexTypes(index.types, index.fields.length) ?? []; - if (types.some(type => !sqlDialectAllowsIndexType(dialect, type))) { - ConduitGrpcSdk.Logger.warn( - `Invalid index type for ${dialect} found in '${schemaName}', ignoring index`, - ); - return false; - } - if (types.some(isPortableDirection)) { - index.fields = index.fields.map((field, i) => ({ - name: field, - order: mapCompatibleToSqlOrder(types[i]), - })) as unknown as string[]; - index.using = PostgresIndexType.BTREE; - } else if (types.length === 1 && isPostgresIndexType(types[0])) { - index.using = types[0]; - } else { - ConduitGrpcSdk.Logger.warn( - `Invalid index type for ${dialect} found in '${schemaName}', ignoring index`, - ); - return false; + + let fields: SqlIndexField[] = [...index.fields]; + let using = PostgresIndexType.BTREE; + if (index.types) { + const types = normalizeIndexTypes(index.types, index.fields.length) ?? []; + if (types.some(type => !sqlDialectAllowsIndexType(dialect, type))) { + skipIndex(schemaName, dialect, 'type'); + return null; + } + if (types.some(isPortableDirection)) { + fields = index.fields.map((field, i) => ({ + name: field, + order: mapCompatibleToSqlOrder(types[i]), + })); + } else if (types.length === 1 && isPostgresIndexType(types[0])) { + using = types[0]; + } else { + skipIndex(schemaName, dialect, 'type'); + return null; + } } - delete index.types; - return true; + + return { + ...index.options, + name: index.name, + fields, + using, + unique: index.options?.unique, + }; } export function convertModelOptionsIndexes(copy: ConduitSchema, dialect = 'postgres') { - const converted: ModelOptionsIndexes[] = []; + const converted: SqlEngineIndex[] = []; for (const raw of copy.modelOptions.indexes ?? []) { - const index = ensureIndexName({ ...raw, fields: [...raw.fields] }); - if (!applySqlIndexTypes(index, dialect, copy.name)) continue; - if (!flattenSqlIndexOptions(index, dialect)) continue; - if (!index.using) index.using = PostgresIndexType.BTREE; - converted.push(index); + const index = toSqlEngineIndex(raw, dialect, copy.name); + if (index) converted.push(index); } - copy.modelOptions.indexes = converted; + setSqlEngineIndexes(copy, converted); return copy; } export function convertSchemaFieldIndexes(copy: ConduitSchema, dialect = 'postgres') { - const indexes: ModelOptionsIndexes[] = []; + const indexes: SqlEngineIndex[] = []; for (const [fieldName, fieldValue] of Object.entries(copy.fields)) { - const index = (fieldValue as ConduitModelField).index; + const field = fieldValue as ConduitModelField; + const index = field.index; if (!index) continue; - const newIndex = ensureIndexName({ - fields: [fieldName], - types: index.type - ? isPortableDirection(index.type) - ? [index.type as CompatibleIndexType] - : (index.type as PostgresIndexType) - : undefined, - options: index.options, - name: (index as { name?: string }).name, - }); if (index.type && !sqlDialectAllowsIndexType(dialect, index.type)) { - ConduitGrpcSdk.Logger.warn( - `Invalid index type for ${dialect} found in '${copy.name}', ignoring index`, - ); - delete (copy.fields[fieldName] as ConduitModelField).index; - continue; - } - if (!applySqlIndexTypes(newIndex, dialect, copy.name)) { - delete (copy.fields[fieldName] as ConduitModelField).index; - continue; - } - if (!flattenSqlIndexOptions(newIndex, dialect)) { - delete (copy.fields[fieldName] as ConduitModelField).index; + skipIndex(copy.name, dialect, 'type'); + delete field.index; continue; } - indexes.push(newIndex); - delete (copy.fields[fieldName] as ConduitModelField).index; - } - if (copy.modelOptions.indexes) { - copy.modelOptions.indexes = [...copy.modelOptions.indexes, ...indexes]; - } else { - copy.modelOptions.indexes = indexes; + const converted = toSqlEngineIndex( + { + fields: [fieldName], + types: index.type === undefined ? undefined : [index.type], + options: index.options, + name: index.name, + }, + dialect, + copy.name, + ); + delete field.index; + if (converted) indexes.push(converted); } + setSqlEngineIndexes(copy, [ + ...((copy.modelOptions.indexes ?? []) as SqlEngineIndex[]), + ...indexes, + ]); return copy; } diff --git a/modules/database/src/adapters/utils/indexes.ts b/modules/database/src/adapters/utils/indexes.ts index 2c1a2776a..9338b6ecb 100644 --- a/modules/database/src/adapters/utils/indexes.ts +++ b/modules/database/src/adapters/utils/indexes.ts @@ -12,7 +12,7 @@ import { ConduitDatabaseSchema } from '../../interfaces/index.js'; export const ADMIN_INDEX_CALLER = 'database'; -export const MONGO_INDEX_TYPE_VALUES: ReadonlyArray = [ +const MONGO_INDEX_TYPE_VALUES: ReadonlySet = new Set([ MongoIndexType.Ascending, MongoIndexType.Descending, MongoIndexType.GeoSpatial2d, @@ -20,10 +20,12 @@ export const MONGO_INDEX_TYPE_VALUES: ReadonlyArray = [ MongoIndexType.GeoHaystack, MongoIndexType.Hashed, MongoIndexType.Text, -]; +]); const SQL_IDENTIFIER_MAX_LEN = 63; +export type SqlIndexField = string | { name: string; order: 'ASC' | 'DESC' }; + export function isCompatibleIndexType(value: unknown): value is CompatibleIndexType { return ( value === CompatibleIndexType.Ascending || value === CompatibleIndexType.Descending @@ -31,7 +33,7 @@ export function isCompatibleIndexType(value: unknown): value is CompatibleIndexT } export function isMongoIndexType(value: unknown): value is MongoIndexType { - return (MONGO_INDEX_TYPE_VALUES as readonly unknown[]).includes(value); + return MONGO_INDEX_TYPE_VALUES.has(value); } export function isPostgresIndexType(value: unknown): value is PostgresIndexType { @@ -54,17 +56,14 @@ export function normalizeIndexTypes( fieldCount: number, ): unknown[] | undefined { if (types === undefined) return undefined; - if (Array.isArray(types)) { - return [...types]; - } + if (Array.isArray(types)) return [...types]; return Array.from({ length: fieldCount }, () => types); } function typeToken(type: unknown): string { - if (type === undefined || type === CompatibleIndexType.Ascending) return 'asc'; - if (type === CompatibleIndexType.Descending) return 'desc'; - if (type === MongoIndexType.Ascending || type === 1) return 'asc'; - if (type === MongoIndexType.Descending || type === -1) return 'desc'; + if (isPortableDirection(type) || type === undefined) { + return mapCompatibleToSqlOrder(type).toLowerCase(); + } if (typeof type === 'string') return type.toLowerCase().replace(/[^a-z0-9]+/g, ''); return String(type); } @@ -89,14 +88,8 @@ export function generateIndexName( export function ensureIndexName(index: ModelOptionsIndexes): ModelOptionsIndexes { const existing = resolveIndexName(index); - if (existing) { - return { - ...index, - name: existing, - options: { ...index.options, name: existing }, - }; - } - const name = generateIndexName(index.fields, index.types, isUniqueIndex(index)); + const name = + existing ?? generateIndexName(index.fields, index.types, isUniqueIndex(index)); return { ...index, name, @@ -105,8 +98,14 @@ export function ensureIndexName(index: ModelOptionsIndexes): ModelOptionsIndexes } export function mapCompatibleToMongo(type: unknown): MongoIndexType { - if (type === CompatibleIndexType.Descending) return MongoIndexType.Descending; - if (type === CompatibleIndexType.Ascending || type === undefined) { + if (type === CompatibleIndexType.Descending || type === MongoIndexType.Descending) { + return MongoIndexType.Descending; + } + if ( + type === undefined || + type === CompatibleIndexType.Ascending || + type === MongoIndexType.Ascending + ) { return MongoIndexType.Ascending; } if (isMongoIndexType(type)) return type; @@ -134,8 +133,7 @@ export function sqlDialectAllowsIndexType(dialect: string, type: unknown): boole if (type === PostgresIndexType.HASH) { return dialect === 'postgres' || dialect === 'mysql' || dialect === 'mariadb'; } - if (isPostgresIndexType(type)) return dialect === 'postgres'; - return false; + return isPostgresIndexType(type) && dialect === 'postgres'; } export function mongoAllowsIndexType(type: unknown): boolean { @@ -146,17 +144,11 @@ export function mergeDeclaredIndexes( existing: readonly ModelOptionsIndexes[] | undefined, incoming: readonly ModelOptionsIndexes[], ): ModelOptionsIndexes[] { - const merged = new Map(); - for (const index of existing ?? []) { - const named = ensureIndexName(index); - merged.set(resolveIndexName(named)!, named); - } + const merged = declaredIndexMap(existing); for (const index of incoming) { const named = ensureIndexName(index); - const name = resolveIndexName(named)!; - if (!merged.has(name)) { - merged.set(name, named); - } + const name = resolveIndexName(named); + if (name && !merged.has(name)) merged.set(name, named); } return [...merged.values()]; } @@ -222,23 +214,18 @@ export function assertUniqueIndexPrivilege(args: { privileged?: boolean; }) { if (!args.unique) return; - if (args.privileged) return; - if (args.schemaOwner === args.callerModule) return; + if (args.privileged || args.schemaOwner === args.callerModule) return; throw new GrpcError(status.PERMISSION_DENIED, 'Not authorized to create unique index'); } export function isIndexAlreadyExistsError(error: unknown): boolean { const err = error as { message?: string; code?: number | string; name?: string }; const message = (err.message ?? '').toLowerCase(); - if ( + return ( message.includes('already exists') || message.includes('already exist') || message.includes('duplicate key name') || - message.includes('index already exists') - ) { - return true; - } - return ( + message.includes('index already exists') || err.code === 85 || err.code === '42P07' || err.name === 'SequelizeUniqueConstraintError' @@ -271,12 +258,21 @@ export async function persistDeclaredSchemaIndexes(args: { export function collectExistingIndexNames( indexes: readonly ModelOptionsIndexes[], ): Set { - const names = new Set(); - for (const index of indexes) { - const name = resolveIndexName(index); - if (name) names.add(name); + return new Set( + indexes.map(resolveIndexName).filter((name): name is string => Boolean(name)), + ); +} + +export function declaredIndexMap( + indexes: readonly ModelOptionsIndexes[] | undefined, +): Map { + const map = new Map(); + for (const index of indexes ?? []) { + const named = ensureIndexName(index); + const name = resolveIndexName(named); + if (name) map.set(name, named); } - return names; + return map; } export function toMutableIndexes( @@ -285,16 +281,13 @@ export function toMutableIndexes( return indexes.map(index => ({ ...index, fields: [...index.fields], - types: Array.isArray(index.types) ? [...index.types] : index.types, options: index.options ? { ...index.options } : index.options, })); } -export function sqlIndexFields( - index: ModelOptionsIndexes, -): Array { +export function sqlIndexFields(index: ModelOptionsIndexes): SqlIndexField[] { const types = normalizeIndexTypes(index.types, index.fields.length); - if (!types || !types.some(isCompatibleIndexType)) { + if (!types || !types.some(isPortableDirection)) { return [...index.fields]; } return index.fields.map((field, i) => ({ @@ -307,22 +300,15 @@ export function inferSqlIndexType( row: { type?: string; definition?: string }, dialect: string, ): PostgresIndexType | undefined { - if (typeof row.type === 'string' && row.type.length > 0) { - const upper = row.type.toUpperCase(); - if (isPostgresIndexType(upper)) return upper; + if (typeof row.type === 'string' && isPostgresIndexType(row.type.toUpperCase())) { + return row.type.toUpperCase() as PostgresIndexType; } if (typeof row.definition === 'string') { const match = /USING\s+(\w+)/i.exec(row.definition); - if (match && isPostgresIndexType(match[1].toUpperCase())) { - return match[1].toUpperCase() as PostgresIndexType; - } + const using = match?.[1]?.toUpperCase(); + if (using && isPostgresIndexType(using)) return using; } - if ( - dialect === 'postgres' || - dialect === 'mysql' || - dialect === 'mariadb' || - dialect === 'sqlite' - ) { + if (['postgres', 'mysql', 'mariadb', 'sqlite'].includes(dialect)) { return PostgresIndexType.BTREE; } return undefined; diff --git a/modules/database/src/admin/schema.admin.ts b/modules/database/src/admin/schema.admin.ts index b676f11a5..dce1cd865 100644 --- a/modules/database/src/admin/schema.admin.ts +++ b/modules/database/src/admin/schema.admin.ts @@ -693,12 +693,7 @@ export class SchemaAdmin { async createIndexes(call: ParsedRouterRequest): Promise { const { id, indexes } = call.request.params; - const requestedSchema = await this.database - .getSchemaModel('_DeclaredSchema') - .model.findOne({ _id: id }); - if (isNil(requestedSchema)) { - throw new GrpcError(status.NOT_FOUND, 'Schema does not exist'); - } + const requestedSchema = await this.findDeclaredSchemaById(id); return await this.database.createIndexes( requestedSchema.name, indexes, @@ -708,25 +703,14 @@ export class SchemaAdmin { } async getIndexes(call: ParsedRouterRequest): Promise { - const id = call.request.params.id; - const requestedSchema = await this.database - .getSchemaModel('_DeclaredSchema') - .model.findOne({ _id: id }); - if (isNil(requestedSchema)) { - throw new GrpcError(status.NOT_FOUND, 'Schema does not exist'); - } + const requestedSchema = await this.findDeclaredSchemaById(call.request.params.id); const indexes = await this.database.getIndexes(requestedSchema.name); return { indexes }; } async deleteIndexes(call: ParsedRouterRequest): Promise { const { id, indexNames } = call.request.params; - const requestedSchema = await this.database - .getSchemaModel('_DeclaredSchema') - .model.findOne({ _id: id }); - if (isNil(requestedSchema)) { - throw new GrpcError(status.NOT_FOUND, 'Schema does not exist'); - } + const requestedSchema = await this.findDeclaredSchemaById(id); if (isNil(indexNames) || indexNames.length === 0) { throw new GrpcError( status.INVALID_ARGUMENT, @@ -812,6 +796,16 @@ export class SchemaAdmin { return 'Indexes imported successfully'; } + private async findDeclaredSchemaById(id: string) { + const requestedSchema = await this.database + .getSchemaModel('_DeclaredSchema') + .model.findOne({ _id: id }); + if (isNil(requestedSchema)) { + throw new GrpcError(status.NOT_FOUND, 'Schema does not exist'); + } + return requestedSchema; + } + async checkRequestedSchema(id: string) { const requestedSchema = await this.database .getSchemaModel('_DeclaredSchema') diff --git a/modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts b/modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts deleted file mode 100644 index a2d6e52fa..000000000 --- a/modules/database/src/utils/__tests__/validateModelOptions.indexes.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { describe, expect, it } from '@jest/globals'; -import { CompatibleIndexType } from '@conduitplatform/grpc-sdk'; -import { validateSchemaInput } from '../utilities.js'; - -describe('validateModelOptions T35 T36', () => { - it('T35 accepts modelOptions.indexes', () => { - expect(() => - validateSchemaInput( - 'User', - { email: 'String' }, - { - timestamps: true, - indexes: [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], - }, - ), - ).not.toThrow(); - }); - - it('T36 keeps conduit.readPreference while accepting indexes', () => { - expect(() => - validateSchemaInput( - 'User', - { email: 'String' }, - { - timestamps: true, - indexes: [{ fields: ['email'] }], - conduit: { readPreference: 'secondaryPreferred' }, - }, - ), - ).not.toThrow(); - }); - - it('still rejects unknown conduit keys and unknown model option keys', () => { - expect(() => - validateSchemaInput('User', { email: 'String' }, { - unknown: true, - } as any), - ).toThrow(/indexes/); - expect(() => - validateSchemaInput('User', { email: 'String' }, { - conduit: { notARealKey: true }, - } as any), - ).toThrow(/readPreference/); - }); -}); From e897821e56b549ae5a9b4b93d896fea5dfa8c2dd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 09:57:49 +0000 Subject: [PATCH 3/4] fix(database): bind live index names and persist partial index changes Adopt engine names by fields+unique so upgrades and custom modules do not create duplicate indexes, keep Admin extras across re-register, persist only applied create/delete subsets, and publish bound names to replicas. --- .../src/__tests__/indexes/adapters.test.ts | 309 ++++++++++++++++-- .../src/__tests__/indexes/helpers.test.ts | 156 ++++++++- .../database/src/adapters/DatabaseAdapter.ts | 26 ++ .../src/adapters/mongoose-adapter/index.ts | 149 ++++++--- .../src/adapters/sequelize-adapter/index.ts | 147 ++++++--- .../database/src/adapters/utils/indexes.ts | 242 +++++++++++++- 6 files changed, 897 insertions(+), 132 deletions(-) diff --git a/modules/database/src/__tests__/indexes/adapters.test.ts b/modules/database/src/__tests__/indexes/adapters.test.ts index cb0810497..565624e53 100644 --- a/modules/database/src/__tests__/indexes/adapters.test.ts +++ b/modules/database/src/__tests__/indexes/adapters.test.ts @@ -11,22 +11,33 @@ import { SequelizeAdapter } from '../../adapters/sequelize-adapter/index.js'; function makeMongooseAdapter(overrides: Record = {}) { const createIndex = jest.fn().mockResolvedValue('email_1'); const dropIndex = jest.fn().mockResolvedValue(undefined); - const indexes = jest.fn().mockResolvedValue([ - { v: 2, key: { _id: 1 }, name: '_id_' }, - { v: 2, key: { email: 1 }, name: 'cnd_idx_email_asc', unique: false }, - ]); - const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); + const indexes = jest.fn().mockResolvedValue([{ v: 2, key: { _id: 1 }, name: '_id_' }]); + const findOne = jest + .fn() + .mockResolvedValue({ _id: 'declared-1', modelOptions: { indexes: [] } }); const findByIdAndUpdate = jest.fn().mockResolvedValue({}); + const publish = jest.fn(); const adapter = Object.create(MongooseAdapter.prototype) as MongooseAdapter; adapter.mongoose = { model: () => ({ collection: { createIndex, dropIndex, indexes } }), } as MongooseAdapter['mongoose']; + (adapter as unknown as { grpcSdk: { bus: { publish: typeof publish } } }).grpcSdk = { + bus: { publish }, + }; const originalSchema = { name: 'User', ownerModule: 'chat', collectionName: 'cnd_User', - fields: { email: { type: 'String' } }, - compiledFields: { email: { type: 'String' } }, + fields: { + email: { type: 'String' }, + room: { type: 'String' }, + createdAt: { type: 'Date' }, + }, + compiledFields: { + email: { type: 'String' }, + room: { type: 'String' }, + createdAt: { type: 'Date' }, + }, modelOptions: { indexes: [] as unknown[] }, ...((overrides.originalSchema as object) ?? {}), }; @@ -34,7 +45,20 @@ function makeMongooseAdapter(overrides: Record = {}) { User: { originalSchema }, _DeclaredSchema: { findOne, findByIdAndUpdate }, } as MongooseAdapter['models']; - return { adapter, createIndex, dropIndex, indexes, findByIdAndUpdate, originalSchema }; + findOne.mockImplementation(async () => ({ + _id: 'declared-1', + modelOptions: originalSchema.modelOptions, + })); + return { + adapter, + createIndex, + dropIndex, + indexes, + findOne, + findByIdAndUpdate, + publish, + originalSchema, + }; } class TestSequelizeAdapter extends SequelizeAdapter { @@ -46,41 +70,54 @@ class TestSequelizeAdapter extends SequelizeAdapter { function makeSequelizeAdapter(dialect = 'postgres') { const addIndex = jest.fn().mockResolvedValue(undefined); const removeIndex = jest.fn().mockResolvedValue(undefined); - const showIndex = jest.fn().mockResolvedValue([ - { - name: 'cnd_idx_email_asc', - unique: false, - fields: [{ attribute: 'email', order: 'ASC' }], - definition: 'CREATE INDEX cnd_idx_email_asc ON cnd_User USING btree (email)', - }, - ]); - const findOne = jest.fn().mockResolvedValue({ _id: 'declared-1' }); + const showIndex = jest.fn().mockResolvedValue([]); + const findOne = jest + .fn() + .mockResolvedValue({ _id: 'declared-1', modelOptions: { indexes: [] } }); const findByIdAndUpdate = jest.fn().mockResolvedValue({}); + const publish = jest.fn(); const sync = jest.fn().mockResolvedValue(undefined); const adapter = Object.create(TestSequelizeAdapter.prototype) as SequelizeAdapter; adapter.sequelize = { getDialect: () => dialect, getQueryInterface: () => ({ addIndex, removeIndex, showIndex }), } as SequelizeAdapter['sequelize']; + (adapter as unknown as { grpcSdk: { bus: { publish: typeof publish } } }).grpcSdk = { + bus: { publish }, + }; const originalSchema = { name: 'User', ownerModule: 'database', collectionName: 'custom_users', - fields: { email: { type: 'String' } }, - compiledFields: { email: { type: 'String' } }, + fields: { + email: { type: 'String' }, + room: { type: 'String' }, + createdAt: { type: 'Date' }, + }, + compiledFields: { + email: { type: 'String' }, + room: { type: 'String' }, + createdAt: { type: 'Date' }, + }, modelOptions: { indexes: [] as unknown[] }, }; adapter.models = { User: { originalSchema, sync }, _DeclaredSchema: { findOne, findByIdAndUpdate }, } as SequelizeAdapter['models']; + findOne.mockImplementation(async () => ({ + _id: 'declared-1', + modelOptions: originalSchema.modelOptions, + })); return { adapter, addIndex, removeIndex, showIndex, sync, + findOne, findByIdAndUpdate, + publish, originalSchema, }; } @@ -137,6 +174,10 @@ describe('mongoose adapter indexes', () => { it('reads live engine indexes', async () => { const { adapter, indexes } = makeMongooseAdapter(); + indexes.mockResolvedValue([ + { v: 2, key: { _id: 1 }, name: '_id_' }, + { v: 2, key: { email: 1 }, name: 'cnd_idx_email_asc', unique: false }, + ]); const result = await adapter.getIndexes('User'); expect(indexes).toHaveBeenCalled(); expect(result.map(index => index.name)).toEqual(['_id_', 'cnd_idx_email_asc']); @@ -162,6 +203,143 @@ describe('mongoose adapter indexes', () => { ), ).resolves.toBe('Indexes created!'); }); + + it('adopts a live compound name and skips createIndex', async () => { + const { adapter, createIndex, indexes, findByIdAndUpdate } = makeMongooseAdapter(); + indexes.mockResolvedValue([ + { v: 2, key: { _id: 1 }, name: '_id_' }, + { v: 2, key: { room: 1, createdAt: 1 }, name: 'room_1_createdAt_1', unique: false }, + ]); + await adapter.createIndexes( + 'User', + [ + { + fields: ['room', 'createdAt'], + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ], + 'chat', + ); + expect(createIndex).not.toHaveBeenCalled(); + const update = findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes[0].name).toBe('room_1_createdAt_1'); + }); + + it('throws on unique-data collisions and does not persist that index', async () => { + const { adapter, createIndex, findByIdAndUpdate } = makeMongooseAdapter(); + createIndex.mockRejectedValue({ code: 11000, message: 'E11000 duplicate key' }); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], options: { unique: true } }], + 'chat', + ), + ).rejects.toMatchObject({ code: status.INTERNAL }); + expect(findByIdAndUpdate).not.toHaveBeenCalled(); + }); + + it('persists the prefix and publishes after a later unique-data failure', async () => { + const { adapter, createIndex, findByIdAndUpdate, publish } = makeMongooseAdapter(); + createIndex + .mockResolvedValueOnce('cnd_idx_email_asc') + .mockRejectedValueOnce({ code: 11000, message: 'E11000 duplicate key' }); + await expect( + adapter.createIndexes( + 'User', + [ + { fields: ['email'], types: [CompatibleIndexType.Ascending] }, + { fields: ['room'], options: { unique: true } }, + ], + 'chat', + ), + ).rejects.toMatchObject({ code: status.INTERNAL }); + expect(findByIdAndUpdate).toHaveBeenCalledTimes(1); + const update = findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes.map(index => index.name)).toEqual([ + 'cnd_idx_email_asc', + ]); + expect(publish).toHaveBeenCalledWith('database:create:schema', expect.any(String)); + }); + + it('overlays declared Compatible types onto a live index by fields', async () => { + const { adapter, indexes, originalSchema } = makeMongooseAdapter(); + originalSchema.modelOptions.indexes = [ + { + fields: ['room', 'createdAt'], + name: 'cnd_idx_room_createdAt_asc_asc', + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ]; + indexes.mockResolvedValue([ + { v: 2, key: { _id: 1 }, name: '_id_' }, + { v: 2, key: { room: 1, createdAt: 1 }, name: 'room_1_createdAt_1', unique: false }, + ]); + const result = await adapter.getIndexes('User'); + expect(result[1].name).toBe('room_1_createdAt_1'); + expect(result[1].types).toEqual([ + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + ]); + }); + + it('publishes the schema after a successful persist', async () => { + const { adapter, publish } = makeMongooseAdapter(); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'chat', + ); + expect(publish).toHaveBeenCalledWith('database:create:schema', expect.any(String)); + }); + + it('rebinds to the live name on Mongo 86 instead of persisting a generated name', async () => { + const { adapter, createIndex, indexes, findByIdAndUpdate } = makeMongooseAdapter(); + createIndex.mockRejectedValue({ code: 86, message: 'IndexKeySpecsConflict' }); + indexes + .mockResolvedValueOnce([{ v: 2, key: { _id: 1 }, name: '_id_' }]) + .mockResolvedValueOnce([ + { v: 2, key: { _id: 1 }, name: '_id_' }, + { v: 2, key: { email: 1 }, name: 'email_1', unique: false }, + ]); + await adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'chat', + ); + const update = findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes[0].name).toBe('email_1'); + }); + + it('persists only names that were actually dropped', async () => { + const { adapter, dropIndex, findByIdAndUpdate } = makeMongooseAdapter({ + originalSchema: { + modelOptions: { + indexes: [ + { fields: ['email'], name: 'idx_a' }, + { fields: ['room'], name: 'idx_b' }, + ], + }, + }, + }); + dropIndex + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('missing')); + await expect(adapter.deleteIndexes('User', ['idx_a', 'idx_b'])).rejects.toMatchObject( + { + code: status.INTERNAL, + }, + ); + const update = findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes.map(index => index.name)).toEqual(['idx_b']); + }); }); describe('sequelize adapter indexes', () => { @@ -205,6 +383,14 @@ describe('sequelize adapter indexes', () => { types: [CompatibleIndexType.Ascending], }, ]; + showIndex.mockResolvedValue([ + { + name: 'cnd_idx_email_asc', + unique: false, + fields: [{ attribute: 'email', order: 'ASC' }], + definition: 'CREATE INDEX cnd_idx_email_asc ON custom_users USING btree (email)', + }, + ]); const result = await adapter.getIndexes('User'); expect(showIndex).toHaveBeenCalledWith('custom_users'); expect(result[0].types).toEqual([CompatibleIndexType.Ascending]); @@ -239,4 +425,89 @@ describe('sequelize adapter indexes', () => { ), ).rejects.toMatchObject({ message: expect.stringMatching(/sqlite/i) }); }); + + it('adopts a live compound name and skips addIndex', async () => { + const { adapter, addIndex, showIndex, findByIdAndUpdate } = makeSequelizeAdapter(); + showIndex.mockResolvedValue([ + { + name: 'room_createdAt', + unique: false, + fields: [{ attribute: 'room' }, { attribute: 'createdAt' }], + }, + ]); + await adapter.createIndexes( + 'User', + [ + { + fields: ['room', 'createdAt'], + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ], + 'database', + ); + expect(addIndex).not.toHaveBeenCalled(); + const update = findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes[0].name).toBe('room_createdAt'); + }); + + it('skips and persists on 42P07 name conflicts', async () => { + const { adapter, addIndex, findByIdAndUpdate, publish } = makeSequelizeAdapter(); + addIndex.mockRejectedValue({ + original: { code: '42P07', message: 'relation "cnd_idx_email_asc" already exists' }, + }); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], types: [CompatibleIndexType.Ascending] }], + 'database', + ), + ).resolves.toBe('Indexes created!'); + expect(findByIdAndUpdate).toHaveBeenCalled(); + expect(publish).toHaveBeenCalled(); + }); + + it('throws on 23505 unique collisions and does not persist that index', async () => { + const { adapter, addIndex, findByIdAndUpdate } = makeSequelizeAdapter(); + addIndex.mockRejectedValue({ + name: 'SequelizeUniqueConstraintError', + original: { code: '23505' }, + }); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], options: { unique: true } }], + 'database', + { privileged: true }, + ), + ).rejects.toMatchObject({ code: status.INTERNAL }); + expect(findByIdAndUpdate).not.toHaveBeenCalled(); + }); + + it('overlays declared Compatible types onto a live SQL index by fields', async () => { + const { adapter, showIndex, originalSchema } = makeSequelizeAdapter(); + originalSchema.modelOptions.indexes = [ + { + fields: ['room', 'createdAt'], + name: 'cnd_idx_room_createdAt_asc_asc', + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ]; + showIndex.mockResolvedValue([ + { + name: 'room_createdAt', + unique: false, + fields: [{ attribute: 'room' }, { attribute: 'createdAt' }], + definition: + 'CREATE INDEX room_createdAt ON custom_users USING btree (room, createdAt)', + }, + ]); + const result = await adapter.getIndexes('User'); + expect(result[0].name).toBe('room_createdAt'); + expect(result[0].types).toEqual([ + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + ]); + }); }); diff --git a/modules/database/src/__tests__/indexes/helpers.test.ts b/modules/database/src/__tests__/indexes/helpers.test.ts index 74be105b6..48ec49ba8 100644 --- a/modules/database/src/__tests__/indexes/helpers.test.ts +++ b/modules/database/src/__tests__/indexes/helpers.test.ts @@ -7,16 +7,21 @@ import { } from '@conduitplatform/grpc-sdk'; import { assertUniqueIndexPrivilege, + bindDeclaredIndexesToLive, collectExistingIndexNames, ensureIndexName, generateIndexName, + indexIdentity, isCompatibleIndexType, isIndexAlreadyExistsError, isMongoIndexType, + keepDeclaredIndexExtras, mapCompatibleToMongo, mapCompatibleToSqlOrder, mergeDeclaredIndexes, mongoAllowsIndexType, + overlayDeclaredOnLive, + persistDeclaredSchemaIndexes, removeDeclaredIndexes, removeIndexFromSchemaFields, resolveIndexName, @@ -170,13 +175,160 @@ describe('index helpers', () => { ).toThrow(/Not authorized to create unique index/); }); - it('collects existing names and detects already-exists errors', () => { + it('collects existing names and detects name/relation already-exists errors only', () => { expect( collectExistingIndexNames([{ fields: ['a'], options: { name: 'idx_a' } }]).has( 'idx_a', ), ).toBe(true); - expect(isIndexAlreadyExistsError(new Error('index already exists'))).toBe(true); + expect( + isIndexAlreadyExistsError({ + code: '42P07', + message: 'relation "x" already exists', + }), + ).toBe(true); + expect( + isIndexAlreadyExistsError({ code: '1061', message: "Duplicate key name 'x'" }), + ).toBe(true); + expect( + isIndexAlreadyExistsError({ + original: { code: '42P07', message: 'relation "idx" already exists' }, + }), + ).toBe(true); + expect(isIndexAlreadyExistsError(new Error('index foo already exists'))).toBe(true); + expect(isIndexAlreadyExistsError(new Error('already exists'))).toBe(false); + expect( + isIndexAlreadyExistsError({ code: 11000, message: 'E11000 duplicate key' }), + ).toBe(false); + expect( + isIndexAlreadyExistsError({ + name: 'SequelizeUniqueConstraintError', + original: { code: '23505' }, + }), + ).toBe(false); + expect( + isIndexAlreadyExistsError({ + code: 85, + message: 'Index with name: x already exists', + }), + ).toBe(false); + expect( + isIndexAlreadyExistsError({ + code: 86, + message: 'Index already exists with different options', + }), + ).toBe(false); + }); + + it('binds unnamed declared indexes to live names by fields+unique', () => { + const bound = bindDeclaredIndexesToLive( + [ + { + fields: ['room', 'createdAt'], + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ], + [ + { + name: '_id_', + fields: ['_id'], + options: { name: '_id_' }, + }, + { + name: 'room_1_createdAt_1', + fields: ['room', 'createdAt'], + options: { name: 'room_1_createdAt_1', unique: false }, + }, + ], + ); + expect(resolveIndexName(bound[0])).toBe('room_1_createdAt_1'); + expect(indexIdentity(bound[0])).toEqual({ + fields: ['room', 'createdAt'], + unique: false, + }); + }); + + it('treats unique vs non-unique as different identities', () => { + const bound = bindDeclaredIndexesToLive( + [{ fields: ['email'], options: { unique: true } }], + [ + { + name: 'email_1', + fields: ['email'], + options: { name: 'email_1', unique: false }, + }, + ], + ); + expect(resolveIndexName(bound[0])).not.toBe('email_1'); + expect(resolveIndexName(bound[0])).toMatch(/uidx/); + }); + + it('keeps Admin extras and drops stale generated names for the same identity', () => { + const unioned = keepDeclaredIndexExtras( + [ + { + fields: ['room', 'createdAt'], + name: 'room_1_createdAt_1', + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ], + [ + { + fields: ['room', 'createdAt'], + name: 'cnd_idx_room_createdAt_asc_asc', + }, + { fields: ['email'], name: 'admin_email_idx' }, + ], + ); + expect(unioned.map(index => index.name)).toEqual([ + 'room_1_createdAt_1', + 'admin_email_idx', + ]); + }); + + it('overlays declared Compatible types onto live indexes by identity when names differ', () => { + const overlaid = overlayDeclaredOnLive( + { + name: 'room_1_createdAt_1', + fields: ['room', 'createdAt'], + types: [MongoIndexType.Ascending, MongoIndexType.Ascending], + options: { name: 'room_1_createdAt_1' }, + }, + [ + { + fields: ['room', 'createdAt'], + name: 'cnd_idx_room_createdAt_asc_asc', + types: [CompatibleIndexType.Ascending, CompatibleIndexType.Ascending], + }, + ], + ); + expect(overlaid.types).toEqual([ + CompatibleIndexType.Ascending, + CompatibleIndexType.Ascending, + ]); + expect(overlaid.name).toBe('room_1_createdAt_1'); + }); + + it('persists applied indexes against a re-read declared schema list', async () => { + const findOne = async () => ({ + _id: 'declared-1', + modelOptions: { indexes: [{ fields: ['keep'], name: 'keep_me' }] }, + }); + const findByIdAndUpdate = async () => ({}); + const originalSchema = { + modelOptions: { indexes: [] as { fields: string[]; name?: string }[] }, + }; + const persisted = await persistDeclaredSchemaIndexes({ + declaredSchemaModel: { findOne, findByIdAndUpdate }, + schemaName: 'User', + originalSchema, + applied: [{ fields: ['email'], name: 'cnd_idx_email_asc' }], + }); + expect(persisted).toBe(true); + expect(originalSchema.modelOptions.indexes.map(index => index.name)).toEqual([ + 'keep_me', + 'cnd_idx_email_asc', + ]); }); it('allows dialect-native types and rejects foreign leftovers', () => { diff --git a/modules/database/src/adapters/DatabaseAdapter.ts b/modules/database/src/adapters/DatabaseAdapter.ts index 346c15ab3..bc4e9f39f 100644 --- a/modules/database/src/adapters/DatabaseAdapter.ts +++ b/modules/database/src/adapters/DatabaseAdapter.ts @@ -18,6 +18,10 @@ import { Schema, } from '../interfaces/index.js'; import { stitchSchema, validateExtensionFields } from './utils/extensions.js'; +import { + keepDeclaredIndexExtras, + persistDeclaredSchemaIndexes, +} from './utils/indexes.js'; import { status } from '@grpc/grpc-js'; import { isEqual, isNil } from 'lodash-es'; import ObjectHash from 'object-hash'; @@ -551,6 +555,24 @@ export abstract class DatabaseAdapter { instanceSync: boolean, ): Promise; + protected async persistIndexesAndPublish(args: { + schemaName: string; + originalSchema: ConduitDatabaseSchema; + applied?: ModelOptionsIndexes[]; + droppedNames?: string[]; + }): Promise { + if (!this.models['_DeclaredSchema']) return false; + const persisted = await persistDeclaredSchemaIndexes({ + declaredSchemaModel: this.models['_DeclaredSchema'], + schemaName: args.schemaName, + originalSchema: args.originalSchema, + applied: args.applied, + droppedNames: args.droppedNames, + }); + if (persisted) this.publishSchema(args.originalSchema); + return persisted; + } + protected async saveSchemaToDatabase(schema: ConduitSchema) { if (schema.name === '_DeclaredSchema') return; const model = await this.models['_DeclaredSchema'].findOne( @@ -558,6 +580,10 @@ export abstract class DatabaseAdapter { { readPreference: 'primary' }, ); if (model) { + schema.modelOptions.indexes = keepDeclaredIndexExtras( + schema.modelOptions.indexes ?? [], + model.modelOptions?.indexes, + ); await this.models['_DeclaredSchema'].findByIdAndUpdate(model._id, { name: schema.name, fields: schema.fields, diff --git a/modules/database/src/adapters/mongoose-adapter/index.ts b/modules/database/src/adapters/mongoose-adapter/index.ts index aeaa53a2e..39196ffb2 100644 --- a/modules/database/src/adapters/mongoose-adapter/index.ts +++ b/modules/database/src/adapters/mongoose-adapter/index.ts @@ -14,15 +14,16 @@ import { DatabaseAdapter } from '../DatabaseAdapter.js'; import { validateFieldChanges, validateFieldConstraints } from '../utils/index.js'; import { assertUniqueIndexPrivilege, - declaredIndexMap, + bindDeclaredIndexesToLive, ensureIndexName, + findLiveIndex, isIndexAlreadyExistsError, + isIndexKeySpecsConflictError, + liveIndexFromMongo, mapCompatibleToMongo, - mergeDeclaredIndexes, mongoAllowsIndexType, normalizeIndexTypes, - persistDeclaredSchemaIndexes, - removeDeclaredIndexes, + overlayDeclaredOnLive, removeIndexFromSchemaFields, toMutableIndexes, validateIndexFields, @@ -641,14 +642,20 @@ export class MongooseAdapter extends DatabaseAdapter { ): Promise { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); - const prepared = this.checkIndexes( - schemaName, - indexes, - callerModule, - options?.privileged, + const live = await this.listLiveIndexes(schemaName); + const prepared = bindDeclaredIndexesToLive( + this.checkIndexes(schemaName, indexes, callerModule, options?.privileged), + live, ); const collection = this.mongoose.model(schemaName).collection; + const applied: ModelOptionsIndexes[] = []; + let failure: unknown; for (const index of prepared) { + const existing = findLiveIndex(live, index); + if (existing) { + applied.push(index); + continue; + } const spec: Record = {}; const types = normalizeIndexTypes(index.types, index.fields.length); for (let i = 0; i < index.fields.length; i++) { @@ -658,22 +665,48 @@ export class MongooseAdapter extends DatabaseAdapter { } try { await collection.createIndex(spec, index.options); + applied.push(index); + live.push(index); } catch (e) { - if (isIndexAlreadyExistsError(e)) continue; - throw new GrpcError(status.INTERNAL, (e as Error).message); + if (isIndexAlreadyExistsError(e)) { + applied.push(index); + continue; + } + if (isIndexKeySpecsConflictError(e)) { + const relisted = await this.listLiveIndexes(schemaName); + const match = findLiveIndex(relisted, index); + if (match) { + applied.push(bindDeclaredIndexesToLive([index], relisted)[0]); + live.splice(0, live.length, ...relisted); + continue; + } + } + failure = e; + break; } } - const original = this.models[schemaName].originalSchema; - const merged = mergeDeclaredIndexes(original.modelOptions.indexes, prepared); - await persistDeclaredSchemaIndexes({ - declaredSchemaModel: this.models['_DeclaredSchema'], - schemaName, - originalSchema: original, - indexes: merged, - }); + if (!failure || applied.length > 0) { + await this.persistIndexesAndPublish({ + schemaName, + originalSchema: this.models[schemaName].originalSchema, + applied, + }); + } + if (failure) { + throw new GrpcError(status.INTERNAL, (failure as Error).message); + } return 'Indexes created!'; } + private async listLiveIndexes(schemaName: string): Promise { + try { + const result = await this.mongoose.model(schemaName).collection.indexes(); + return result.map(liveIndexFromMongo); + } catch { + return []; + } + } + private async createMongooseFieldIndexes(schemaName: string): Promise { const model = this.models[schemaName]; if (!model) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); @@ -682,6 +715,7 @@ export class MongooseAdapter extends DatabaseAdapter { const declaredIndexes = model.model.schema.indexes(); if (!declaredIndexes.length) return; + const live = await this.listLiveIndexes(schemaName); const collection = this.mongoose.model(schemaName).collection; for (const [keys, rawOptions] of declaredIndexes) { const indexKeys = keys as IndexSpecification; @@ -690,9 +724,20 @@ export class MongooseAdapter extends DatabaseAdapter { const options = this.sanitizeMongooseIndexOptions( rawOptions as Record, ); - await collection.createIndex(indexKeys, options).catch((e: Error) => { - throw new GrpcError(status.INTERNAL, e.message); - }); + const declared = { + fields: Object.keys((indexKeys as Record) ?? {}), + options: { + unique: options?.unique === true, + name: typeof options?.name === 'string' ? options.name : undefined, + }, + }; + if (findLiveIndex(live, declared)) continue; + try { + await collection.createIndex(indexKeys, options); + } catch (e) { + if (isIndexAlreadyExistsError(e) || isIndexKeySpecsConflictError(e)) continue; + throw new GrpcError(status.INTERNAL, (e as Error).message); + } } } @@ -716,9 +761,7 @@ export class MongooseAdapter extends DatabaseAdapter { throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); const collection = this.mongoose.model(schemaName).collection; const result = await collection.indexes(); - const declaredByName = declaredIndexMap( - this.models[schemaName].originalSchema.modelOptions.indexes, - ); + const declared = this.models[schemaName].originalSchema.modelOptions.indexes; return result.map(index => { const options: Record = {}; for (const [key, value] of Object.entries(index)) { @@ -732,19 +775,15 @@ export class MongooseAdapter extends DatabaseAdapter { types.push(type as MongoIndexType); } const name = typeof options.name === 'string' ? options.name : index.name; - const declaredIndex = name ? declaredByName.get(name) : undefined; - const live: ModelOptionsIndexes = { - name, - fields, - types, - options: { ...options, name }, - }; - if (!declaredIndex) return live; - return { - ...live, - types: declaredIndex.types ?? live.types, - options: { ...declaredIndex.options, ...live.options, name }, - }; + return overlayDeclaredOnLive( + { + name, + fields, + types, + options: { ...options, name }, + }, + declared, + ); }); } @@ -752,24 +791,31 @@ export class MongooseAdapter extends DatabaseAdapter { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); const collection = this.mongoose.model(schemaName).collection; + const dropped: string[] = []; + let failure: unknown; for (const name of indexNames) { try { await collection.dropIndex(name); - } catch { - throw new GrpcError(status.INTERNAL, 'Unsuccessful index deletion'); + dropped.push(name); + } catch (e) { + failure = e; + break; } } const original = this.models[schemaName].originalSchema; - for (const name of indexNames) { + for (const name of dropped) { removeIndexFromSchemaFields(original, name); } - const remaining = removeDeclaredIndexes(original.modelOptions.indexes, indexNames); - await persistDeclaredSchemaIndexes({ - declaredSchemaModel: this.models['_DeclaredSchema'], - schemaName, - originalSchema: original, - indexes: remaining, - }); + if (!failure || dropped.length > 0) { + await this.persistIndexesAndPublish({ + schemaName, + originalSchema: original, + droppedNames: dropped, + }); + } + if (failure) { + throw new GrpcError(status.INTERNAL, 'Unsuccessful index deletion'); + } return 'Indexes deleted'; } @@ -889,6 +935,13 @@ export class MongooseAdapter extends DatabaseAdapter { schema, this, ); + if (!isInstanceSync && schema.modelOptions.indexes?.length) { + const live = await this.listLiveIndexes(schema.name); + schema.modelOptions.indexes = bindDeclaredIndexesToLive( + schema.modelOptions.indexes, + live, + ); + } if (saveToDb) { await this.compareAndStoreMigratedSchema(schema); await this.saveSchemaToDatabase(schema); diff --git a/modules/database/src/adapters/sequelize-adapter/index.ts b/modules/database/src/adapters/sequelize-adapter/index.ts index 4e3c1128f..d8ac166ea 100644 --- a/modules/database/src/adapters/sequelize-adapter/index.ts +++ b/modules/database/src/adapters/sequelize-adapter/index.ts @@ -30,15 +30,16 @@ import { pgSchemaConverter } from './postgres-adapter/PgSchemaConverter.js'; import { isEqual, isNil } from 'lodash-es'; import { assertUniqueIndexPrivilege, - declaredIndexMap, + bindDeclaredIndexesToLive, ensureIndexName, + findLiveIndex, inferSqlIndexType, isIndexAlreadyExistsError, + isIndexKeySpecsConflictError, isPostgresIndexType, - mergeDeclaredIndexes, + liveIndexFromSql, + overlayDeclaredOnLive, normalizeIndexTypes, - persistDeclaredSchemaIndexes, - removeDeclaredIndexes, removeIndexFromSchemaFields, sqlDialectAllowsIndexType, sqlIndexFields, @@ -261,10 +262,26 @@ export abstract class SequelizeAdapter extends DatabaseAdapter this.sequelize.models, ); const dialect = this.sequelize.getDialect(); + const live = isInstanceSync + ? [] + : await this.listLiveIndexesForCollection(this.getCollectionName(schema)); + if (!isInstanceSync && schema.modelOptions.indexes?.length) { + schema.modelOptions.indexes = bindDeclaredIndexesToLive( + schema.modelOptions.indexes, + live, + ); + compiledSchema.modelOptions.indexes = schema.modelOptions.indexes; + } const [newSchema, objectPaths, extractedRelations] = dialect === 'postgres' ? pgSchemaConverter(compiledSchema) : sqlSchemaConverter(compiledSchema, dialect as 'mysql' | 'mariadb' | 'sqlite'); + if (!isInstanceSync && newSchema.modelOptions.indexes?.length) { + newSchema.modelOptions.indexes = bindDeclaredIndexesToLive( + newSchema.modelOptions.indexes, + live, + ); + } this.registeredSchemas.set( schema.name, Object.freeze(JSON.parse(JSON.stringify(schema))), @@ -378,36 +395,72 @@ export abstract class SequelizeAdapter extends DatabaseAdapter ): Promise { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); - const prepared = this.checkAndConvertIndexes( - schemaName, - indexes, - callerModule, - options?.privileged, - ); const collectionName = this.models[schemaName].originalSchema.collectionName; + const live = await this.listLiveIndexesForCollection(collectionName); + const prepared = bindDeclaredIndexesToLive( + this.checkAndConvertIndexes(schemaName, indexes, callerModule, options?.privileged), + live, + ); const queryInterface = this.sequelize.getQueryInterface(); + const applied: ModelOptionsIndexes[] = []; + let failure: unknown; for (const index of prepared) { + const existing = findLiveIndex(live, index); + if (existing) { + applied.push(index); + continue; + } try { await queryInterface.addIndex(collectionName, { fields: sqlIndexFields(index), ...index.options, }); + applied.push(index); + live.push(index); } catch (e) { - if (isIndexAlreadyExistsError(e)) continue; - throw new GrpcError(status.INTERNAL, 'Unsuccessful index creation'); + if (isIndexAlreadyExistsError(e)) { + applied.push(index); + continue; + } + if (isIndexKeySpecsConflictError(e)) { + const relisted = await this.listLiveIndexesForCollection(collectionName); + const match = findLiveIndex(relisted, index); + if (match) { + applied.push(bindDeclaredIndexesToLive([index], relisted)[0]); + live.splice(0, live.length, ...relisted); + continue; + } + } + failure = e; + break; } } - const original = this.models[schemaName].originalSchema; - const merged = mergeDeclaredIndexes(original.modelOptions.indexes, prepared); - await persistDeclaredSchemaIndexes({ - declaredSchemaModel: this.models['_DeclaredSchema'], - schemaName, - originalSchema: original, - indexes: merged, - }); + if (!failure || applied.length > 0) { + await this.persistIndexesAndPublish({ + schemaName, + originalSchema: this.models[schemaName].originalSchema, + applied, + }); + } + if (failure) { + throw new GrpcError(status.INTERNAL, 'Unsuccessful index creation'); + } return 'Indexes created!'; } + private async listLiveIndexesForCollection( + collectionName: string, + ): Promise { + try { + const result = (await this.sequelize + .getQueryInterface() + .showIndex(collectionName)) as UntypedArray; + return result.map(liveIndexFromSql); + } catch { + return []; + } + } + async getIndexes(schemaName: string): Promise { if (!this.models[schemaName]) throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); @@ -415,27 +468,22 @@ export abstract class SequelizeAdapter extends DatabaseAdapter const queryInterface = this.sequelize.getQueryInterface(); const result = (await queryInterface.showIndex(collectionName)) as UntypedArray; const dialect = this.sequelize.getDialect(); - const declaredByName = declaredIndexMap( - this.models[schemaName].originalSchema.modelOptions.indexes, - ); + const declared = this.models[schemaName].originalSchema.modelOptions.indexes; return result.map(row => { const fields = (row.fields ?? []).map((field: unknown) => typeof field === 'string' ? field : (field as { attribute?: string }).attribute, ); const name = row.name as string; - const declaredIndex = declaredByName.get(name); - const live: ModelOptionsIndexes = { - name, - fields, - types: inferSqlIndexType(row, dialect), - options: { name, unique: !!row.unique }, - }; - if (!declaredIndex) return live; - return { - ...live, - types: declaredIndex.types ?? live.types, - options: { ...declaredIndex.options, ...live.options, name }, - }; + return overlayDeclaredOnLive( + { + name, + fields, + types: inferSqlIndexType(row, dialect), + options: { name, unique: !!row.unique }, + ...(row.primary ? { primary: true } : {}), + }, + declared, + ); }); } @@ -444,24 +492,31 @@ export abstract class SequelizeAdapter extends DatabaseAdapter throw new GrpcError(status.NOT_FOUND, 'Requested schema not found'); const collectionName = this.models[schemaName].originalSchema.collectionName; const queryInterface = this.sequelize.getQueryInterface(); + const dropped: string[] = []; + let failure: unknown; for (const name of indexNames) { try { await queryInterface.removeIndex(collectionName, name); - } catch { - throw new GrpcError(status.INTERNAL, 'Unsuccessful index deletion'); + dropped.push(name); + } catch (e) { + failure = e; + break; } } const original = this.models[schemaName].originalSchema; - for (const name of indexNames) { + for (const name of dropped) { removeIndexFromSchemaFields(original, name); } - const remaining = removeDeclaredIndexes(original.modelOptions.indexes, indexNames); - await persistDeclaredSchemaIndexes({ - declaredSchemaModel: this.models['_DeclaredSchema'], - schemaName, - originalSchema: original, - indexes: remaining, - }); + if (!failure || dropped.length > 0) { + await this.persistIndexesAndPublish({ + schemaName, + originalSchema: original, + droppedNames: dropped, + }); + } + if (failure) { + throw new GrpcError(status.INTERNAL, 'Unsuccessful index deletion'); + } return 'Indexes deleted'; } diff --git a/modules/database/src/adapters/utils/indexes.ts b/modules/database/src/adapters/utils/indexes.ts index 9338b6ecb..3c1964e1f 100644 --- a/modules/database/src/adapters/utils/indexes.ts +++ b/modules/database/src/adapters/utils/indexes.ts @@ -48,7 +48,158 @@ export function resolveIndexName(index: ModelOptionsIndexes): string | undefined } export function isUniqueIndex(index: ModelOptionsIndexes): boolean { - return index.options?.unique === true; + return index.options?.unique === true || index.unique === true; +} + +export type IndexIdentity = { fields: string[]; unique: boolean }; + +export function indexFieldNames( + index: Pick | { fields?: readonly unknown[] }, +): string[] { + return (index.fields ?? []) + .map(field => { + if (typeof field === 'string') return field; + if (field && typeof field === 'object') { + const obj = field as { name?: string; attribute?: string }; + if (typeof obj.name === 'string' && obj.name.length > 0) return obj.name; + if (typeof obj.attribute === 'string' && obj.attribute.length > 0) { + return obj.attribute; + } + } + return ''; + }) + .filter(name => name.length > 0); +} + +export function indexIdentity(index: ModelOptionsIndexes): IndexIdentity { + return { + fields: indexFieldNames(index), + unique: isUniqueIndex(index), + }; +} + +export function indexIdentitiesEqual(a: IndexIdentity, b: IndexIdentity): boolean { + return ( + a.unique === b.unique && + a.fields.length === b.fields.length && + a.fields.every((field, i) => field === b.fields[i]) + ); +} + +export function indexIdentityKey(identity: IndexIdentity): string { + return `${identity.unique ? 'u' : 'n'}:${identity.fields.join('\0')}`; +} + +export function isSkippedLiveIndex(index: ModelOptionsIndexes): boolean { + if (index.primary === true) return true; + const name = resolveIndexName(index); + return name === '_id_' || name === 'PRIMARY'; +} + +export function findLiveIndex( + live: readonly ModelOptionsIndexes[], + declared: ModelOptionsIndexes, +): ModelOptionsIndexes | undefined { + const wanted = indexIdentity(declared); + return live.find( + row => !isSkippedLiveIndex(row) && indexIdentitiesEqual(indexIdentity(row), wanted), + ); +} + +export function bindDeclaredIndexesToLive( + declared: readonly T[], + live: readonly ModelOptionsIndexes[], +): T[] { + return declared.map(index => { + const match = findLiveIndex(live, index); + if (match) { + const name = resolveIndexName(match); + if (name) { + return { + ...index, + name, + options: { ...index.options, name }, + }; + } + } + const fields = indexFieldNames(index); + const stringFields = Array.isArray(index.fields) + ? index.fields.every(field => typeof field === 'string') + : false; + if (stringFields && fields.length === index.fields.length) { + return ensureIndexName(index) as T; + } + return index; + }); +} + +export function keepDeclaredIndexExtras( + incomingBound: readonly ModelOptionsIndexes[], + existingDb: readonly ModelOptionsIndexes[] | undefined, +): ModelOptionsIndexes[] { + const incoming = incomingBound.map(index => { + const name = resolveIndexName(index); + return name + ? { ...index, name, options: { ...index.options, name } } + : ensureIndexName(index); + }); + const incomingNames = new Set( + incoming.map(resolveIndexName).filter((name): name is string => Boolean(name)), + ); + const incomingIdentities = new Set( + incoming.map(index => indexIdentityKey(indexIdentity(index))), + ); + const extras: ModelOptionsIndexes[] = []; + for (const index of existingDb ?? []) { + const name = resolveIndexName(index); + if (name && incomingNames.has(name)) continue; + if (incomingIdentities.has(indexIdentityKey(indexIdentity(index)))) continue; + extras.push(ensureIndexName(index)); + } + return [...incoming, ...extras]; +} + +export function overlayDeclaredOnLive( + live: ModelOptionsIndexes, + declared: readonly ModelOptionsIndexes[] | undefined, +): ModelOptionsIndexes { + if (!declared?.length) return live; + const name = resolveIndexName(live); + const byName = name ? declaredIndexMap(declared).get(name) : undefined; + const match = byName ?? findLiveIndex(declared, live); + if (!match) return live; + const liveName = name ?? resolveIndexName(match); + return { + ...live, + types: match.types ?? live.types, + options: { ...match.options, ...live.options, name: liveName }, + }; +} + +export function liveIndexFromMongo(index: { + key?: Record; + name?: string; + unique?: boolean; +}): ModelOptionsIndexes { + return { + name: index.name, + fields: Object.keys(index.key ?? {}), + options: { name: index.name, unique: !!index.unique }, + }; +} + +export function liveIndexFromSql(row: { + name?: string; + unique?: boolean; + primary?: boolean; + fields?: Array; +}): ModelOptionsIndexes { + return { + name: row.name, + fields: indexFieldNames({ fields: row.fields ?? [] }), + options: { name: row.name, unique: !!row.unique }, + ...(row.primary ? { primary: true } : {}), + }; } export function normalizeIndexTypes( @@ -218,23 +369,72 @@ export function assertUniqueIndexPrivilege(args: { throw new GrpcError(status.PERMISSION_DENIED, 'Not authorized to create unique index'); } +type ErrorPart = { code?: number | string; message?: string; name?: string }; + +function walkError(error: unknown): ErrorPart[] { + const parts: ErrorPart[] = []; + const seen = new Set(); + let current: unknown = error; + while (current && typeof current === 'object' && !seen.has(current)) { + seen.add(current); + const err = current as ErrorPart & { + original?: unknown; + parent?: unknown; + cause?: unknown; + }; + parts.push({ code: err.code, message: err.message, name: err.name }); + current = err.original ?? err.parent ?? err.cause; + } + return parts; +} + +const UNIQUE_OR_OPTIONS_CONFLICT_CODES = new Set([ + 85, + '85', + 86, + '86', + 11000, + '11000', + 23505, + '23505', + 1062, + '1062', +]); + +export function isIndexKeySpecsConflictError(error: unknown): boolean { + return walkError(error).some(part => part.code === 86 || part.code === '86'); +} + export function isIndexAlreadyExistsError(error: unknown): boolean { - const err = error as { message?: string; code?: number | string; name?: string }; - const message = (err.message ?? '').toLowerCase(); - return ( - message.includes('already exists') || - message.includes('already exist') || - message.includes('duplicate key name') || - message.includes('index already exists') || - err.code === 85 || - err.code === '42P07' || - err.name === 'SequelizeUniqueConstraintError' - ); + const parts = walkError(error); + if (parts.some(part => part.name === 'SequelizeUniqueConstraintError')) { + return false; + } + if ( + parts.some( + part => part.code !== undefined && UNIQUE_OR_OPTIONS_CONFLICT_CODES.has(part.code), + ) + ) { + return false; + } + for (const part of parts) { + if (part.code === '42P07' || part.code === 1061 || part.code === '1061') { + return true; + } + const message = part.message ?? ''; + if (/index .+ already exists/i.test(message)) return true; + if (/duplicate key name/i.test(message)) return true; + if (/relation .+ already exists/i.test(message)) return true; + } + return false; } export async function persistDeclaredSchemaIndexes(args: { declaredSchemaModel: { - findOne: (query: Record) => Promise<{ _id: string } | null>; + findOne: (query: Record) => Promise<{ + _id: string; + modelOptions?: { indexes?: ModelOptionsIndexes[] }; + } | null>; findByIdAndUpdate: (id: string, update: Record) => Promise; }; schemaName: string; @@ -243,16 +443,24 @@ export async function persistDeclaredSchemaIndexes(args: { fields?: Record; compiledFields?: Record; }; - indexes: ModelOptionsIndexes[]; -}): Promise { - args.originalSchema.modelOptions.indexes = args.indexes; + applied?: ModelOptionsIndexes[]; + droppedNames?: string[]; +}): Promise { const found = await args.declaredSchemaModel.findOne({ name: args.schemaName }); - if (!found) return; + const memoryIndexes = (args.originalSchema.modelOptions.indexes ?? + []) as ModelOptionsIndexes[]; + const dbIndexes = found?.modelOptions?.indexes ?? memoryIndexes; + const next = args.droppedNames + ? removeDeclaredIndexes(dbIndexes, args.droppedNames) + : mergeDeclaredIndexes(dbIndexes, args.applied ?? []); + args.originalSchema.modelOptions.indexes = next; + if (!found) return false; await args.declaredSchemaModel.findByIdAndUpdate(found._id, { modelOptions: args.originalSchema.modelOptions, fields: args.originalSchema.fields, compiledFields: args.originalSchema.compiledFields, }); + return true; } export function collectExistingIndexNames( From 4025444dbe38ecfb2905fb9c430ef36ae2f80b9e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 10:34:54 +0000 Subject: [PATCH 4/4] fix(database): persist indexes from primary and snapshot after bind Read _DeclaredSchema from primary before index persist, reuse SQL name-exists errors only when fields+unique match, and freeze registeredSchemas after bind/save so replica catch-up gets live names. --- .../src/__tests__/indexes/adapters.test.ts | 83 ++++++++++++++++++- .../src/__tests__/indexes/helpers.test.ts | 52 +++++++++++- .../database/src/adapters/DatabaseAdapter.ts | 7 ++ .../src/adapters/mongoose-adapter/index.ts | 51 +++++++----- .../sequelize-adapter/SequelizeSchema.ts | 1 + .../src/adapters/sequelize-adapter/index.ts | 45 +++++----- .../database/src/adapters/utils/indexes.ts | 28 ++++++- 7 files changed, 218 insertions(+), 49 deletions(-) diff --git a/modules/database/src/__tests__/indexes/adapters.test.ts b/modules/database/src/__tests__/indexes/adapters.test.ts index 565624e53..fbf3b8fa5 100644 --- a/modules/database/src/__tests__/indexes/adapters.test.ts +++ b/modules/database/src/__tests__/indexes/adapters.test.ts @@ -143,6 +143,7 @@ describe('mongoose adapter indexes', () => { 'chat', ); expect(created.findByIdAndUpdate).toHaveBeenCalledTimes(1); + expect(created.findOne.mock.calls[0][1]).toEqual({ readPreference: 'primary' }); const update = created.findByIdAndUpdate.mock.calls[0][1] as { modelOptions: { indexes: { name?: string }[] }; }; @@ -296,6 +297,21 @@ describe('mongoose adapter indexes', () => { expect(publish).toHaveBeenCalledWith('database:create:schema', expect.any(String)); }); + it('throws on a name-already-exists error when the live name indexes different fields', async () => { + const { adapter, createIndex, indexes, findByIdAndUpdate } = makeMongooseAdapter(); + createIndex.mockRejectedValue({ message: 'index user_idx already exists' }); + indexes + .mockResolvedValueOnce([{ v: 2, key: { _id: 1 }, name: '_id_' }]) + .mockResolvedValueOnce([ + { v: 2, key: { _id: 1 }, name: '_id_' }, + { v: 2, key: { room: 1 }, name: 'user_idx', unique: false }, + ]); + await expect( + adapter.createIndexes('User', [{ fields: ['email'], name: 'user_idx' }], 'chat'), + ).rejects.toMatchObject({ code: status.INTERNAL }); + expect(findByIdAndUpdate).not.toHaveBeenCalled(); + }); + it('rebinds to the live name on Mongo 86 instead of persisting a generated name', async () => { const { adapter, createIndex, indexes, findByIdAndUpdate } = makeMongooseAdapter(); createIndex.mockRejectedValue({ code: 86, message: 'IndexKeySpecsConflict' }); @@ -452,8 +468,16 @@ describe('sequelize adapter indexes', () => { expect(update.modelOptions.indexes[0].name).toBe('room_createdAt'); }); - it('skips and persists on 42P07 name conflicts', async () => { - const { adapter, addIndex, findByIdAndUpdate, publish } = makeSequelizeAdapter(); + it('skips and persists on 42P07 when the live name has the same identity', async () => { + const { adapter, addIndex, showIndex, findByIdAndUpdate, publish } = + makeSequelizeAdapter(); + showIndex.mockResolvedValueOnce([]).mockResolvedValueOnce([ + { + name: 'cnd_idx_email_asc', + unique: false, + fields: [{ attribute: 'email', order: 'ASC' }], + }, + ]); addIndex.mockRejectedValue({ original: { code: '42P07', message: 'relation "cnd_idx_email_asc" already exists' }, }); @@ -468,6 +492,61 @@ describe('sequelize adapter indexes', () => { expect(publish).toHaveBeenCalled(); }); + it('throws on 42P07 when the live name indexes different fields', async () => { + const { adapter, addIndex, showIndex, findByIdAndUpdate } = makeSequelizeAdapter(); + showIndex.mockResolvedValueOnce([]).mockResolvedValueOnce([ + { + name: 'user_idx', + unique: false, + fields: [{ attribute: 'room' }], + }, + ]); + addIndex.mockRejectedValue({ + original: { code: '42P07', message: 'relation "user_idx" already exists' }, + }); + await expect( + adapter.createIndexes( + 'User', + [{ fields: ['email'], name: 'user_idx' }], + 'database', + ), + ).rejects.toMatchObject({ code: status.INTERNAL }); + expect(findByIdAndUpdate).not.toHaveBeenCalled(); + }); + + it('persists the applied prefix when a later 42P07 name has different fields', async () => { + const { adapter, addIndex, showIndex, findByIdAndUpdate, publish } = + makeSequelizeAdapter(); + showIndex.mockResolvedValueOnce([]).mockResolvedValueOnce([ + { + name: 'user_idx', + unique: false, + fields: [{ attribute: 'username' }], + }, + ]); + addIndex.mockResolvedValueOnce(undefined).mockRejectedValueOnce({ + original: { code: '42P07', message: 'relation "user_idx" already exists' }, + }); + await expect( + adapter.createIndexes( + 'User', + [ + { fields: ['email'], types: [CompatibleIndexType.Ascending] }, + { fields: ['room'], name: 'user_idx' }, + ], + 'database', + ), + ).rejects.toMatchObject({ code: status.INTERNAL }); + expect(findByIdAndUpdate).toHaveBeenCalledTimes(1); + const update = findByIdAndUpdate.mock.calls[0][1] as { + modelOptions: { indexes: { name?: string }[] }; + }; + expect(update.modelOptions.indexes.map(index => index.name)).toEqual([ + 'cnd_idx_email_asc', + ]); + expect(publish).toHaveBeenCalled(); + }); + it('throws on 23505 unique collisions and does not persist that index', async () => { const { adapter, addIndex, findByIdAndUpdate } = makeSequelizeAdapter(); addIndex.mockRejectedValue({ diff --git a/modules/database/src/__tests__/indexes/helpers.test.ts b/modules/database/src/__tests__/indexes/helpers.test.ts index 48ec49ba8..c6e5aab40 100644 --- a/modules/database/src/__tests__/indexes/helpers.test.ts +++ b/modules/database/src/__tests__/indexes/helpers.test.ts @@ -16,6 +16,7 @@ import { isIndexAlreadyExistsError, isMongoIndexType, keepDeclaredIndexExtras, + liveNameConflictAllowsReuse, mapCompatibleToMongo, mapCompatibleToSqlOrder, mergeDeclaredIndexes, @@ -310,10 +311,14 @@ describe('index helpers', () => { }); it('persists applied indexes against a re-read declared schema list', async () => { - const findOne = async () => ({ - _id: 'declared-1', - modelOptions: { indexes: [{ fields: ['keep'], name: 'keep_me' }] }, - }); + let findOneOptions: unknown; + const findOne = async (_query: Record, options?: unknown) => { + findOneOptions = options; + return { + _id: 'declared-1', + modelOptions: { indexes: [{ fields: ['keep'], name: 'keep_me' }] }, + }; + }; const findByIdAndUpdate = async () => ({}); const originalSchema = { modelOptions: { indexes: [] as { fields: string[]; name?: string }[] }, @@ -325,12 +330,51 @@ describe('index helpers', () => { applied: [{ fields: ['email'], name: 'cnd_idx_email_asc' }], }); expect(persisted).toBe(true); + expect(findOneOptions).toEqual({ readPreference: 'primary' }); expect(originalSchema.modelOptions.indexes.map(index => index.name)).toEqual([ 'keep_me', 'cnd_idx_email_asc', ]); }); + it('reuses a live name-conflict only when identity matches', () => { + const live = [ + { + name: 'user_idx', + fields: ['email'], + options: { name: 'user_idx', unique: false }, + }, + ]; + expect( + liveNameConflictAllowsReuse( + { name: 'user_idx', fields: ['email'], options: { name: 'user_idx' } }, + live, + ), + ).toBe(true); + expect( + liveNameConflictAllowsReuse( + { name: 'user_idx', fields: ['username'], options: { name: 'user_idx' } }, + live, + ), + ).toBe(false); + expect( + liveNameConflictAllowsReuse( + { + name: 'user_idx', + fields: ['email'], + options: { name: 'user_idx', unique: true }, + }, + live, + ), + ).toBe(false); + expect( + liveNameConflictAllowsReuse( + { name: 'user_idx', fields: ['email'], options: { name: 'user_idx' } }, + [], + ), + ).toBe(false); + }); + it('allows dialect-native types and rejects foreign leftovers', () => { expect(sqlDialectAllowsIndexType('mysql', CompatibleIndexType.Ascending)).toBe(true); expect(sqlDialectAllowsIndexType('mariadb', CompatibleIndexType.Descending)).toBe( diff --git a/modules/database/src/adapters/DatabaseAdapter.ts b/modules/database/src/adapters/DatabaseAdapter.ts index bc4e9f39f..69c9e469d 100644 --- a/modules/database/src/adapters/DatabaseAdapter.ts +++ b/modules/database/src/adapters/DatabaseAdapter.ts @@ -555,6 +555,13 @@ export abstract class DatabaseAdapter { instanceSync: boolean, ): Promise; + protected snapshotRegisteredSchema(schema: ConduitSchema) { + this.registeredSchemas.set( + schema.name, + Object.freeze(JSON.parse(JSON.stringify(schema))), + ); + } + protected async persistIndexesAndPublish(args: { schemaName: string; originalSchema: ConduitDatabaseSchema; diff --git a/modules/database/src/adapters/mongoose-adapter/index.ts b/modules/database/src/adapters/mongoose-adapter/index.ts index 39196ffb2..e82358288 100644 --- a/modules/database/src/adapters/mongoose-adapter/index.ts +++ b/modules/database/src/adapters/mongoose-adapter/index.ts @@ -20,6 +20,7 @@ import { isIndexAlreadyExistsError, isIndexKeySpecsConflictError, liveIndexFromMongo, + liveNameConflictAllowsReuse, mapCompatibleToMongo, mongoAllowsIndexType, normalizeIndexTypes, @@ -669,8 +670,14 @@ export class MongooseAdapter extends DatabaseAdapter { live.push(index); } catch (e) { if (isIndexAlreadyExistsError(e)) { - applied.push(index); - continue; + const relisted = await this.listLiveIndexes(schemaName); + if (liveNameConflictAllowsReuse(index, relisted)) { + applied.push(index); + live.splice(0, live.length, ...relisted); + continue; + } + failure = e; + break; } if (isIndexKeySpecsConflictError(e)) { const relisted = await this.listLiveIndexes(schemaName); @@ -924,10 +931,6 @@ export class MongooseAdapter extends DatabaseAdapter { const newSchema = schemaConverter(compiledSchema); const indexes = newSchema.modelOptions.indexes; delete newSchema.modelOptions.indexes; - this.registeredSchemas.set( - schema.name, - Object.freeze(JSON.parse(JSON.stringify(schema))), - ); this.models[schema.name] = new MongooseSchema( this.grpcSdk, this.mongoose, @@ -935,23 +938,27 @@ export class MongooseAdapter extends DatabaseAdapter { schema, this, ); - if (!isInstanceSync && schema.modelOptions.indexes?.length) { - const live = await this.listLiveIndexes(schema.name); - schema.modelOptions.indexes = bindDeclaredIndexesToLive( - schema.modelOptions.indexes, - live, - ); - } - if (saveToDb) { - await this.compareAndStoreMigratedSchema(schema); - await this.saveSchemaToDatabase(schema); - } + try { + if (!isInstanceSync && schema.modelOptions.indexes?.length) { + const live = await this.listLiveIndexes(schema.name); + schema.modelOptions.indexes = bindDeclaredIndexesToLive( + schema.modelOptions.indexes, + live, + ); + } + if (saveToDb) { + await this.compareAndStoreMigratedSchema(schema); + await this.saveSchemaToDatabase(schema); + } - if (indexes && !isInstanceSync) { - await this.createIndexes(schema.name, indexes, schema.ownerModule); - } - if (!isInstanceSync) { - await this.createMongooseFieldIndexes(schema.name); + if (indexes && !isInstanceSync) { + await this.createIndexes(schema.name, indexes, schema.ownerModule); + } + if (!isInstanceSync) { + await this.createMongooseFieldIndexes(schema.name); + } + } finally { + this.snapshotRegisteredSchema(schema); } return this.models[schema.name]; } diff --git a/modules/database/src/adapters/sequelize-adapter/SequelizeSchema.ts b/modules/database/src/adapters/sequelize-adapter/SequelizeSchema.ts index c7dec1ecd..47b93c6a2 100644 --- a/modules/database/src/adapters/sequelize-adapter/SequelizeSchema.ts +++ b/modules/database/src/adapters/sequelize-adapter/SequelizeSchema.ts @@ -339,6 +339,7 @@ export class SequelizeSchema extends SchemaAdapter> { scope?: string; select?: string; populate?: string[]; + readPreference?: string; }, ) { const filter = await this.getAuthorizedQuery( diff --git a/modules/database/src/adapters/sequelize-adapter/index.ts b/modules/database/src/adapters/sequelize-adapter/index.ts index d8ac166ea..3a662712d 100644 --- a/modules/database/src/adapters/sequelize-adapter/index.ts +++ b/modules/database/src/adapters/sequelize-adapter/index.ts @@ -38,6 +38,7 @@ import { isIndexKeySpecsConflictError, isPostgresIndexType, liveIndexFromSql, + liveNameConflictAllowsReuse, overlayDeclaredOnLive, normalizeIndexTypes, removeIndexFromSchemaFields, @@ -282,10 +283,6 @@ export abstract class SequelizeAdapter extends DatabaseAdapter live, ); } - this.registeredSchemas.set( - schema.name, - Object.freeze(JSON.parse(JSON.stringify(schema))), - ); const relatedSchemas = await resolveRelatedSchemas( schema, extractedRelations, @@ -301,19 +298,23 @@ export abstract class SequelizeAdapter extends DatabaseAdapter objectPaths, ); - const noSync = - this.models[schema.name].originalSchema.modelOptions.conduit!.noSync || - isInstanceSync; - // do not sync extracted schemas - if (isNil(noSync) || !noSync) { - await this.models[schema.name].sync(); - } else { - this.models[schema.name].synced = true; - } - // do not store extracted schemas to db - if (saveToDb && !isInstanceSync) { - await this.compareAndStoreMigratedSchema(schema); - await this.saveSchemaToDatabase(schema); + try { + const noSync = + this.models[schema.name].originalSchema.modelOptions.conduit!.noSync || + isInstanceSync; + // do not sync extracted schemas + if (isNil(noSync) || !noSync) { + await this.models[schema.name].sync(); + } else { + this.models[schema.name].synced = true; + } + // do not store extracted schemas to db + if (saveToDb && !isInstanceSync) { + await this.compareAndStoreMigratedSchema(schema); + await this.saveSchemaToDatabase(schema); + } + } finally { + this.snapshotRegisteredSchema(schema); } return this.models[schema.name]; } @@ -419,8 +420,14 @@ export abstract class SequelizeAdapter extends DatabaseAdapter live.push(index); } catch (e) { if (isIndexAlreadyExistsError(e)) { - applied.push(index); - continue; + const relisted = await this.listLiveIndexesForCollection(collectionName); + if (liveNameConflictAllowsReuse(index, relisted)) { + applied.push(index); + live.splice(0, live.length, ...relisted); + continue; + } + failure = e; + break; } if (isIndexKeySpecsConflictError(e)) { const relisted = await this.listLiveIndexesForCollection(collectionName); diff --git a/modules/database/src/adapters/utils/indexes.ts b/modules/database/src/adapters/utils/indexes.ts index 3c1964e1f..5717bae86 100644 --- a/modules/database/src/adapters/utils/indexes.ts +++ b/modules/database/src/adapters/utils/indexes.ts @@ -106,6 +106,24 @@ export function findLiveIndex( ); } +export function findIndexByName( + indexes: readonly ModelOptionsIndexes[], + name: string, +): ModelOptionsIndexes | undefined { + return indexes.find(index => resolveIndexName(index) === name); +} + +export function liveNameConflictAllowsReuse( + declared: ModelOptionsIndexes, + live: readonly ModelOptionsIndexes[], +): boolean { + const name = resolveIndexName(declared); + if (!name) return false; + const row = findIndexByName(live, name); + if (!row) return false; + return indexIdentitiesEqual(indexIdentity(row), indexIdentity(declared)); +} + export function bindDeclaredIndexesToLive( declared: readonly T[], live: readonly ModelOptionsIndexes[], @@ -431,7 +449,10 @@ export function isIndexAlreadyExistsError(error: unknown): boolean { export async function persistDeclaredSchemaIndexes(args: { declaredSchemaModel: { - findOne: (query: Record) => Promise<{ + findOne: ( + query: Record, + options?: { readPreference?: string }, + ) => Promise<{ _id: string; modelOptions?: { indexes?: ModelOptionsIndexes[] }; } | null>; @@ -446,7 +467,10 @@ export async function persistDeclaredSchemaIndexes(args: { applied?: ModelOptionsIndexes[]; droppedNames?: string[]; }): Promise { - const found = await args.declaredSchemaModel.findOne({ name: args.schemaName }); + const found = await args.declaredSchemaModel.findOne( + { name: args.schemaName }, + { readPreference: 'primary' }, + ); const memoryIndexes = (args.originalSchema.modelOptions.indexes ?? []) as ModelOptionsIndexes[]; const dbIndexes = found?.modelOptions?.indexes ?? memoryIndexes;