Skip to content

Commit 6bb4f4d

Browse files
alenakhineikaalenakhineika
authored
fix: allow updates on a sharded collection COMPASS-6058 (#4340)
* fix: an update on a sharded collection is failing unless the user has admin privileges COMPASS-6058 * refactor: check for docs * test: check if updateOne and replaceOne are being called * refactor: return error early * build: update package lock json * fix: use first document id * refactor: check for docs * fix: handle errors * fix: return the actual error when exists * fix: bump mongoLogId for replaceOne * refactor: get rid of repeating code
1 parent 31abf18 commit 6bb4f4d

File tree

3 files changed

+301
-104
lines changed

3 files changed

+301
-104
lines changed

‎packages/compass-crud/src/stores/crud-store.spec.ts‎

Lines changed: 179 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,114 +2011,239 @@ describe('store', function () {
20112011

20122012
describe('#findAndModifyWithFLEFallback', function () {
20132013
let dataServiceStub;
2014+
let findFake;
2015+
let findOneAndReplaceFake;
2016+
let findOneAndUpdateFake;
2017+
let updateOneFake;
2018+
let replaceOneFake;
2019+
2020+
const document = { _id: 1234, name: 'document_1234' };
2021+
const updatedDocument = { _id: 1234, name: 'document_12345' };
20142022

20152023
beforeEach(function () {
2024+
findFake = sinon.stub();
2025+
findOneAndReplaceFake = sinon.stub();
2026+
findOneAndUpdateFake = sinon.stub();
2027+
updateOneFake = sinon.stub();
2028+
replaceOneFake = sinon.stub();
20162029
dataServiceStub = {
2017-
find: sinon.stub().callsFake((ns, query) => Promise.resolve([query])),
2030+
find: findFake,
2031+
findOneAndReplace: findOneAndReplaceFake,
2032+
findOneAndUpdate: findOneAndUpdateFake,
2033+
updateOne: updateOneFake,
2034+
replaceOne: replaceOneFake,
20182035
};
20192036
});
20202037

2021-
it('does the original findAndModify operation and nothing more if it succeeds', async function () {
2022-
const document = { _id: 1234 };
2023-
const stub = sinon.stub().callsFake(() => {
2024-
return [undefined, document];
2038+
afterEach(function () {
2039+
sinon.restore();
2040+
});
2041+
2042+
it('does the original findOneAndUpdate operation and nothing more if it succeeds', async function () {
2043+
findFake.callsFake(() => Promise.resolve([]));
2044+
findOneAndReplaceFake.resolves({});
2045+
findOneAndUpdateFake.resolves(updatedDocument);
2046+
const [error, d] = await findAndModifyWithFLEFallback(
2047+
dataServiceStub,
2048+
'db.coll',
2049+
{ _id: 1234 } as any,
2050+
{ name: 'document_12345' },
2051+
'update'
2052+
);
2053+
expect(error).to.equal(undefined);
2054+
expect(d).to.equal(updatedDocument);
2055+
expect(findOneAndReplaceFake).to.have.callCount(0);
2056+
expect(findOneAndUpdateFake).to.have.callCount(1);
2057+
expect(findOneAndUpdateFake.firstCall.args[0]).to.equal('db.coll');
2058+
expect(findOneAndUpdateFake.firstCall.args[1]).to.deep.equal({
2059+
_id: 1234,
2060+
});
2061+
expect(findOneAndUpdateFake.firstCall.args[2]).to.deep.equal({
2062+
name: 'document_12345',
2063+
});
2064+
expect(findOneAndUpdateFake.firstCall.args[3]).to.deep.equal({
2065+
returnDocument: 'after',
2066+
promoteValues: false,
20252067
});
2068+
});
2069+
2070+
it('does the original findOneAndReplace operation and nothing more if it succeeds', async function () {
2071+
findFake.callsFake(() => Promise.resolve([]));
2072+
findOneAndReplaceFake.resolves(updatedDocument);
2073+
findOneAndUpdateFake.resolves({});
20262074
const [error, d] = await findAndModifyWithFLEFallback(
20272075
dataServiceStub,
20282076
'db.coll',
2029-
stub
2077+
{ _id: 1234 } as any,
2078+
{ name: 'document_12345' },
2079+
'replace'
20302080
);
20312081
expect(error).to.equal(undefined);
2032-
expect(d).to.equal(document);
2033-
expect(stub).to.have.callCount(1);
2034-
expect(stub.firstCall.args[0]).to.equal(dataServiceStub);
2035-
expect(stub.firstCall.args[1]).to.equal('db.coll');
2036-
expect(stub.firstCall.args[2]).to.deep.equal({
2082+
expect(d).to.equal(updatedDocument);
2083+
expect(findOneAndUpdateFake).to.have.callCount(0);
2084+
expect(findOneAndReplaceFake).to.have.callCount(1);
2085+
expect(findOneAndReplaceFake.firstCall.args[0]).to.equal('db.coll');
2086+
expect(findOneAndReplaceFake.firstCall.args[1]).to.deep.equal({
2087+
_id: 1234,
2088+
});
2089+
expect(findOneAndReplaceFake.firstCall.args[2]).to.deep.equal({
2090+
name: 'document_12345',
2091+
});
2092+
expect(findOneAndReplaceFake.firstCall.args[3]).to.deep.equal({
20372093
returnDocument: 'after',
20382094
promoteValues: false,
20392095
});
20402096
});
20412097

2042-
it('does the original findAndModify operation and nothing more if it fails with a non-FLE error', async function () {
2098+
it('does the original findOneAndUpdate operation and nothing more if it fails with a non-FLE error', async function () {
20432099
const err = new Error('failed');
2044-
const stub = sinon.stub().callsFake(() => {
2045-
return [err];
2046-
});
2100+
findFake.callsFake(() => Promise.resolve([]));
2101+
findOneAndReplaceFake.resolves({});
2102+
findOneAndUpdateFake.rejects(err);
20472103
const [error, d] = await findAndModifyWithFLEFallback(
20482104
dataServiceStub,
20492105
'db.coll',
2050-
stub
2106+
{ _id: 1234 } as any,
2107+
{ name: 'document_12345' },
2108+
'update'
20512109
);
20522110
expect(error).to.equal(err);
20532111
expect(d).to.equal(undefined);
2054-
expect(stub).to.have.callCount(1);
2055-
expect(stub.firstCall.args[0]).to.equal(dataServiceStub);
2056-
expect(stub.firstCall.args[1]).to.equal('db.coll');
2057-
expect(stub.firstCall.args[2]).to.deep.equal({
2112+
expect(findOneAndUpdateFake).to.have.callCount(1);
2113+
expect(findOneAndUpdateFake.firstCall.args[0]).to.equal('db.coll');
2114+
expect(findOneAndUpdateFake.firstCall.args[1]).to.deep.equal({
2115+
_id: 1234,
2116+
});
2117+
expect(findOneAndUpdateFake.firstCall.args[2]).to.deep.equal({
2118+
name: 'document_12345',
2119+
});
2120+
expect(findOneAndUpdateFake.firstCall.args[3]).to.deep.equal({
20582121
returnDocument: 'after',
20592122
promoteValues: false,
20602123
});
20612124
});
20622125

2063-
it('retries findAndModify with FLE returnDocument: "after"', async function () {
2064-
const document = { _id: 1234 };
2126+
it('retries findOneAndUpdate with FLE returnDocument: "after"', async function () {
20652127
const err = Object.assign(new Error('failed'), { code: 6371402 });
2066-
const stub = sinon.stub();
2067-
stub.onFirstCall().callsFake(() => {
2068-
return [err];
2069-
});
2070-
stub.onSecondCall().callsFake(() => {
2071-
return [undefined, document];
2072-
});
2128+
findFake.callsFake(() => Promise.resolve([updatedDocument]));
2129+
findOneAndReplaceFake.resolves({});
2130+
findOneAndUpdateFake.onCall(0).rejects(err);
2131+
findOneAndUpdateFake.onCall(1).resolves(document);
20732132
const [error, d] = await findAndModifyWithFLEFallback(
20742133
dataServiceStub,
20752134
'db.coll',
2076-
stub
2135+
{ _id: 1234 } as any,
2136+
{ name: 'document_12345' },
2137+
'update'
20772138
);
20782139
expect(error).to.equal(undefined);
2079-
expect(d).to.deep.equal(document);
2080-
expect(stub).to.have.callCount(2);
2081-
expect(stub.firstCall.args[0]).to.equal(dataServiceStub);
2082-
expect(stub.firstCall.args[1]).to.equal('db.coll');
2083-
expect(stub.firstCall.args[2]).to.deep.equal({
2140+
expect(d).to.deep.equal(updatedDocument);
2141+
expect(findOneAndUpdateFake).to.have.callCount(2);
2142+
2143+
expect(findOneAndUpdateFake.firstCall.args[0]).to.equal('db.coll');
2144+
expect(findOneAndUpdateFake.firstCall.args[1]).to.deep.equal({
2145+
_id: 1234,
2146+
});
2147+
expect(findOneAndUpdateFake.firstCall.args[2]).to.deep.equal({
2148+
name: 'document_12345',
2149+
});
2150+
expect(findOneAndUpdateFake.firstCall.args[3]).to.deep.equal({
20842151
returnDocument: 'after',
20852152
promoteValues: false,
20862153
});
2087-
expect(stub.secondCall.args[0]).to.equal(dataServiceStub);
2088-
expect(stub.secondCall.args[1]).to.equal('db.coll');
2089-
expect(stub.secondCall.args[2]).to.deep.equal({
2154+
2155+
expect(findOneAndUpdateFake.secondCall.args[0]).to.equal('db.coll');
2156+
expect(findOneAndUpdateFake.secondCall.args[1]).to.deep.equal({
2157+
_id: 1234,
2158+
});
2159+
expect(findOneAndUpdateFake.secondCall.args[2]).to.deep.equal({
2160+
name: 'document_12345',
2161+
});
2162+
expect(findOneAndUpdateFake.secondCall.args[3]).to.deep.equal({
20902163
returnDocument: 'before',
20912164
promoteValues: false,
20922165
});
2093-
expect(dataServiceStub.find).to.have.callCount(1);
2094-
expect(dataServiceStub.find.firstCall.args[0]).to.equal('db.coll');
2095-
expect(dataServiceStub.find.firstCall.args[1]).to.deep.equal(document);
2096-
expect(dataServiceStub.find.firstCall.args[2]).to.deep.equal({
2166+
2167+
expect(findFake).to.have.callCount(1);
2168+
expect(findFake.firstCall.args[0]).to.equal('db.coll');
2169+
expect(findFake.firstCall.args[1]).to.deep.equal({ _id: 1234 });
2170+
expect(findFake.firstCall.args[2]).to.deep.equal({
20972171
returnDocument: 'before',
20982172
promoteValues: false,
20992173
});
21002174
});
21012175

21022176
it('returns the original error if the fallback find operation fails', async function () {
2103-
dataServiceStub.find.yields(new Error('find failed'));
2104-
const document = { _id: 1234 };
21052177
const err = Object.assign(new Error('failed'), { code: 6371402 });
2106-
const stub = sinon.stub();
2107-
stub.onFirstCall().callsFake(() => {
2108-
return [err];
2109-
});
2110-
stub.onSecondCall().callsFake(() => {
2111-
return [undefined, document];
2112-
});
2178+
findFake.yields(new Error('find failed'));
2179+
findOneAndReplaceFake.resolves({});
2180+
findOneAndUpdateFake.onCall(0).rejects(err);
2181+
findOneAndUpdateFake.onCall(1).resolves(document);
21132182
const [error, d] = await findAndModifyWithFLEFallback(
21142183
dataServiceStub,
21152184
'db.coll',
2116-
stub
2185+
{ _id: 1234 } as any,
2186+
{ name: 'document_12345' },
2187+
'update'
21172188
);
21182189
expect(error).to.equal(err);
21192190
expect(d).to.equal(undefined);
2120-
expect(stub).to.have.callCount(2);
2121-
expect(dataServiceStub.find).to.have.callCount(1);
2191+
expect(findOneAndUpdateFake).to.have.callCount(2);
2192+
expect(findFake).to.have.callCount(1);
2193+
});
2194+
2195+
it('calls updateOne if findOneAndUpdate returns the ShardKeyNotFound error', async function () {
2196+
const err = Object.assign(
2197+
new Error('Query for sharded findAndModify must contain the shard key'),
2198+
{ codeName: 'ShardKeyNotFound' }
2199+
);
2200+
findFake.callsFake(() => Promise.resolve([updatedDocument]));
2201+
findOneAndReplaceFake.resolves({});
2202+
findOneAndUpdateFake.rejects(err);
2203+
updateOneFake.resolves(updatedDocument);
2204+
const [error, d] = await findAndModifyWithFLEFallback(
2205+
dataServiceStub,
2206+
'db.coll',
2207+
{ _id: 1234 } as any,
2208+
{ name: 'document_12345' },
2209+
'update'
2210+
);
2211+
expect(error).to.equal(undefined);
2212+
expect(d).to.equal(updatedDocument);
2213+
expect(findOneAndUpdateFake).to.have.callCount(1);
2214+
expect(updateOneFake).to.have.callCount(1);
2215+
expect(updateOneFake.firstCall.args[0]).to.equal('db.coll');
2216+
expect(updateOneFake.firstCall.args[1]).to.deep.equal({ _id: 1234 });
2217+
expect(updateOneFake.firstCall.args[2]).to.deep.equal({
2218+
name: 'document_12345',
2219+
});
2220+
});
2221+
2222+
it('calls replaceOne if findOneAndReplace returns the ShardKeyNotFound error', async function () {
2223+
const err = Object.assign(
2224+
new Error('Query for sharded findAndModify must contain the shard key'),
2225+
{ codeName: 'ShardKeyNotFound' }
2226+
);
2227+
findFake.callsFake(() => Promise.resolve([updatedDocument]));
2228+
findOneAndReplaceFake.rejects(err);
2229+
findOneAndUpdateFake.resolves({});
2230+
replaceOneFake.resolves(updatedDocument);
2231+
const [error, d] = await findAndModifyWithFLEFallback(
2232+
dataServiceStub,
2233+
'db.coll',
2234+
{ _id: 1234 } as any,
2235+
{ name: 'document_12345' },
2236+
'replace'
2237+
);
2238+
expect(error).to.equal(undefined);
2239+
expect(d).to.equal(updatedDocument);
2240+
expect(findOneAndReplaceFake).to.have.callCount(1);
2241+
expect(replaceOneFake).to.have.callCount(1);
2242+
expect(replaceOneFake.firstCall.args[0]).to.equal('db.coll');
2243+
expect(replaceOneFake.firstCall.args[1]).to.deep.equal({ _id: 1234 });
2244+
expect(replaceOneFake.firstCall.args[2]).to.deep.equal({
2245+
name: 'document_12345',
2246+
});
21222247
});
21232248
});
21242249
});

0 commit comments

Comments
 (0)