From 3bc4b10020b8567af5b0bfaef457e68e0815b717 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 2 Jul 2024 17:11:56 +0200 Subject: [PATCH 1/3] fix: the explainable cursor should reflect simple collation MONGOSH-1670 --- packages/shell-api/src/explainable.spec.ts | 86 ++++++++++++++-------- packages/shell-api/src/explainable.ts | 6 +- packages/shell-api/src/integration.spec.ts | 8 ++ 3 files changed, 67 insertions(+), 33 deletions(-) diff --git a/packages/shell-api/src/explainable.spec.ts b/packages/shell-api/src/explainable.spec.ts index c6daa6c2a3..4ce1b5ddaf 100644 --- a/packages/shell-api/src/explainable.spec.ts +++ b/packages/shell-api/src/explainable.spec.ts @@ -110,46 +110,70 @@ describe('Explainable', function () { }); describe('find', function () { - let cursorStub; - let explainResult; - beforeEach(async function () { - explainResult = { ok: 1 }; + let explainResult: Document; + const expectedExplainResult = { ok: 1 }; - const cursorSpy = { - explain: sinon.spy(() => explainResult), - } as unknown; - collection.find = sinon.spy(() => Promise.resolve(cursorSpy as Cursor)); + context('without options', function () { + let cursorStub; + let explainResult; - cursorStub = await explainable.find({ query: 1 }, { projection: 1 }); - }); + beforeEach(async function () { + explainResult = { ok: 1 }; - it('calls collection.find with arguments', function () { - expect(collection.find).to.have.been.calledOnceWithExactly( - { query: 1 }, - { projection: 1 } - ); - }); + const cursorSpy = { + explain: sinon.spy(() => explainResult), + } as unknown; + collection.find = sinon.spy(() => + Promise.resolve(cursorSpy as Cursor) + ); - it('returns an cursor that has toShellResult when evaluated', async function () { - expect((await toShellResult(cursorStub)).type).to.equal( - 'ExplainableCursor' + cursorStub = await explainable.find({ query: 1 }, { projection: 1 }); + }); + + it('calls collection.find with arguments', function () { + expect(collection.find).to.have.been.calledOnceWithExactly( + { query: 1 }, + { projection: 1 }, + {} + ); + }); + + it('returns an cursor that has toShellResult when evaluated', async function () { + expect((await toShellResult(cursorStub)).type).to.equal( + 'ExplainableCursor' + ); + }); + + context( + 'when calling toShellResult().printable on the result', + function () { + it('calls explain with verbosity', function () { + expect(cursorStub._verbosity).to.equal('queryPlanner'); + }); + + it('returns the explain result', async function () { + expect((await toShellResult(cursorStub)).printable).to.equal( + explainResult + ); + }); + } ); }); - context( - 'when calling toShellResult().printable on the result', - function () { - it('calls explain with verbosity', function () { - expect(cursorStub._verbosity).to.equal('queryPlanner'); - }); + context('with options', function () { + beforeEach(function () { + collection.aggregate = sinon.spy(() => + Promise.resolve(expectedExplainResult) + ) as any; + }); - it('returns the explain result', async function () { - expect((await toShellResult(cursorStub)).printable).to.equal( - explainResult - ); + it('returns the explain result', async function () { + explainResult = await explainable.aggregate([{ pipeline: 1 }], { + aggregate: 1, }); - } - ); + expect(explainResult).to.equal(expectedExplainResult); + }); + }); }); describe('aggregate', function () { diff --git a/packages/shell-api/src/explainable.ts b/packages/shell-api/src/explainable.ts index c3a9240234..393840af81 100644 --- a/packages/shell-api/src/explainable.ts +++ b/packages/shell-api/src/explainable.ts @@ -31,6 +31,7 @@ import type { FindOneAndDeleteOptions, FindOneAndReplaceOptions, FindOneAndUpdateOptions, + FindOptions, } from '@mongosh/service-provider-core'; @shellApiClassDefault @@ -97,11 +98,12 @@ export default class Explainable extends ShellApiWithMongoClass { @returnsPromise async find( query?: Document, - projection?: Document + projection?: Document, + options: FindOptions = {} ): Promise { this._emitExplainableApiCall('find', { query, projection }); - const cursor = await this._collection.find(query, projection); + const cursor = await this._collection.find(query, projection, options); return new ExplainableCursor(this._mongo, cursor, this._verbosity); } diff --git a/packages/shell-api/src/integration.spec.ts b/packages/shell-api/src/integration.spec.ts index 8847ef9a35..19fccd9a3e 100644 --- a/packages/shell-api/src/integration.spec.ts +++ b/packages/shell-api/src/integration.spec.ts @@ -1568,6 +1568,14 @@ describe('Shell API (integration)', function () { 'serverInfo', ]); }); + + it('the explainable cursor reflects collation', async function () { + const cursor = await explainable.find({}, undefined, { + collation: { locale: 'simple' }, + }); + const result = await toShellResult(cursor); + expect(result.printable.command.collation.locale).to.be.equal('simple'); + }); }); describe('aggregate', function () { From 18bed726ef86299ae83d7d7d2663c47d0af8a9f3 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 2 Jul 2024 17:20:08 +0200 Subject: [PATCH 2/3] test: update find test with options --- packages/shell-api/src/explainable.spec.ts | 40 +++++++++++++++------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/shell-api/src/explainable.spec.ts b/packages/shell-api/src/explainable.spec.ts index 4ce1b5ddaf..f59e1e7ef9 100644 --- a/packages/shell-api/src/explainable.spec.ts +++ b/packages/shell-api/src/explainable.spec.ts @@ -110,9 +110,6 @@ describe('Explainable', function () { }); describe('find', function () { - let explainResult: Document; - const expectedExplainResult = { ok: 1 }; - context('without options', function () { let cursorStub; let explainResult; @@ -161,17 +158,36 @@ describe('Explainable', function () { }); context('with options', function () { - beforeEach(function () { - collection.aggregate = sinon.spy(() => - Promise.resolve(expectedExplainResult) - ) as any; - }); + let cursorStub; + let explainResult; - it('returns the explain result', async function () { - explainResult = await explainable.aggregate([{ pipeline: 1 }], { - aggregate: 1, + beforeEach(async function () { + explainResult = { ok: 1 }; + + const cursorSpy = { + explain: sinon.spy(() => explainResult), + } as unknown; + collection.find = sinon.spy(() => + Promise.resolve(cursorSpy as Cursor) + ); + + cursorStub = await explainable.find({}, undefined, { + collation: { locale: 'simple' }, }); - expect(explainResult).to.equal(expectedExplainResult); + }); + + it('calls collection.find with arguments', function () { + expect(collection.find).to.have.been.calledOnceWithExactly( + {}, + undefined, + { collation: { locale: 'simple' } } + ); + }); + + it('returns an cursor that has toShellResult when evaluated', async function () { + expect((await toShellResult(cursorStub)).type).to.equal( + 'ExplainableCursor' + ); }); }); }); From 7df55cdc3ad351def6813fb60a07a2f62ed787a7 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 3 Jul 2024 14:23:28 +0200 Subject: [PATCH 3/3] test: skip if server prior 4.4 --- packages/shell-api/src/integration.spec.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/shell-api/src/integration.spec.ts b/packages/shell-api/src/integration.spec.ts index 19fccd9a3e..b7fcb37a20 100644 --- a/packages/shell-api/src/integration.spec.ts +++ b/packages/shell-api/src/integration.spec.ts @@ -1569,12 +1569,17 @@ describe('Shell API (integration)', function () { ]); }); - it('the explainable cursor reflects collation', async function () { - const cursor = await explainable.find({}, undefined, { - collation: { locale: 'simple' }, + describe('after server 4.4', function () { + skipIfServerVersion(testServer, '<= 4.4'); + it('the explainable cursor reflects collation', async function () { + const cursor = await explainable.find({}, undefined, { + collation: { locale: 'simple' }, + }); + const result = await toShellResult(cursor); + expect(result.printable.command.collation.locale).to.be.equal( + 'simple' + ); }); - const result = await toShellResult(cursor); - expect(result.printable.command.collation.locale).to.be.equal('simple'); }); });