From 0ce6dcb5169dc90c13c0cdd25fc0e5a991d456b8 Mon Sep 17 00:00:00 2001 From: Arthur Morrow Date: Fri, 8 Nov 2024 13:38:01 -0500 Subject: [PATCH] increased coverage, fixed logic error in office-sync Jira ticket: CAMS-442 Co-authored-by: Arthur Morrow <133667008+amorrow-flexion@users.noreply.github.com> Co-authored-by: James Brooks <12275865+jamesobrooks@users.noreply.github.com> Co-authored-by: Brian Posey <15091170+btposey@users.noreply.github.com>, --- .../mongo/offices.mongo.repository.test.ts | 7 +- .../mongo/offices.mongo.repository.ts | 5 +- .../mongo/utils/mongo-adapter.test.ts | 130 ++++++++++++++---- .../gateways/mongo/utils/mongo-adapter.ts | 45 +++--- 4 files changed, 131 insertions(+), 56 deletions(-) diff --git a/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.test.ts b/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.test.ts index 29a5c4ecc..7eb144332 100644 --- a/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.test.ts +++ b/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.test.ts @@ -57,13 +57,12 @@ describe('offices repo', () => { ...session.user, ttl, }); - - const insertOneSpy = jest - .spyOn(MongoCollectionAdapter.prototype, 'insertOne') + const replaceOneSpy = jest + .spyOn(MongoCollectionAdapter.prototype, 'replaceOne') .mockResolvedValue('inserted-id'); await repo.putOfficeStaff(officeCode, session.user); - expect(insertOneSpy).toHaveBeenCalledWith({ ...staff, updatedOn: expect.anything() }); + expect(replaceOneSpy).toHaveBeenCalledWith(expect.anything(), staff, true); }); describe('error handling', () => { diff --git a/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.ts b/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.ts index d6abb23b6..a32640137 100644 --- a/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.ts +++ b/backend/functions/lib/adapters/gateways/mongo/offices.mongo.repository.ts @@ -34,8 +34,11 @@ export class OfficesMongoRepository extends BaseMongoRepository implements Offic ...user, ttl, }); + const query = QueryBuilder.build( + and(equals('id', staff.id), equals('officeCode', officeCode)), + ); try { - await this.getAdapter().insertOne(staff); + await this.getAdapter().replaceOne(query, staff, true); } catch (originalError) { throw getCamsError(originalError, MODULE_NAME); } diff --git a/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.test.ts b/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.test.ts index 642ab771c..9165818f8 100644 --- a/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.test.ts +++ b/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.test.ts @@ -1,9 +1,9 @@ import { CamsError } from '../../../../common-errors/cams-error'; import { NotFoundError } from '../../../../common-errors/not-found-error'; import { UnknownError } from '../../../../common-errors/unknown-error'; -import { CollectionHumble } from '../../../../humble-objects/mongo-humble'; +import { CollectionHumble, DocumentClient } from '../../../../humble-objects/mongo-humble'; import QueryBuilder from '../../../../query/query-builder'; -import { MongoCollectionAdapter } from './mongo-adapter'; +import { MongoCollectionAdapter, removeIds } from './mongo-adapter'; const { and, orderBy } = QueryBuilder; @@ -29,7 +29,11 @@ const spies = { countDocuments, }; -type TestType = object; +type TestType = { + _id?: unknown; + id?: string; + foo?: string; +}; describe('Mongo adapter', () => { const testQuery = QueryBuilder.build(and()); @@ -40,6 +44,23 @@ describe('Mongo adapter', () => { jest.resetAllMocks(); }); + test('should return an instance of the adapter from newAdapter', () => { + const mockClient: DocumentClient = { + database: () => { + return { + collection: <_t>() => {}, + }; + }, + } as unknown as DocumentClient; + const adapter = MongoCollectionAdapter.newAdapter( + 'module', + 'collection', + 'database', + mockClient, + ); + expect(adapter).toBeInstanceOf(MongoCollectionAdapter); + }); + test('should return items from a getAll', async () => { find.mockResolvedValue([{}, {}, {}]); const item = await adapter.getAll(); @@ -47,6 +68,13 @@ describe('Mongo adapter', () => { expect(find).toHaveBeenCalled(); }); + // TODO: maybe remove this? + test('should remove Ids from an item', async () => { + const expectedItem = { arbitraryValue: 'arbitrary-value' }; + const item = { _id: 'some_id', id: 'someId', ...expectedItem }; + expect(removeIds(item)).toEqual(expectedItem); + }); + test('should return a sorted list of items from a getAll', async () => { function* generator() { yield Promise.resolve({}); @@ -101,24 +129,77 @@ describe('Mongo adapter', () => { ); }); - test('should return a single Id from a replaceOne', async () => { - const id = '123456'; - replaceOne.mockResolvedValue({ acknowdledged: true, upsertedId: id }); - const result = await adapter.replaceOne(testQuery, {}); - expect(result).toEqual(id); + test('should return a single Id from replaceOne', async () => { + const testObject: TestType = { id: '12345', foo: 'bar' }; + const _id = 'mongoGeneratedId'; + replaceOne.mockResolvedValue({ + acknowledged: true, + matchedCount: 1, + modifiedCount: 1, + upsertedId: _id, + }); + const result = await adapter.replaceOne(testQuery, testObject); + expect(result).not.toEqual(_id); + expect(result).toEqual(testObject.id); + }); + + test('should throw an error calling replaceOne for a nonexistant record and upsert=false', async () => { + const testObject: TestType = { id: '12345', foo: 'bar' }; + replaceOne.mockResolvedValue({ + acknowledged: false, + matchedCount: 0, + modifiedCount: 0, + upsertedId: null, + }); + await expect(adapter.replaceOne(testQuery, testObject)).rejects.toThrow( + 'No matching item found.', + ); + }); + + test('should return a single Id from replaceOne when upsert = true', async () => { + const testObject: TestType = { id: '12345', foo: 'bar' }; + const _id = 'mongoGeneratedId'; + + replaceOne.mockResolvedValue({ + acknowledged: true, + matchedCount: 0, + modifiedCount: 1, + upsertedId: _id, + }); + const result = await adapter.replaceOne(testQuery, testObject, true); + expect(result).toEqual(testObject.id); + expect(result).not.toEqual(_id); + }); + + test('should throw an error if replaceOne does not match.', async () => { + const testObject: TestType = { id: '12345', foo: 'bar' }; + replaceOne.mockResolvedValue({ + acknowledged: false, + matchedCount: 0, + modifiedCount: 0, + upsertedId: null, + }); + await expect(adapter.replaceOne(testQuery, testObject, true)).rejects.toThrow( + 'Failed to insert document into database.', + ); }); test('should return a single Id from insertOne', async () => { const id = '123456'; - insertOne.mockResolvedValue({ acknowdledged: true, insertedId: id }); + insertOne.mockResolvedValue({ acknowledged: true, insertedId: id }); const result = await adapter.insertOne({}); expect(result.split('-').length).toEqual(5); }); + test('should throw an error if insertOne does not insert.', async () => { + insertOne.mockResolvedValue({ acknowledged: false }); + await expect(adapter.insertOne({})).rejects.toThrow('Failed to insert document into database.'); + }); + test('should return a list of Ids from insertMany', async () => { const ids = ['0', '1', '2', '3', '4']; insertMany.mockResolvedValue({ - acknowdledged: true, + acknowledged: true, insertedIds: ids, insertedCount: ids.length, }); @@ -127,7 +208,7 @@ describe('Mongo adapter', () => { }); test('should return a count of 1 for 1 item deleted', async () => { - deleteOne.mockResolvedValue({ acknowdledged: true, deletedCount: 1 }); + deleteOne.mockResolvedValue({ acknowledged: true, deletedCount: 1 }); const result = await adapter.deleteOne(testQuery); expect(result).toEqual(1); }); @@ -140,7 +221,7 @@ describe('Mongo adapter', () => { }); test('should return a count of 5 for 5 items deleted', async () => { - deleteMany.mockResolvedValue({ acknowdledged: true, deletedCount: 5 }); + deleteMany.mockResolvedValue({ acknowledged: true, deletedCount: 5 }); const result = await adapter.deleteMany(testQuery); expect(result).toEqual(5); }); @@ -158,8 +239,15 @@ describe('Mongo adapter', () => { expect(result).toEqual(5); }); + test('should return a count of 6 when countAllDocuments is called and there are 6 documents', async () => { + countDocuments.mockResolvedValue(6); + const result = await adapter.countAllDocuments(); + expect(result).toEqual(6); + }); + test('should throw CamsError when some but not all items are inserted', async () => { insertMany.mockResolvedValue({ + acknowledged: true, insertedIds: { one: 'one', two: 'two', @@ -176,22 +264,6 @@ describe('Mongo adapter', () => { expect(async () => await adapter.insertMany([{}, {}, {}, {}])).rejects.toThrow(error); }); - test('should handle acknowledged == false', async () => { - const response = { acknowledged: false }; - const error = new UnknownError(MODULE_NAME, { - message: 'Operation returned Not Acknowledged.', - }); - Object.values(spies).forEach((spy) => { - spy.mockResolvedValue(response); - }); - - await expect(adapter.replaceOne(testQuery, {})).rejects.toThrow(error); - await expect(adapter.insertOne({})).rejects.toThrow(error); - await expect(adapter.insertMany([{}])).rejects.toThrow(error); - await expect(adapter.deleteOne(testQuery)).rejects.toThrow(error); - await expect(adapter.deleteMany(testQuery)).rejects.toThrow(error); - }); - test('should handle errors', async () => { const originalError = new Error('Test Exception'); const expectedError = new UnknownError(MODULE_NAME, { originalError }); @@ -206,6 +278,8 @@ describe('Mongo adapter', () => { await expect(adapter.deleteMany(testQuery)).rejects.toThrow(expectedError); await expect(adapter.find(testQuery)).rejects.toThrow(expectedError); await expect(adapter.countDocuments(testQuery)).rejects.toThrow(expectedError); + await expect(adapter.countAllDocuments()).rejects.toThrow(expectedError); await expect(adapter.findOne(testQuery)).rejects.toThrow(expectedError); + await expect(adapter.getAll()).rejects.toThrow(expectedError); }); }); diff --git a/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.ts b/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.ts index 3d4285085..9f069ac09 100644 --- a/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.ts +++ b/backend/functions/lib/adapters/gateways/mongo/utils/mongo-adapter.ts @@ -10,21 +10,11 @@ import { randomUUID } from 'crypto'; export class MongoCollectionAdapter implements DocumentCollectionAdapter { private collectionHumble: CollectionHumble; - private readonly notAcknowledged: UnknownError; private readonly moduleName: string; - private testAcknowledged(result: { acknowledged?: boolean }) { - if (result.acknowledged === false) { - throw this.notAcknowledged; - } - } - constructor(moduleName: string, collection: CollectionHumble) { this.collectionHumble = collection; this.moduleName = moduleName; - this.notAcknowledged = new UnknownError(this.moduleName, { - message: 'Operation returned Not Acknowledged.', - }); } public async find(query: ConditionOrConjunction, sort?: Sort): Promise { @@ -79,9 +69,16 @@ export class MongoCollectionAdapter implements DocumentCollectionAdapter { const mongoItem = createOrGetId(item); try { const result = await this.collectionHumble.replaceOne(mongoQuery, mongoItem, upsert); - this.testAcknowledged(result); - - return result.upsertedId?.toString(); + if (!result.acknowledged) { + if (upsert) { + throw new UnknownError(this.moduleName, { + message: 'Failed to insert document into database.', + }); + } else { + throw new NotFoundError(this.moduleName, { message: 'No matching item found.' }); + } + } + return mongoItem.id; } catch (originalError) { throw getCamsError(originalError, this.moduleName); } @@ -90,11 +87,15 @@ export class MongoCollectionAdapter implements DocumentCollectionAdapter { public async insertOne(item: T) { try { const cleanItem = removeIds(item); - const identifiableItem = createOrGetId(cleanItem); - const result = await this.collectionHumble.insertOne(identifiableItem); - this.testAcknowledged(result); + const mongoItem = createOrGetId(cleanItem); + const result = await this.collectionHumble.insertOne(mongoItem); + if (!result.acknowledged) { + throw new UnknownError(this.moduleName, { + message: 'Failed to insert document into database.', + }); + } - return identifiableItem.id; + return mongoItem.id; } catch (originalError) { throw getCamsError(originalError, this.moduleName); } @@ -107,7 +108,6 @@ export class MongoCollectionAdapter implements DocumentCollectionAdapter { return createOrGetId(cleanItem); }); const result = await this.collectionHumble.insertMany(mongoItems); - this.testAcknowledged(result); const insertedIds = mongoItems.map((item) => item.id); if (insertedIds.length !== result.insertedCount) { throw new CamsError(this.moduleName, { @@ -125,7 +125,6 @@ export class MongoCollectionAdapter implements DocumentCollectionAdapter { const mongoQuery = toMongoQuery(query); try { const result = await this.collectionHumble.deleteOne(mongoQuery); - this.testAcknowledged(result); if (result.deletedCount !== 1) { throw new NotFoundError(this.moduleName, { message: 'No items deleted' }); } @@ -140,7 +139,6 @@ export class MongoCollectionAdapter implements DocumentCollectionAdapter { const mongoQuery = toMongoQuery(query); try { const result = await this.collectionHumble.deleteMany(mongoQuery); - this.testAcknowledged(result); if (result.deletedCount < 1) { throw new NotFoundError(this.moduleName, { message: 'No items deleted' }); } @@ -190,10 +188,11 @@ function createOrGetId(item: CamsItem): CamsItem { return mongoItem; } -function removeIds(item: CamsItem): CamsItem { +// TODO: sus. +export function removeIds(item: CamsItem): CamsItem { const cleanItem = { ...item }; - delete cleanItem?._id; - delete cleanItem?.id; + delete cleanItem._id; + delete cleanItem.id; return cleanItem; }