Skip to content

Commit

Permalink
fix(NODE-5126): find operations fail when passed an ObjectId as filter (
Browse files Browse the repository at this point in the history
#3604)

Co-authored-by: Neal Beeken <neal.beeken@mongodb.com>
  • Loading branch information
JoCat and nbbeeken committed Mar 28, 2023
1 parent 6537e27 commit 2647b61
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/operations/find.ts
Expand Up @@ -104,7 +104,7 @@ export class FindOperation extends CommandOperation<Document> {
}

// special case passing in an ObjectId as a filter
this.filter = filter != null && filter._bsontype === 'ObjectID' ? { _id: filter } : filter;
this.filter = filter != null && filter._bsontype === 'ObjectId' ? { _id: filter } : filter;
}

override execute(
Expand Down
40 changes: 40 additions & 0 deletions test/integration/crud/find.test.js
Expand Up @@ -2577,4 +2577,44 @@ describe('Find', function () {
});
}
});

context('when passed an ObjectId instance as the filter', () => {
let client;
let findsStarted;

beforeEach(function () {
client = this.configuration.newClient({ monitorCommands: true });
findsStarted = [];
client.on('commandStarted', ev => {
if (ev.commandName === 'find') findsStarted.push(ev.command);
});
});

afterEach(async function () {
findsStarted = undefined;
await client.close();
});

context('find(oid)', () => {
it('wraps the objectId in a document with _id as the only key', async () => {
const collection = client.db('test').collection('test');
const oid = new ObjectId();
await collection.find(oid).toArray();
expect(findsStarted).to.have.lengthOf(1);
expect(findsStarted[0]).to.have.nested.property('filter._id', oid);
expect(findsStarted[0].filter).to.have.all.keys('_id');
});
});

context('findOne(oid)', () => {
it('wraps the objectId in a document with _id as the only key', async () => {
const collection = client.db('test').collection('test');
const oid = new ObjectId();
await collection.findOne(oid);
expect(findsStarted).to.have.lengthOf(1);
expect(findsStarted[0]).to.have.nested.property('filter._id', oid);
expect(findsStarted[0].filter).to.have.all.keys('_id');
});
});
});
});
55 changes: 55 additions & 0 deletions test/integration/crud/find_and_modify.test.js
Expand Up @@ -4,6 +4,8 @@ const { format: f } = require('util');
const { setupDatabase, assert: test } = require(`../shared`);
const { expect } = require('chai');

const { ObjectId, MongoServerError } = require('../../mongodb');

describe('Find and Modify', function () {
before(function () {
return setupDatabase(this.configuration);
Expand Down Expand Up @@ -318,4 +320,57 @@ describe('Find and Modify', function () {
expect(error.message).to.match(/must not contain atomic operators/);
await client.close();
});

context('when passed an ObjectId instance as the filter', () => {
let client;
let findAndModifyStarted;

beforeEach(function () {
client = this.configuration.newClient({ monitorCommands: true });
findAndModifyStarted = [];
client.on('commandStarted', ev => {
if (ev.commandName === 'findAndModify') findAndModifyStarted.push(ev.command);
});
});

afterEach(async function () {
findAndModifyStarted = undefined;
await client.close();
});

context('findOneAndDelete(oid)', () => {
it('sets the query to be the ObjectId instance', async () => {
const collection = client.db('test').collection('test');
const oid = new ObjectId();
const error = await collection.findOneAndDelete(oid).catch(error => error);
expect(error).to.be.instanceOf(MongoServerError);
expect(findAndModifyStarted).to.have.lengthOf(1);
expect(findAndModifyStarted[0]).to.have.property('query', oid);
});
});

context('findOneAndReplace(oid)', () => {
it('sets the query to be the ObjectId instance', async () => {
const collection = client.db('test').collection('test');
const oid = new ObjectId();
const error = await collection.findOneAndReplace(oid, {}).catch(error => error);
expect(error).to.be.instanceOf(MongoServerError);
expect(findAndModifyStarted).to.have.lengthOf(1);
expect(findAndModifyStarted[0]).to.have.property('query', oid);
});
});

context('findOneAndUpdate(oid)', () => {
it('sets the query to be the ObjectId instance', async () => {
const collection = client.db('test').collection('test');
const oid = new ObjectId();
const error = await collection
.findOneAndUpdate(oid, { $set: { a: 1 } })
.catch(error => error);
expect(error).to.be.instanceOf(MongoServerError);
expect(findAndModifyStarted).to.have.lengthOf(1);
expect(findAndModifyStarted[0]).to.have.property('query', oid);
});
});
});
});
34 changes: 33 additions & 1 deletion test/integration/gridfs/gridfs.test.ts
@@ -1,7 +1,13 @@
import { expect } from 'chai';
import { once } from 'events';

import { CommandStartedEvent, type Db, GridFSBucket, type MongoClient } from '../../mongodb';
import {
CommandStartedEvent,
type Db,
GridFSBucket,
type MongoClient,
ObjectId
} from '../../mongodb';
import { sleep } from '../../tools/utils';

describe('GridFS', () => {
Expand Down Expand Up @@ -115,5 +121,31 @@ describe('GridFS', () => {
const createIndexes = commandStartedEvents.filter(e => e.commandName === 'createIndexes');
expect(createIndexes).to.have.lengthOf(0);
});

context('find(oid)', () => {
let findsStarted;

beforeEach(function () {
findsStarted = [];
client.on('commandStarted', ev => {
if (ev.commandName === 'find') findsStarted.push(ev.command);
});
});

afterEach(async function () {
findsStarted = undefined;
await client.close();
});

context('when passed an ObjectId instance as the filter', () => {
it('wraps the objectId in a document with _id as the only key', async () => {
const oid = new ObjectId();
await bucket.find(oid).toArray();
expect(findsStarted).to.have.lengthOf(1);
expect(findsStarted[0]).to.have.nested.property('filter._id', oid);
expect(findsStarted[0].filter).to.have.all.keys('_id');
});
});
});
});
});

0 comments on commit 2647b61

Please sign in to comment.