Skip to content

Commit fb97582

Browse files
authored
Merge pull request #965 from objectstack-ai/copilot/fix-database-auto-upgrade-issue
2 parents e84befe + 2c55849 commit fb97582

File tree

8 files changed

+212
-14
lines changed

8 files changed

+212
-14
lines changed

CHANGELOG.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
- **Login API error — database tables not created** — Fixed a critical naming mismatch between
12+
the FQN (Fully Qualified Name) used by SchemaRegistry (e.g., `sys__user` with double underscore)
13+
and the physical table name derived by `ObjectSchema.create()` (e.g., `sys_user` with single
14+
underscore). `syncRegisteredSchemas()` now uses the `tableName` property from object definitions
15+
for DDL operations, ensuring tables are created with the correct physical name that matches
16+
what the auth adapter and `SystemObjectName` constants expect.
17+
- **`SchemaRegistry.getObject()` — protocol name resolution** — Added a third fallback that
18+
matches objects by their `tableName` property (e.g., `getObject('sys_user')` now correctly
19+
finds the object registered as FQN `sys__user`). This bridges protocol-layer names
20+
(`SystemObjectName.USER = 'sys_user'`) with the registry's FQN naming convention.
21+
- **`ObjectQL.resolveObjectName()` — physical table name** — Now returns `schema.tableName`
22+
(the physical table/collection name) instead of `schema.name` (the FQN) when available,
23+
ensuring driver SQL queries target the correct table.
24+
- **`SqlDriver.ensureDatabaseExists()` — multi-driver support** — Extended database
25+
auto-creation to support MySQL (error code `ER_BAD_DB_ERROR` / errno 1049) alongside
26+
PostgreSQL (error code `3D000`). SQLite is explicitly skipped (auto-creates files).
27+
- **`SqlDriver.createDatabase()` — MySQL support** — Added MySQL-specific logic that
28+
connects without a database specified and uses `CREATE DATABASE IF NOT EXISTS`.
29+
1030
### Added
1131
- **`@objectstack/driver-turso` plugin** — Migrated and standardized the Turso/libSQL driver from
1232
`@objectql/driver-turso` into `packages/plugins/driver-turso/`. The driver **extends** `SqlDriver`

packages/objectql/src/engine.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,10 @@ export class ObjectQL implements IDataEngine {
545545
private resolveObjectName(name: string): string {
546546
const schema = SchemaRegistry.getObject(name);
547547
if (schema) {
548-
return schema.name; // FQN from registry (e.g., 'todo__task')
548+
// Prefer the physical table name (e.g., 'sys_user') over the FQN
549+
// (e.g., 'sys__user'). ObjectSchema.create() auto-derives tableName
550+
// as {namespace}_{name} which matches the storage convention.
551+
return schema.tableName || schema.name;
549552
}
550553
return name; // Ad-hoc object, keep as-is
551554
}

packages/objectql/src/plugin.integration.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,5 +468,84 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => {
468468
// Act & Assert - should not throw
469469
await expect(kernel.bootstrap()).resolves.not.toThrow();
470470
});
471+
472+
it('should use tableName for syncSchema when objects have auto-derived tableName', async () => {
473+
// Arrange - driver that tracks syncSchema calls
474+
const synced: Array<{ object: string; schema: any }> = [];
475+
const mockDriver = {
476+
name: 'table-name-driver',
477+
version: '1.0.0',
478+
connect: async () => {},
479+
disconnect: async () => {},
480+
find: async () => [],
481+
findOne: async () => null,
482+
create: async (_o: string, d: any) => d,
483+
update: async (_o: string, _i: any, d: any) => d,
484+
delete: async () => true,
485+
syncSchema: async (object: string, schema: any) => {
486+
synced.push({ object, schema });
487+
},
488+
};
489+
490+
await kernel.use({
491+
name: 'mock-driver-plugin',
492+
type: 'driver',
493+
version: '1.0.0',
494+
init: async (ctx) => {
495+
ctx.registerService('driver.table-name', mockDriver);
496+
},
497+
});
498+
499+
// Objects with tableName (simulating ObjectSchema.create() output)
500+
const appManifest = {
501+
id: 'com.test.system',
502+
name: 'system',
503+
namespace: 'sys',
504+
version: '1.0.0',
505+
objects: [
506+
{
507+
name: 'user',
508+
label: 'User',
509+
namespace: 'sys',
510+
tableName: 'sys_user',
511+
fields: {
512+
email: { name: 'email', label: 'Email', type: 'text' },
513+
},
514+
},
515+
{
516+
name: 'session',
517+
label: 'Session',
518+
namespace: 'sys',
519+
tableName: 'sys_session',
520+
fields: {
521+
token: { name: 'token', label: 'Token', type: 'text' },
522+
},
523+
},
524+
],
525+
};
526+
527+
await kernel.use({
528+
name: 'mock-app-plugin',
529+
type: 'app',
530+
version: '1.0.0',
531+
init: async (ctx) => {
532+
ctx.registerService('app.system', appManifest);
533+
},
534+
});
535+
536+
const plugin = new ObjectQLPlugin();
537+
await kernel.use(plugin);
538+
539+
// Act
540+
await kernel.bootstrap();
541+
542+
// Assert - syncSchema should use tableName (single underscore) not FQN (double underscore)
543+
const syncedNames = synced.map((s) => s.object).sort();
544+
expect(syncedNames).toContain('sys_user');
545+
expect(syncedNames).toContain('sys_session');
546+
// Should NOT contain double-underscore FQN
547+
expect(syncedNames).not.toContain('sys__user');
548+
expect(syncedNames).not.toContain('sys__session');
549+
});
471550
});
472551
});

