Skip to content

Commit e954fa4

Browse files
Copilothotlong
andcommitted
fix: resolve CI build errors and address all PR review comments
- Fix DTS build: remove legacy tuple indexing in orderBy (item[0]/item[1]) - Fix DTS build: widen SqlDriver name/version types to string for subclass override - Fix test: update orderBy test to use standard {field, order} format - Move better-sqlite3 from devDependencies to dependencies (runtime dep) - Throw error for unsupported remote-only URLs without syncUrl - Clarify TursoDriverConfig docs: encryptionKey/concurrency/timeout need syncUrl - Add in-flight promise dedup in multi-tenant router (prevents concurrent duplicates) - Interpolate {tenant} in syncUrl and other string config fields - Use os.tmpdir() + random suffix in tests (cross-platform, no collisions) - Update README: clarify connection modes, syncUrl interpolation, concurrency safety Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/e1258eb3-0b40-4278-af78-15e3ede22d41
1 parent 67cb98b commit e954fa4

9 files changed

Lines changed: 205 additions & 80 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ describe('SqlDriver (QueryAST Format)', () => {
144144
const results = await driver.find('products', {
145145
fields: ['name'],
146146
limit: 2,
147-
orderBy: [['price', 'asc']],
147+
orderBy: [{ field: 'price', order: 'asc' }],
148148
} as any);
149149

150150
expect(results.length).toBe(2);

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ export type SqlDriverConfig = Knex.Config;
6565
*/
6666
export class SqlDriver implements IDataDriver {
6767
// IDataDriver metadata
68-
public readonly name = 'com.objectstack.driver.sql';
69-
public readonly version = '1.0.0';
68+
public readonly name: string = 'com.objectstack.driver.sql';
69+
public readonly version: string = '1.0.0';
7070
public readonly supports = {
7171
// Basic CRUD Operations
7272
create: true,
@@ -185,10 +185,8 @@ export class SqlDriver implements IDataDriver {
185185
// ORDER BY
186186
if (query.orderBy && Array.isArray(query.orderBy)) {
187187
for (const item of query.orderBy) {
188-
const field = item.field || item[0];
189-
const dir = item.order || item[1] || 'asc';
190-
if (field) {
191-
builder.orderBy(this.mapSortField(field), dir);
188+
if (item.field) {
189+
builder.orderBy(this.mapSortField(item.field), item.order || 'asc');
192190
}
193191
}
194192
}

packages/plugins/driver-turso/README.md

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ await driver.connect();
6969
await driver.sync();
7070
```
7171

72+
> **Note:** Remote-only URLs (`url: 'libsql://...'`) without `syncUrl` are
73+
> not supported and will throw. Always use a local `file:` or `:memory:` URL
74+
> as the primary store. For remote persistence, use embedded replica mode with `syncUrl`.
75+
7276
## Multi-Tenant Routing
7377

7478
Database-per-tenant architecture with automatic driver caching:
@@ -92,7 +96,9 @@ const users = await driver.find('users', { where: { active: true } });
9296
await router.destroyAll();
9397
```
9498

95-
### Multi-Tenant with Turso Cloud
99+
### Multi-Tenant with Embedded Replicas
100+
101+
Both `urlTemplate` and `driverConfigOverrides.syncUrl` support `{tenant}` placeholder interpolation:
96102

97103
```typescript
98104
const router = createMultiTenantRouter({
@@ -105,32 +111,46 @@ const router = createMultiTenantRouter({
105111
});
106112
```
107113

114+
### Concurrency Safety
115+
116+
Concurrent `getDriverForTenant()` calls for the same tenant are deduplicated — only one driver is created, and all callers share the same instance.
117+
108118
## Configuration
109119

110120
```typescript
111121
interface TursoDriverConfig {
112-
/** Database URL (file:, :memory:, libsql://, https://) */
122+
/** Database URL for the local store (file: or :memory:) */
113123
url: string;
114124

115-
/** JWT auth token for remote Turso database */
125+
/** JWT auth token for the remote Turso database (used with syncUrl) */
116126
authToken?: string;
117127

118-
/** AES-256 encryption key for local files */
128+
/**
129+
* AES-256 encryption key for local database file.
130+
* Only effective in embedded replica mode (requires syncUrl).
131+
*/
119132
encryptionKey?: string;
120133

121-
/** Maximum concurrent requests. Default: 20 */
134+
/**
135+
* Maximum concurrent requests to the remote database.
136+
* Only effective in embedded replica mode (requires syncUrl).
137+
* Default: 20
138+
*/
122139
concurrency?: number;
123140

124-
/** Remote sync URL for embedded replica mode */
141+
/** Remote sync URL for embedded replica mode (libsql:// or https://) */
125142
syncUrl?: string;
126143

127-
/** Sync configuration */
144+
/** Sync configuration (requires syncUrl) */
128145
sync?: {
129146
intervalSeconds?: number; // Default: 60
130147
onConnect?: boolean; // Default: true
131148
};
132149

133-
/** Operation timeout in milliseconds */
150+
/**
151+
* Operation timeout in milliseconds for remote operations.
152+
* Only effective in embedded replica mode (requires syncUrl).
153+
*/
134154
timeout?: number;
135155
}
136156
```
@@ -164,7 +184,7 @@ await kernel.enablePlugin(tursoPlugin, {
164184
## Testing
165185

166186
```bash
167-
pnpm test # Run all 53 tests
187+
pnpm test # Run all tests
168188
```
169189

170190
Tests run against in-memory SQLite (`:memory:`) — no external services required.

packages/plugins/driver-turso/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@
3333
"@objectstack/driver-sql": "workspace:*",
3434
"@objectstack/spec": "workspace:*",
3535
"@libsql/client": "^0.17.2",
36+
"better-sqlite3": "^11.9.1",
3637
"nanoid": "^3.3.11"
3738
},
3839
"devDependencies": {
3940
"@types/node": "^25.5.0",
40-
"better-sqlite3": "^11.9.1",
4141
"typescript": "^5.0.0",
4242
"vitest": "^4.1.0"
4343
}

packages/plugins/driver-turso/src/multi-tenant.test.ts

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ import { describe, it, expect, afterEach } from 'vitest';
44
import { createMultiTenantRouter, type MultiTenantRouter } from '../src/multi-tenant.js';
55
import { TursoDriver } from '../src/turso-driver.js';
66
import { SqlDriver } from '@objectstack/driver-sql';
7+
import { tmpdir } from 'node:os';
8+
import { join } from 'node:path';
9+
10+
/** Build a unique temp path template for test isolation. */
11+
function tmpTemplate(label: string): string {
12+
const rand = Math.random().toString(36).slice(2, 8);
13+
return `file:${join(tmpdir(), `test-turso-${label}-${rand}-{tenant}.db`)}`;
14+
}
715

816
describe('Multi-Tenant Router', () => {
917
let router: MultiTenantRouter | null = null;
@@ -29,7 +37,7 @@ describe('Multi-Tenant Router', () => {
2937

3038
it('should create a TursoDriver for a tenant', async () => {
3139
router = createMultiTenantRouter({
32-
urlTemplate: 'file:/tmp/test-turso-mt-{tenant}.db',
40+
urlTemplate: tmpTemplate('mt'),
3341
});
3442

3543
const driver = await router.getDriverForTenant('acme');
@@ -40,7 +48,7 @@ describe('Multi-Tenant Router', () => {
4048

4149
it('should cache drivers and return same instance', async () => {
4250
router = createMultiTenantRouter({
43-
urlTemplate: 'file:/tmp/test-turso-cache-{tenant}.db',
51+
urlTemplate: tmpTemplate('cache'),
4452
});
4553

4654
const driver1 = await router.getDriverForTenant('acme');
@@ -50,7 +58,7 @@ describe('Multi-Tenant Router', () => {
5058

5159
it('should create separate drivers per tenant', async () => {
5260
router = createMultiTenantRouter({
53-
urlTemplate: 'file:/tmp/test-turso-sep-{tenant}.db',
61+
urlTemplate: tmpTemplate('sep'),
5462
});
5563

5664
const driver1 = await router.getDriverForTenant('tenant-a');
@@ -63,15 +71,15 @@ describe('Multi-Tenant Router', () => {
6371

6472
it('should reject empty tenantId', async () => {
6573
router = createMultiTenantRouter({
66-
urlTemplate: 'file:/tmp/test-turso-val-{tenant}.db',
74+
urlTemplate: tmpTemplate('val'),
6775
});
6876

6977
await expect(router.getDriverForTenant('')).rejects.toThrow();
7078
});
7179

7280
it('should reject invalid tenantId with special characters', async () => {
7381
router = createMultiTenantRouter({
74-
urlTemplate: 'file:/tmp/test-turso-val2-{tenant}.db',
82+
urlTemplate: tmpTemplate('val2'),
7583
});
7684

7785
await expect(router.getDriverForTenant('a')).rejects.toThrow();
@@ -80,7 +88,7 @@ describe('Multi-Tenant Router', () => {
8088

8189
it('should accept valid tenantId', async () => {
8290
router = createMultiTenantRouter({
83-
urlTemplate: 'file:/tmp/test-turso-accept-{tenant}.db',
91+
urlTemplate: tmpTemplate('accept'),
8492
});
8593

8694
const driver = await router.getDriverForTenant('valid-tenant');
@@ -91,7 +99,7 @@ describe('Multi-Tenant Router', () => {
9199

92100
it('should report cache size', async () => {
93101
router = createMultiTenantRouter({
94-
urlTemplate: 'file:/tmp/test-turso-size-{tenant}.db',
102+
urlTemplate: tmpTemplate('size'),
95103
});
96104

97105
expect(router.getCacheSize()).toBe(0);
@@ -103,7 +111,7 @@ describe('Multi-Tenant Router', () => {
103111

104112
it('should invalidate cache for a tenant', async () => {
105113
router = createMultiTenantRouter({
106-
urlTemplate: 'file:/tmp/test-turso-inv-{tenant}.db',
114+
urlTemplate: tmpTemplate('inv'),
107115
});
108116

109117
await router.getDriverForTenant('to-invalidate');
@@ -115,7 +123,7 @@ describe('Multi-Tenant Router', () => {
115123

116124
it('should destroy all cached drivers', async () => {
117125
router = createMultiTenantRouter({
118-
urlTemplate: 'file:/tmp/test-turso-destroy-{tenant}.db',
126+
urlTemplate: tmpTemplate('destroy'),
119127
});
120128

121129
await router.getDriverForTenant('destroy-a');
@@ -132,7 +140,7 @@ describe('Multi-Tenant Router', () => {
132140
const created: string[] = [];
133141

134142
router = createMultiTenantRouter({
135-
urlTemplate: 'file:/tmp/test-turso-cb-{tenant}.db',
143+
urlTemplate: tmpTemplate('cb'),
136144
onTenantCreate: async (tenantId) => {
137145
created.push(tenantId);
138146
},
@@ -150,7 +158,7 @@ describe('Multi-Tenant Router', () => {
150158
const evicted: string[] = [];
151159

152160
router = createMultiTenantRouter({
153-
urlTemplate: 'file:/tmp/test-turso-evict-{tenant}.db',
161+
urlTemplate: tmpTemplate('evict'),
154162
onTenantEvict: async (tenantId) => {
155163
evicted.push(tenantId);
156164
},
@@ -163,11 +171,39 @@ describe('Multi-Tenant Router', () => {
163171
expect(evicted).toEqual(['evict-test']);
164172
});
165173

174+
// ── Concurrent Access Guard ────────────────────────────────────────────
175+
176+
it('should deduplicate concurrent getDriverForTenant calls', async () => {
177+
let createCount = 0;
178+
179+
router = createMultiTenantRouter({
180+
urlTemplate: tmpTemplate('conc'),
181+
onTenantCreate: async () => {
182+
createCount++;
183+
},
184+
});
185+
186+
// Fire multiple concurrent calls for the same tenant
187+
const [d1, d2, d3] = await Promise.all([
188+
router.getDriverForTenant('same-tenant'),
189+
router.getDriverForTenant('same-tenant'),
190+
router.getDriverForTenant('same-tenant'),
191+
]);
192+
193+
// All calls should return the same driver instance
194+
expect(d1).toBe(d2);
195+
expect(d2).toBe(d3);
196+
197+
// onTenantCreate should only be called once
198+
expect(createCount).toBe(1);
199+
expect(router.getCacheSize()).toBe(1);
200+
});
201+
166202
// ── CRUD through Multi-Tenant Driver ───────────────────────────────────
167203

168204
it('should perform CRUD operations through a tenant driver', async () => {
169205
router = createMultiTenantRouter({
170-
urlTemplate: 'file:/tmp/test-turso-crud-{tenant}.db',
206+
urlTemplate: tmpTemplate('crud'),
171207
});
172208

173209
const driver = await router.getDriverForTenant('crud-test');

0 commit comments

Comments
 (0)