Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- **Login API error — database tables not created** — Fixed a critical naming mismatch between
the FQN (Fully Qualified Name) used by SchemaRegistry (e.g., `sys__user` with double underscore)
and the physical table name derived by `ObjectSchema.create()` (e.g., `sys_user` with single
underscore). `syncRegisteredSchemas()` now uses the `tableName` property from object definitions
for DDL operations, ensuring tables are created with the correct physical name that matches
what the auth adapter and `SystemObjectName` constants expect.
- **`SchemaRegistry.getObject()` — protocol name resolution** — Added a third fallback that
matches objects by their `tableName` property (e.g., `getObject('sys_user')` now correctly
finds the object registered as FQN `sys__user`). This bridges protocol-layer names
(`SystemObjectName.USER = 'sys_user'`) with the registry's FQN naming convention.
- **`ObjectQL.resolveObjectName()` — physical table name** — Now returns `schema.tableName`
(the physical table/collection name) instead of `schema.name` (the FQN) when available,
ensuring driver SQL queries target the correct table.
- **`SqlDriver.ensureDatabaseExists()` — multi-driver support** — Extended database
auto-creation to support MySQL (error code `ER_BAD_DB_ERROR` / errno 1049) alongside
PostgreSQL (error code `3D000`). SQLite is explicitly skipped (auto-creates files).
- **`SqlDriver.createDatabase()` — MySQL support** — Added MySQL-specific logic that
connects without a database specified and uses `CREATE DATABASE IF NOT EXISTS`.

### Added
- **`@objectstack/driver-turso` plugin** — Migrated and standardized the Turso/libSQL driver from
`@objectql/driver-turso` into `packages/plugins/driver-turso/`. The driver **extends** `SqlDriver`
Expand Down
5 changes: 4 additions & 1 deletion packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,10 @@ export class ObjectQL implements IDataEngine {
private resolveObjectName(name: string): string {
const schema = SchemaRegistry.getObject(name);
if (schema) {
return schema.name; // FQN from registry (e.g., 'todo__task')
// Prefer the physical table name (e.g., 'sys_user') over the FQN
// (e.g., 'sys__user'). ObjectSchema.create() auto-derives tableName
// as {namespace}_{name} which matches the storage convention.
return schema.tableName || schema.name;
}
Comment on lines 545 to 552
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing resolveObjectName() to return schema.tableName makes the engine’s canonical object identifier become the physical table name for objects that define tableName. This affects more than driver I/O: hook/middleware matching uses context.object string equality, and action keys are built from the object name, so existing code registering hooks/actions against the FQN (e.g. sys__user) will no longer match when operations run as sys_user. Consider keeping FQN as the canonical engine object identifier (for hooks/actions/registry) and passing tableName only at the driver boundary (or carrying both values in OperationContext) to avoid a breaking semantic change.

Copilot uses AI. Check for mistakes.
return name; // Ad-hoc object, keep as-is
}
Expand Down
79 changes: 79 additions & 0 deletions packages/objectql/src/plugin.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,5 +468,84 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => {
// Act & Assert - should not throw
await expect(kernel.bootstrap()).resolves.not.toThrow();
});

it('should use tableName for syncSchema when objects have auto-derived tableName', async () => {
// Arrange - driver that tracks syncSchema calls
const synced: Array<{ object: string; schema: any }> = [];
const mockDriver = {
name: 'table-name-driver',
version: '1.0.0',
connect: async () => {},
disconnect: async () => {},
find: async () => [],
findOne: async () => null,
create: async (_o: string, d: any) => d,
update: async (_o: string, _i: any, d: any) => d,
delete: async () => true,
syncSchema: async (object: string, schema: any) => {
synced.push({ object, schema });
},
};

await kernel.use({
name: 'mock-driver-plugin',
type: 'driver',
version: '1.0.0',
init: async (ctx) => {
ctx.registerService('driver.table-name', mockDriver);
},
});

// Objects with tableName (simulating ObjectSchema.create() output)
const appManifest = {
id: 'com.test.system',
name: 'system',
namespace: 'sys',
version: '1.0.0',
objects: [
{
name: 'user',
label: 'User',
namespace: 'sys',
tableName: 'sys_user',
fields: {
email: { name: 'email', label: 'Email', type: 'text' },
},
},
{
name: 'session',
label: 'Session',
namespace: 'sys',
tableName: 'sys_session',
fields: {
token: { name: 'token', label: 'Token', type: 'text' },
},
},
],
};

await kernel.use({
name: 'mock-app-plugin',
type: 'app',
version: '1.0.0',
init: async (ctx) => {
ctx.registerService('app.system', appManifest);
},
});

const plugin = new ObjectQLPlugin();
await kernel.use(plugin);

// Act
await kernel.bootstrap();

// Assert - syncSchema should use tableName (single underscore) not FQN (double underscore)
const syncedNames = synced.map((s) => s.object).sort();
expect(syncedNames).toContain('sys_user');
expect(syncedNames).toContain('sys_session');
// Should NOT contain double-underscore FQN
expect(syncedNames).not.toContain('sys__user');
expect(syncedNames).not.toContain('sys__session');
});
});
});
8 changes: 7 additions & 1 deletion packages/objectql/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,18 @@ export class ObjectQLPlugin implements Plugin {
continue;
}