packages/objectql/src/plugin.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,12 +274,18 @@ export class ObjectQLPlugin implements Plugin {
274274
continue;
275275
}
276276

277+
// Use the physical table name (e.g., 'sys_user') for DDL operations
278+
// instead of the FQN (e.g., 'sys__user'). ObjectSchema.create()
279+
// auto-derives tableName as {namespace}_{name}.
280+
const tableName = obj.tableName || obj.name;
281+
277282
try {
278-
await driver.syncSchema(obj.name, obj);
283+
await driver.syncSchema(tableName, obj);
279284
synced++;
280285
} catch (e: unknown) {
281286
ctx.logger.warn('Failed to sync schema for object', {
282287
object: obj.name,
288+
tableName,
283289
driver: driver.name,
284290
error: e instanceof Error ? e.message : String(e),
285291
});

packages/objectql/src/registry.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,31 @@ describe('SchemaRegistry', () => {
187187
expect(SchemaRegistry.getObject('task')).toBeDefined();
188188
});
189189

190+
it('should resolve by tableName (protocol name fallback)', () => {
191+
// Simulates ObjectSchema.create() which auto-derives tableName
192+
// as {namespace}_{name} (single underscore)
193+
const obj = { name: 'user', tableName: 'sys_user', namespace: 'sys', fields: {} };
194+
SchemaRegistry.registerObject(obj as any, 'com.objectstack.system', 'sys', 'own');
195+
196+
// FQN is 'sys__user' (double underscore)
197+
expect(SchemaRegistry.getObject('sys__user')).toBeDefined();
198+
199+
// Protocol name 'sys_user' (single underscore) should also resolve
200+
const resolved = SchemaRegistry.getObject('sys_user');
201+
expect(resolved).toBeDefined();
202+
expect(resolved?.name).toBe('sys__user');
203+
expect((resolved as any).tableName).toBe('sys_user');
204+
});
205+
206+
it('should resolve by tableName for any namespace', () => {
207+
const obj = { name: 'account', tableName: 'crm_account', namespace: 'crm', fields: {} };
208+
SchemaRegistry.registerObject(obj as any, 'com.crm', 'crm', 'own');
209+
210+
// FQN: 'crm__account', tableName: 'crm_account'
211+
expect(SchemaRegistry.getObject('crm__account')).toBeDefined();
212+
expect(SchemaRegistry.getObject('crm_account')).toBeDefined();
213+
});
214+
190215
it('should cache merged objects', () => {
191216
const obj = { name: 'cached', fields: {} };
192217
SchemaRegistry.registerObject(obj as any, 'com.test', 'test', 'own');

packages/objectql/src/registry.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,14 @@ export class SchemaRegistry {
314314
}
315315

316316
/**
317-
* Get object by name (FQN or short name with fallback scan).
318-
* For compatibility, tries exact match first, then scans for suffix match.
317+
* Get object by name (FQN, short name, or physical table name).
318+
*
319+
* Resolution order:
320+
* 1. Exact FQN match (e.g., 'crm__account')
321+
* 2. Short name fallback (e.g., 'account' → 'crm__account')
322+
* 3. Physical table name match (e.g., 'sys_user' → 'sys__user')
323+
* ObjectSchema.create() auto-derives tableName as {namespace}_{name},
324+
* which uses a single underscore — different from the FQN double underscore.
319325
*/
320326
static getObject(name: string): ServiceObject | undefined {
321327
// Direct FQN lookup
@@ -331,6 +337,15 @@ export class SchemaRegistry {
331337
}
332338
}
333339

340+
// Fallback: match by physical table name (e.g., 'sys_user' → FQN 'sys__user')
341+
// This bridges the gap between protocol names (SystemObjectName) and FQN.
342+
for (const fqn of this.objectContributors.keys()) {
343+
const resolved = this.resolveObject(fqn);
344+
if (resolved?.tableName === name) {
345+
return resolved;
346+
}
347+
}
348+
334349
return undefined;
335350
}
336351

packages/plugins/driver-sql/src/sql-driver-schema.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,23 @@ describe('SqlDriver Schema Sync (SQLite)', () => {
243243
expect(row.email).toBe('test@example.com');
244244
expect(row.office_loc).toEqual({ lat: 10, lng: 20 });
245245
});
246+
247+
it('should skip ensureDatabaseExists for SQLite (no-op)', async () => {
248+
// SQLite auto-creates database files, so ensureDatabaseExists should be a no-op
249+
// This test verifies that initObjects works normally for SQLite without errors
250+
const objects = [
251+
{
252+
name: 'db_check_test',
253+
fields: {
254+
value: { type: 'string' },
255+
},
256+
},
257+
];
258+
259+
// Should not throw — SQLite skips ensureDatabaseExists
260+
await driver.initObjects(objects);
261+
262+
const exists = await knexInstance.schema.hasTable('db_check_test');
263+
expect(exists).toBe(true);
264+
});
246265
});

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -995,12 +995,22 @@ export class SqlDriver implements IDataDriver {
995995
// ── Database helpers ────────────────────────────────────────────────────────
996996

997997
protected async ensureDatabaseExists() {
998-
if (!this.isPostgres) return;
998+
// SQLite auto-creates database files — no need to check
999+
if (this.isSqlite) return;
1000+
1001+
// Only PostgreSQL and MySQL support programmatic database creation
1002+
if (!this.isPostgres && !this.isMysql) return;
9991003

10001004
try {
10011005
await this.knex.raw('SELECT 1');
10021006
} catch (e: any) {
1003-
if (e.code === '3D000') {
1007+
// PostgreSQL: '3D000' = database does not exist
1008+
// MySQL: 'ER_BAD_DB_ERROR' (errno 1049) = unknown database
1009+
if (
1010+
e.code === '3D000' ||
1011+
e.code === 'ER_BAD_DB_ERROR' ||
1012+
e.errno === 1049
1013+
) {
10041014
await this.createDatabase();
10051015
} else {
10061016
throw e;
@@ -1014,19 +1024,40 @@ export class SqlDriver implements IDataDriver {
10141024
let dbName = '';
10151025
const adminConfig = { ...config };
10161026

1017-
if (typeof connection === 'string') {
1018-
const url = new URL(connection);
1019-
dbName = url.pathname.slice(1);
1020-
url.pathname = '/postgres';
1021-
adminConfig.connection = url.toString();
1027+
if (this.isPostgres) {
1028+
// PostgreSQL: connect to the 'postgres' maintenance database
1029+
if (typeof connection === 'string') {
1030+
const url = new URL(connection);
1031+
dbName = url.pathname.slice(1);
1032+
url.pathname = '/postgres';
1033+
adminConfig.connection = url.toString();
1034+
} else {
1035+
dbName = connection.database;
1036+
adminConfig.connection = { ...connection, database: 'postgres' };
1037+
}
1038+
} else if (this.isMysql) {
1039+
// MySQL: connect without specifying a database
1040+
if (typeof connection === 'string') {
1041+
const url = new URL(connection);
1042+
dbName = url.pathname.slice(1);
1043+
url.pathname = '/';
1044+
adminConfig.connection = url.toString();
1045+
} else {
1046+
dbName = connection.database;
1047+
const { database: _db, ...rest } = connection;
1048+
adminConfig.connection = rest;
1049+
}
10221050
} else {
1023-
dbName = connection.database;
1024-
adminConfig.connection = { ...connection, database: 'postgres' };
1051+
return; // Unsupported dialect for auto-creation
10251052
}
10261053

10271054
const adminKnex = knex(adminConfig);
10281055
try {
1029-
await adminKnex.raw(`CREATE DATABASE "${dbName}"`);
1056+
if (this.isPostgres) {
1057+
await adminKnex.raw(`CREATE DATABASE "${dbName}"`);
1058+
} else if (this.isMysql) {
1059+
await adminKnex.raw(`CREATE DATABASE IF NOT EXISTS \`${dbName}\``);
1060+
}
10301061
} finally {
10311062
await adminKnex.destroy();
10321063
}

0 commit comments

Comments
 (0)