Skip to content

Commit 2dfb0fc

Browse files
authored
Merge pull request #307 from objectstack-ai/copilot/fix-action-run-issue-again
2 parents 7375249 + f483701 commit 2dfb0fc

5 files changed

Lines changed: 76 additions & 42 deletions

File tree

packages/drivers/mongo/src/index.ts

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ type DriverInterface = Data.DriverInterface;
1111
*/
1212

1313
import { Driver } from '@objectql/types';
14-
import { MongoClient, Db, Filter, ObjectId, FindOptions, ChangeStream, ChangeStreamDocument } from 'mongodb';
14+
import { MongoClient, Db, Filter, ObjectId, FindOptions, FindOneAndUpdateOptions, ChangeStream, ChangeStreamDocument } from 'mongodb';
1515

1616
/**
1717
* Change stream event handler callback
@@ -410,9 +410,20 @@ export class MongoDriver implements Driver {
410410
mongoDoc._id = new ObjectId().toHexString();
411411
}
412412

413-
// Pass session for transactional operations
414-
const mongoOptions = options?.session ? { session: options.session } : {};
415-
const result = await collection.insertOne(mongoDoc, mongoOptions);
413+
// Add timestamps if not already present
414+
const now = new Date().toISOString();
415+
if (!mongoDoc.created_at) {
416+
mongoDoc.created_at = now;
417+
}
418+
if (!mongoDoc.updated_at) {
419+
mongoDoc.updated_at = now;
420+
}
421+
422+
// Pass session for transactional operations only if it exists
423+
const result = options?.session
424+
? await collection.insertOne(mongoDoc, { session: options.session })
425+
: await collection.insertOne(mongoDoc);
426+
416427
// Return API format document (convert _id to id)
417428
return this.mapFromMongo({ ...mongoDoc, _id: result.insertedId });
418429
}
@@ -422,23 +433,37 @@ export class MongoDriver implements Driver {
422433

423434
// Map API document (id) to MongoDB document (_id) for update data
424435
// But we should not allow updating the _id field itself
425-
const { id: _ignoredId, ...updateData } = data; // intentionally ignore id to prevent updating primary key
436+
const { id: _ignoredId, created_at: _ignoredCreatedAt, ...updateData } = data; // intentionally ignore id and created_at to prevent updating them
437+
438+
// Add updated_at timestamp
439+
updateData.updated_at = new Date().toISOString();
426440

427441
// Handle atomic operators if present
428442
const isAtomic = Object.keys(updateData).some(k => k.startsWith('$'));
429443
const update = isAtomic ? updateData : { $set: updateData };
430444

431-
// Pass session for transactional operations
432-
const mongoOptions = options?.session ? { session: options.session } : {};
433-
const result = await collection.updateOne({ _id: this.normalizeId(id) }, update, mongoOptions);
434-
return result.modifiedCount; // or return updated document?
445+
// Use findOneAndUpdate to return the updated document
446+
const mongoOptions: FindOneAndUpdateOptions = { returnDocument: 'after' };
447+
if (options?.session) {
448+
mongoOptions.session = options.session;
449+
}
450+
451+
const result = await collection.findOneAndUpdate(
452+
{ _id: this.normalizeId(id) },
453+
update,
454+
mongoOptions
455+
);
456+
457+
// Return API format document (convert _id to id)
458+
return this.mapFromMongo(result);
435459
}
436460

437461
async delete(objectName: string, id: string | number, options?: any) {
438462
const collection = await this.getCollection(objectName);
439-
// Pass session for transactional operations
440-
const mongoOptions = options?.session ? { session: options.session } : {};
441-
const result = await collection.deleteOne({ _id: this.normalizeId(id) }, mongoOptions);
463+
// Pass session for transactional operations only if it exists
464+
const result = options?.session
465+
? await collection.deleteOne({ _id: this.normalizeId(id) }, { session: options.session })
466+
: await collection.deleteOne({ _id: this.normalizeId(id) });
442467
return result.deletedCount;
443468
}
444469

packages/drivers/mongo/test/index.test.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const mockCollection = {
2222
insertedCount: 2
2323
}),
2424
updateOne: jest.fn().mockResolvedValue({ modifiedCount: 1 }),
25+
findOneAndUpdate: jest.fn().mockResolvedValue({ _id: '123', name: 'Updated' }),
2526
deleteOne: jest.fn().mockResolvedValue({ deletedCount: 1 }),
2627
countDocuments: jest.fn().mockResolvedValue(10)
2728
};
@@ -437,20 +438,17 @@ describe('MongoDriver', () => {
437438
data: { name: 'Updated User' }
438439
};
439440

440-
mockCollection.updateOne.mockResolvedValue({
441-
modifiedCount: 1,
442-
acknowledged: true
443-
} as any);
444-
mockCollection.findOne.mockResolvedValue({
441+
mockCollection.findOneAndUpdate.mockResolvedValue({
445442
_id: '123',
446-
name: 'Updated User'
443+
name: 'Updated User',
444+
updated_at: new Date().toISOString()
447445
});
448446

449447
const result = await driver.executeCommand(command);
450448

451449
expect(result.success).toBe(true);
452450
expect(result.affected).toBe(1);
453-
expect(mockCollection.updateOne).toHaveBeenCalled();
451+
expect(mockCollection.findOneAndUpdate).toHaveBeenCalled();
454452
});
455453

456454
it('should execute delete command', async () => {
@@ -504,11 +502,11 @@ describe('MongoDriver', () => {
504502
]
505503
};
506504

507-
mockCollection.updateOne.mockResolvedValue({
508-
modifiedCount: 1,
509-
acknowledged: true
510-
} as any);
511-
mockCollection.findOne.mockResolvedValue({ _id: '1', name: 'Updated 1' });
505+
mockCollection.findOneAndUpdate.mockResolvedValue({
506+
_id: '1',
507+
name: 'Updated 1',
508+
updated_at: new Date().toISOString()
509+
});
512510

513511
const result = await driver.executeCommand(command);
514512

packages/drivers/redis/src/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,13 @@ export class RedisDriver implements Driver {
394394
};
395395

396396
const key = this.generateRedisKey(objectName, id);
397-
await this.client.set(key, JSON.stringify(doc));
397+
398+
// Use SET with NX option to prevent overwriting existing records
399+
const result = await this.client.set(key, JSON.stringify(doc), { NX: true });
400+
401+
if (!result) {
402+
throw new Error(`Record with ID ${id} already exists in ${objectName}`);
403+
}
398404

399405
return doc;
400406
}

packages/drivers/sql/test/tck.test.ts

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ describe('SqlDriver TCK Compliance', () => {
2222
() => {
2323
// Use SQLite in-memory database for testing
2424
driver = new SqlDriver({
25-
client: 'better-sqlite3',
25+
client: 'sqlite3',
2626
connection: {
2727
filename: ':memory:'
2828
},
@@ -38,24 +38,29 @@ describe('SqlDriver TCK Compliance', () => {
3838
timeout: 30000,
3939
hooks: {
4040
beforeEach: async () => {
41-
// Clear all tables
42-
if (driver) {
43-
try {
44-
const tables = await driver['knex'].raw(`
45-
SELECT name FROM sqlite_master
46-
WHERE type='table' AND name NOT LIKE 'sqlite_%'
47-
`);
48-
49-
for (const table of tables) {
50-
await driver['knex'].raw(`DROP TABLE IF EXISTS "${table.name}"`);
41+
// Initialize the tck_test table
42+
await driver.init([
43+
{
44+
name: 'tck_test',
45+
fields: {
46+
name: { type: 'string' },
47+
email: { type: 'string' },
48+
age: { type: 'number' },
49+
role: { type: 'string' },
50+
active: { type: 'boolean' },
51+
status: { type: 'string' },
52+
department: { type: 'string' },
53+
description: { type: 'string' },
54+
optionalField: { type: 'string' }
5155
}
52-
} catch (error) {
53-
// Ignore errors during cleanup
5456
}
55-
}
57+
]);
5658
},
5759
afterEach: async () => {
58-
// Cleanup handled in beforeEach
60+
// Close database connection
61+
if (driver && driver['knex']) {
62+
await driver['knex'].destroy();
63+
}
5964
}
6065
}
6166
}

packages/tools/driver-tck/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export function runDriverTCK(
5656

5757
beforeEach(async () => {
5858
driver = createDriver();
59-
if (driver.clear) {
59+
if (driver && driver.clear) {
6060
await driver.clear();
6161
}
6262
if (hooks.beforeEach) {
@@ -68,7 +68,7 @@ export function runDriverTCK(
6868
if (hooks.afterEach) {
6969
await hooks.afterEach();
7070
}
71-
if (driver.clear) {
71+
if (driver && driver.clear) {
7272
await driver.clear();
7373
}
7474
}, timeout);

0 commit comments

Comments
 (0)