// Use the physical table name (e.g., 'sys_user') for DDL operations
// instead of the FQN (e.g., 'sys__user'). ObjectSchema.create()
// auto-derives tableName as {namespace}_{name}.
const tableName = obj.tableName || obj.name;

try {
await driver.syncSchema(obj.name, obj);
await driver.syncSchema(tableName, obj);
synced++;
Comment on lines +277 to 284
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syncRegisteredSchemas() now passes tableName as the first argument to driver.syncSchema(), but SqlDriver.syncSchema() currently ignores that argument and uses schema.name when calling initObjects(). Since SchemaRegistry.registerObject() overwrites schema.name with the FQN (e.g. sys__user), SqlDriver will still create/alter the FQN table instead of the physical tableName (e.g. sys_user). To make this change effective, either update SqlDriver.syncSchema to use the object parameter as the physical table name, or pass a schema copy with name rewritten to tableName for drivers that derive DDL names from schema.name.

Copilot uses AI. Check for mistakes.
} catch (e: unknown) {
ctx.logger.warn('Failed to sync schema for object', {
object: obj.name,
tableName,
driver: driver.name,
error: e instanceof Error ? e.message : String(e),
});
Expand Down
25 changes: 25 additions & 0 deletions packages/objectql/src/registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,31 @@ describe('SchemaRegistry', () => {
expect(SchemaRegistry.getObject('task')).toBeDefined();
});

it('should resolve by tableName (protocol name fallback)', () => {
// Simulates ObjectSchema.create() which auto-derives tableName
// as {namespace}_{name} (single underscore)
const obj = { name: 'user', tableName: 'sys_user', namespace: 'sys', fields: {} };
SchemaRegistry.registerObject(obj as any, 'com.objectstack.system', 'sys', 'own');

// FQN is 'sys__user' (double underscore)
expect(SchemaRegistry.getObject('sys__user')).toBeDefined();

// Protocol name 'sys_user' (single underscore) should also resolve
const resolved = SchemaRegistry.getObject('sys_user');
expect(resolved).toBeDefined();
expect(resolved?.name).toBe('sys__user');
expect((resolved as any).tableName).toBe('sys_user');
});

it('should resolve by tableName for any namespace', () => {
const obj = { name: 'account', tableName: 'crm_account', namespace: 'crm', fields: {} };
SchemaRegistry.registerObject(obj as any, 'com.crm', 'crm', 'own');

// FQN: 'crm__account', tableName: 'crm_account'
expect(SchemaRegistry.getObject('crm__account')).toBeDefined();
expect(SchemaRegistry.getObject('crm_account')).toBeDefined();
});

it('should cache merged objects', () => {
const obj = { name: 'cached', fields: {} };
SchemaRegistry.registerObject(obj as any, 'com.test', 'test', 'own');
Expand Down
19 changes: 17 additions & 2 deletions packages/objectql/src/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,14 @@ export class SchemaRegistry {
}

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

// Fallback: match by physical table name (e.g., 'sys_user' → FQN 'sys__user')
// This bridges the gap between protocol names (SystemObjectName) and FQN.
for (const fqn of this.objectContributors.keys()) {
const resolved = this.resolveObject(fqn);
if (resolved?.tableName === name) {
return resolved;
Comment on lines +342 to +345
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SchemaRegistry.getObject() now adds a second full scan over objectContributors and calls resolveObject() for each entry to match by tableName. Since getObject() is used on every engine operation (resolve + driver selection), resolving by physical table name can become O(n) per call and scale poorly as the number of objects grows. Consider maintaining an index/map from tableName -> fqn (updated on register/unregister/reset) so tableName lookups are O(1) and don’t require merging every object during resolution.

Suggested change
for (const fqn of this.objectContributors.keys()) {
const resolved = this.resolveObject(fqn);
if (resolved?.tableName === name) {
return resolved;
//
// To avoid repeatedly merging objects on every iteration, we derive the
// physical table name directly from the FQN using the convention:
// tableName = `${namespace}_${name}`
// and only resolve the object once we find a match.
for (const fqn of this.objectContributors.keys()) {
const { namespace, shortName } = parseFQN(fqn);
const tableName = `${namespace}_${shortName}`;
if (tableName === name) {
return this.resolveObject(fqn);

Copilot uses AI. Check for mistakes.
}
}

return undefined;
}

Expand Down
19 changes: 19 additions & 0 deletions packages/plugins/driver-sql/src/sql-driver-schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,23 @@ describe('SqlDriver Schema Sync (SQLite)', () => {
expect(row.email).toBe('test@example.com');
expect(row.office_loc).toEqual({ lat: 10, lng: 20 });
});

it('should skip ensureDatabaseExists for SQLite (no-op)', async () => {
// SQLite auto-creates database files, so ensureDatabaseExists should be a no-op
// This test verifies that initObjects works normally for SQLite without errors
const objects = [
{
name: 'db_check_test',
fields: {
value: { type: 'string' },
},
},
];

// Should not throw — SQLite skips ensureDatabaseExists
await driver.initObjects(objects);

const exists = await knexInstance.schema.hasTable('db_check_test');
expect(exists).toBe(true);
});
});
51 changes: 41 additions & 10 deletions packages/plugins/driver-sql/src/sql-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -995,12 +995,22 @@ export class SqlDriver implements IDataDriver {
// ── Database helpers ────────────────────────────────────────────────────────

protected async ensureDatabaseExists() {
if (!this.isPostgres) return;
// SQLite auto-creates database files — no need to check
if (this.isSqlite) return;

// Only PostgreSQL and MySQL support programmatic database creation
if (!this.isPostgres && !this.isMysql) return;

try {
await this.knex.raw('SELECT 1');
} catch (e: any) {
if (e.code === '3D000') {
// PostgreSQL: '3D000' = database does not exist
// MySQL: 'ER_BAD_DB_ERROR' (errno 1049) = unknown database
if (
e.code === '3D000' ||
e.code === 'ER_BAD_DB_ERROR' ||
e.errno === 1049
) {
await this.createDatabase();
} else {
Comment on lines 997 to 1015
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new MySQL handling in ensureDatabaseExists() (ER_BAD_DB_ERROR / errno 1049) and the MySQL branch in createDatabase() are not covered by tests. The added SQLite test still passes even with the old behavior (SQLite already skipped the PostgreSQL-only check), so it doesn’t validate the new MySQL upgrade fix. Consider adding unit-level tests that stub knex.raw() to throw MySQL/PG "unknown database" errors and assert createDatabase() is invoked (and not invoked for other error codes).

Copilot uses AI. Check for mistakes.
throw e;
Expand All @@ -1014,19 +1024,40 @@ export class SqlDriver implements IDataDriver {
let dbName = '';
const adminConfig = { ...config };

if (typeof connection === 'string') {
const url = new URL(connection);
dbName = url.pathname.slice(1);
url.pathname = '/postgres';
adminConfig.connection = url.toString();
if (this.isPostgres) {
// PostgreSQL: connect to the 'postgres' maintenance database
if (typeof connection === 'string') {
const url = new URL(connection);
dbName = url.pathname.slice(1);
url.pathname = '/postgres';
adminConfig.connection = url.toString();
} else {
dbName = connection.database;
adminConfig.connection = { ...connection, database: 'postgres' };
}
} else if (this.isMysql) {
// MySQL: connect without specifying a database
if (typeof connection === 'string') {
const url = new URL(connection);
dbName = url.pathname.slice(1);
url.pathname = '/';
adminConfig.connection = url.toString();
} else {
dbName = connection.database;
const { database: _db, ...rest } = connection;
adminConfig.connection = rest;
}
} else {
dbName = connection.database;
adminConfig.connection = { ...connection, database: 'postgres' };
return; // Unsupported dialect for auto-creation
}

const adminKnex = knex(adminConfig);
try {
await adminKnex.raw(`CREATE DATABASE "${dbName}"`);
if (this.isPostgres) {
await adminKnex.raw(`CREATE DATABASE "${dbName}"`);
} else if (this.isMysql) {
await adminKnex.raw(`CREATE DATABASE IF NOT EXISTS \`${dbName}\``);
}
Comment on lines 1054 to +1060
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createDatabase() interpolates dbName directly into raw SQL (CREATE DATABASE "${dbName}" / ``CREATE DATABASE IF NOT EXISTS `${dbName}```), but dbName comes from the connection string/object and is not validated/escaped. This can fail for empty/undefined names and also risks SQL injection if the connection config can be influenced. Prefer Knex identifier bindings (e.g., `??`) or a strict identifier allowlist validation before executing the raw statement, and explicitly error if `dbName` is missing.

Copilot uses AI. Check for mistakes.
} finally {
await adminKnex.destroy();
}
Expand Down
Loading