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
49 changes: 37 additions & 12 deletions packages/drivers/mongo/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type DriverInterface = Data.DriverInterface;
*/

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

/**
* Change stream event handler callback
Expand Down Expand Up @@ -410,9 +410,20 @@ export class MongoDriver implements Driver {
mongoDoc._id = new ObjectId().toHexString();
}

// Pass session for transactional operations
const mongoOptions = options?.session ? { session: options.session } : {};
const result = await collection.insertOne(mongoDoc, mongoOptions);
// Add timestamps if not already present
const now = new Date().toISOString();
if (!mongoDoc.created_at) {
mongoDoc.created_at = now;
}
if (!mongoDoc.updated_at) {
mongoDoc.updated_at = now;
}

// Pass session for transactional operations only if it exists
const result = options?.session
? await collection.insertOne(mongoDoc, { session: options.session })
: await collection.insertOne(mongoDoc);

// Return API format document (convert _id to id)
return this.mapFromMongo({ ...mongoDoc, _id: result.insertedId });
}
Expand All @@ -422,23 +433,37 @@ export class MongoDriver implements Driver {

// Map API document (id) to MongoDB document (_id) for update data
// But we should not allow updating the _id field itself
const { id: _ignoredId, ...updateData } = data; // intentionally ignore id to prevent updating primary key
const { id: _ignoredId, created_at: _ignoredCreatedAt, ...updateData } = data; // intentionally ignore id and created_at to prevent updating them

// Add updated_at timestamp
updateData.updated_at = new Date().toISOString();

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

// Pass session for transactional operations
const mongoOptions = options?.session ? { session: options.session } : {};
const result = await collection.updateOne({ _id: this.normalizeId(id) }, update, mongoOptions);
return result.modifiedCount; // or return updated document?
// Use findOneAndUpdate to return the updated document
const mongoOptions: FindOneAndUpdateOptions = { returnDocument: 'after' };
if (options?.session) {
mongoOptions.session = options.session;
}

const result = await collection.findOneAndUpdate(
{ _id: this.normalizeId(id) },
update,
mongoOptions
);

// Return API format document (convert _id to id)
return this.mapFromMongo(result);
}

async delete(objectName: string, id: string | number, options?: any) {
const collection = await this.getCollection(objectName);
// Pass session for transactional operations
const mongoOptions = options?.session ? { session: options.session } : {};
const result = await collection.deleteOne({ _id: this.normalizeId(id) }, mongoOptions);
// Pass session for transactional operations only if it exists
const result = options?.session
? await collection.deleteOne({ _id: this.normalizeId(id) }, { session: options.session })
: await collection.deleteOne({ _id: this.normalizeId(id) });
return result.deletedCount;
}

Expand Down
22 changes: 10 additions & 12 deletions packages/drivers/mongo/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const mockCollection = {
insertedCount: 2
}),
updateOne: jest.fn().mockResolvedValue({ modifiedCount: 1 }),
findOneAndUpdate: jest.fn().mockResolvedValue({ _id: '123', name: 'Updated' }),
deleteOne: jest.fn().mockResolvedValue({ deletedCount: 1 }),
countDocuments: jest.fn().mockResolvedValue(10)
};
Expand Down Expand Up @@ -437,20 +438,17 @@ describe('MongoDriver', () => {
data: { name: 'Updated User' }
};

mockCollection.updateOne.mockResolvedValue({
modifiedCount: 1,
acknowledged: true
} as any);
mockCollection.findOne.mockResolvedValue({
mockCollection.findOneAndUpdate.mockResolvedValue({
_id: '123',
name: 'Updated User'
name: 'Updated User',
updated_at: new Date().toISOString()
});

const result = await driver.executeCommand(command);

expect(result.success).toBe(true);
expect(result.affected).toBe(1);
expect(mockCollection.updateOne).toHaveBeenCalled();
expect(mockCollection.findOneAndUpdate).toHaveBeenCalled();
});

it('should execute delete command', async () => {
Expand Down Expand Up @@ -504,11 +502,11 @@ describe('MongoDriver', () => {
]
};

mockCollection.updateOne.mockResolvedValue({
modifiedCount: 1,
acknowledged: true
} as any);
mockCollection.findOne.mockResolvedValue({ _id: '1', name: 'Updated 1' });
mockCollection.findOneAndUpdate.mockResolvedValue({
_id: '1',
name: 'Updated 1',
updated_at: new Date().toISOString()
});

const result = await driver.executeCommand(command);

Expand Down
8 changes: 7 additions & 1 deletion packages/drivers/redis/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,13 @@ export class RedisDriver implements Driver {
};

const key = this.generateRedisKey(objectName, id);
await this.client.set(key, JSON.stringify(doc));

// Use SET with NX option to prevent overwriting existing records
const result = await this.client.set(key, JSON.stringify(doc), { NX: true });

if (!result) {
throw new Error(`Record with ID ${id} already exists in ${objectName}`);
}

return doc;
}
Expand Down
35 changes: 20 additions & 15 deletions packages/drivers/sql/test/tck.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('SqlDriver TCK Compliance', () => {
() => {
// Use SQLite in-memory database for testing
driver = new SqlDriver({
client: 'better-sqlite3',
client: 'sqlite3',
connection: {
filename: ':memory:'
},
Expand All @@ -38,24 +38,29 @@ describe('SqlDriver TCK Compliance', () => {
timeout: 30000,
hooks: {
beforeEach: async () => {
// Clear all tables
if (driver) {
try {
const tables = await driver['knex'].raw(`
SELECT name FROM sqlite_master
WHERE type='table' AND name NOT LIKE 'sqlite_%'
`);

for (const table of tables) {
await driver['knex'].raw(`DROP TABLE IF EXISTS "${table.name}"`);
// Initialize the tck_test table
await driver.init([
{
name: 'tck_test',
fields: {
name: { type: 'string' },
email: { type: 'string' },
age: { type: 'number' },
role: { type: 'string' },
active: { type: 'boolean' },
status: { type: 'string' },
department: { type: 'string' },
description: { type: 'string' },
optionalField: { type: 'string' }
}
} catch (error) {
// Ignore errors during cleanup
}
}
]);
},
afterEach: async () => {
// Cleanup handled in beforeEach
// Close database connection
if (driver && driver['knex']) {
await driver['knex'].destroy();
}
Comment on lines +60 to +63
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

Accessing private member knex using bracket notation is fragile and bypasses type safety. Consider exposing a close() or destroy() method on the driver interface to properly handle cleanup.

Suggested change
// Close database connection
if (driver && driver['knex']) {
await driver['knex'].destroy();
}
// Close database connection using driver's public API if available
if (!driver) {
return;
}
const closable = driver as unknown as {
destroy?: () => Promise<void> | void;
close?: () => Promise<void> | void;
};
if (typeof closable.destroy === 'function') {
await closable.destroy();
} else if (typeof closable.close === 'function') {
await closable.close();
}

Copilot uses AI. Check for mistakes.
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/tools/driver-tck/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function runDriverTCK(

beforeEach(async () => {
driver = createDriver();
if (driver.clear) {
if (driver && driver.clear) {
await driver.clear();
}
if (hooks.beforeEach) {
Expand All @@ -68,7 +68,7 @@ export function runDriverTCK(
if (hooks.afterEach) {
await hooks.afterEach();
}
if (driver.clear) {
if (driver && driver.clear) {
await driver.clear();
}
}, timeout);
Expand Down