diff --git a/app/src/controllers/sync.js b/app/src/controllers/sync.js index 6ce4d7ca..7e903c2b 100644 --- a/app/src/controllers/sync.js +++ b/app/src/controllers/sync.js @@ -29,7 +29,7 @@ const controller = { // Aggregate and dedupe all file paths to consider const jobs = [...new Set([ - ...dbResponse.map(object => object.path), + ...dbResponse.data.map(object => object.path), ...s3Response.DeleteMarkers.map(object => object.Key), ...s3Response.Versions.map(object => object.Key) ])].map(path => ({ path: path, bucketId: bucketId })); diff --git a/app/src/services/object.js b/app/src/services/object.js index a32b6c5a..e7398ff0 100644 --- a/app/src/services/object.js +++ b/app/src/services/object.js @@ -112,18 +112,23 @@ const service = { * @param {string} [params.name] Optional metadata name string to match on * @param {object} [params.metadata] Optional object of metadata key/value pairs * @param {object} [params.tag] Optional object of tag key/value pairs + * @param {object} [params.limit] Optional number of records to limit by + * @param {object} [params.order] Optional column attribute to order by + * @param {object} [params.page] Optional page set to return + * @param {object} [params.sort] Optional `asc` or `desc` sort ordering * @param {object} [etrx=undefined] An optional Objection Transaction object - * @returns {Promise} The result of running the find operation + * @returns {Promise<{total: number, data: object[]}>} The find operation result containing the `total` length of + * the search query, and the relevant array of COMS objects. */ searchObjects: async (params, etrx = undefined) => { let trx; - let response = []; + let response = {}; try { trx = etrx ? etrx : await ObjectModel.startTransaction(); // GroupBy() seems to be working faster with ObjectionJS Graphs // when comparing with distinct() response.data = await ObjectModel.query(trx) - .allowGraph('[objectPermission, version, bucketPermission]') + .allowGraph('[bucketPermission, objectPermission, version]') .groupBy('object.id') .modify('filterIds', params.id) .modify('filterBucketIds', params.bucketId) @@ -142,16 +147,16 @@ const service = { .modify('pagination', params.page, params.limit) .modify('sortOrder', params.sort, params.order) .then(result => { - let resultObject = []; + let results = []; if (Object.hasOwn(result, 'results')) { - resultObject = result.results; + results = result.results; response.total = result.total; } else { - resultObject = result; + results = result; response.total = result.length; } return Promise.all( - resultObject.map(row => { + results.map(row => { // eslint-disable-next-line no-unused-vars const { objectPermission, bucketPermission, version, ...object } = row; if (params.permissions) { diff --git a/app/src/services/sync.js b/app/src/services/sync.js index 63d708d2..63012c38 100644 --- a/app/src/services/sync.js +++ b/app/src/services/sync.js @@ -143,7 +143,7 @@ const service = { .catch((e) => { // return boolean true if object is soft-deleted in S3 return !!e.$response.headers['x-amz-delete-marker']; }) - ]).then(settled => settled.map(promise => Array.isArray(promise.value) ? promise.value[0] : promise.value)); + ]).then(([comsPromise, s3Promise]) => ([comsPromise.value?.data[0], s3Promise.value])); if (s3Object) { // Determine S3 Public status diff --git a/app/tests/unit/controllers/sync.spec.js b/app/tests/unit/controllers/sync.spec.js index edbaa08e..29bcbb5c 100644 --- a/app/tests/unit/controllers/sync.spec.js +++ b/app/tests/unit/controllers/sync.spec.js @@ -37,7 +37,7 @@ describe('syncBucket', () => { DeleteMarkers: [{ Key: path }], Versions: [{ Key: path }] }); - searchObjectsSpy.mockResolvedValue([{ path: path }]); + searchObjectsSpy.mockResolvedValue({ total: 1, data: [{ path: path }] }); await controller.syncBucket(req, res, next); diff --git a/app/tests/unit/services/object.spec.js b/app/tests/unit/services/object.spec.js index 0c760788..3aaa604a 100644 --- a/app/tests/unit/services/object.spec.js +++ b/app/tests/unit/services/object.spec.js @@ -104,22 +104,84 @@ describe('getBucketKey', () => { }); describe('searchObjects', () => { - it('Search and filter for specific object records', async () => { - ObjectModel.then.mockImplementation(() => { }); + // TODO: Add in other untested multiplicity cases + it('Search and filter for specific object records with permissions and without pagination', async () => { const params = { bucketId: BUCKET_ID, bucketName: 'bucketName', active: 'true', key: 'key', - userId: SYSTEM_USER + userId: 'ae8a58f6-62bc-4dd1-acef-79f123609d48', + permissions: true }; - await service.searchObjects(params); + // We only care about mocking the final 13th chained modify result + for (let i = 0; i < 12; i++) ObjectModel.modify.mockReturnValueOnce(ObjectModel); + ObjectModel.modify.mockResolvedValueOnce([{ + objectPermission: [{ permCode: 'READ' }], ...params + }]); + + const result = await service.searchObjects(params); + + expect(result).toBeTruthy(); + expect(result).toHaveProperty('total'); + expect(result).toHaveProperty('data'); + expect(Array.isArray(result.data)).toBeTruthy(); + expect(result.data[0]).toHaveProperty('permissions'); + expect(result.data[0].permissions).toEqual(expect.arrayContaining(['READ'])); + + expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); + expect(ObjectModel.query).toHaveBeenCalledTimes(1); + expect(ObjectModel.query).toHaveBeenCalledWith(expect.anything()); + expect(ObjectModel.allowGraph).toHaveBeenCalledTimes(1); + expect(ObjectModel.allowGraph).toHaveBeenCalledWith('[bucketPermission, objectPermission, version]'); + expect(ObjectModel.groupBy).toHaveBeenCalledTimes(1); + expect(ObjectModel.modify).toHaveBeenCalledTimes(13); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(1, 'filterIds', params.id); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(2, 'filterBucketIds', params.bucketId); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(3, 'filterName', params.name); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(4, 'filterPath', params.path); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(5, 'filterPublic', params.public); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(6, 'filterActive', params.active); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(7, 'filterMimeType', params.mimeType); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(8, 'filterDeleteMarker', params.deleteMarker); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(9, 'filterLatest', params.latest); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(10, 'filterMetadataTag', { + metadata: params.metadata, + tag: params.tag + }); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(11, 'hasPermission', params.userId, 'READ'); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(12, 'pagination', params.page, params.limit); + expect(ObjectModel.modify).toHaveBeenNthCalledWith(13, 'sortOrder', params.sort, params.order); + expect(objectModelTrx.commit).toHaveBeenCalledTimes(1); + }); + + it('Search and filter for specific object records with pagination', async () => { + // We only care about mocking the final 13th chained modify result + for (let i = 0; i < 12; i++) ObjectModel.modify.mockReturnValueOnce(ObjectModel); + ObjectModel.modify.mockResolvedValueOnce({ total: 0, results: [] }); + + const params = { + bucketId: BUCKET_ID, + bucketName: 'bucketName', + active: 'true', + key: 'key', + userId: SYSTEM_USER, + permissions: false + }; + + const result = await service.searchObjects(params); + + expect(result).toBeTruthy(); + expect(result).toHaveProperty('total'); + expect(result).toHaveProperty('data'); + expect(Array.isArray(result.data)).toBeTruthy(); expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); expect(ObjectModel.query).toHaveBeenCalledTimes(1); expect(ObjectModel.query).toHaveBeenCalledWith(expect.anything()); expect(ObjectModel.allowGraph).toHaveBeenCalledTimes(1); + expect(ObjectModel.allowGraph).toHaveBeenCalledWith('[bucketPermission, objectPermission, version]'); expect(ObjectModel.groupBy).toHaveBeenCalledTimes(1); expect(ObjectModel.modify).toHaveBeenCalledTimes(13); expect(ObjectModel.modify).toHaveBeenNthCalledWith(1, 'filterIds', params.id); @@ -138,7 +200,6 @@ describe('searchObjects', () => { expect(ObjectModel.modify).toHaveBeenNthCalledWith(11, 'hasPermission', params.userId, 'READ'); expect(ObjectModel.modify).toHaveBeenNthCalledWith(12, 'pagination', params.page, params.limit); expect(ObjectModel.modify).toHaveBeenNthCalledWith(13, 'sortOrder', params.sort, params.order); - expect(ObjectModel.then).toHaveBeenCalledTimes(1); expect(objectModelTrx.commit).toHaveBeenCalledTimes(1); }); }); diff --git a/app/tests/unit/services/sync.spec.js b/app/tests/unit/services/sync.spec.js index b3aea5c5..80b62d5b 100644 --- a/app/tests/unit/services/sync.spec.js +++ b/app/tests/unit/services/sync.spec.js @@ -377,7 +377,7 @@ describe('syncObject', () => { it('should return object when already synced', async () => { const comsObject = { id: validUuidv4, path: path, public: true }; headObjectSpy.mockResolvedValue({}); - searchObjectsSpy.mockResolvedValue([comsObject]); + searchObjectsSpy.mockResolvedValue({ total: 1, data: [comsObject] }); getObjectPublicSpy.mockResolvedValue(true); const result = await service.syncObject(path, bucketId); @@ -411,7 +411,7 @@ describe('syncObject', () => { it('should return object when already synced but public mismatch', async () => { const comsObject = { id: validUuidv4, path: path, public: true }; headObjectSpy.mockResolvedValue({}); - searchObjectsSpy.mockResolvedValue([comsObject]); + searchObjectsSpy.mockResolvedValue({ total: 1, data: [comsObject] }); getObjectPublicSpy.mockResolvedValue(false); updateSpy.mockResolvedValue(comsObject); @@ -449,7 +449,7 @@ describe('syncObject', () => { it('should return object when already synced but S3 ACL errors out', async () => { const comsObject = { id: validUuidv4, path: path, public: true }; headObjectSpy.mockResolvedValue({}); - searchObjectsSpy.mockResolvedValue([comsObject]); + searchObjectsSpy.mockResolvedValue({ total: 1, data: [comsObject] }); getObjectPublicSpy.mockImplementation(() => { throw new Error(); }); const result = await service.syncObject(path, bucketId); @@ -485,7 +485,7 @@ describe('syncObject', () => { _deriveObjectIdSpy.mockResolvedValue(validUuidv4); createSpy.mockResolvedValue(comsObject); headObjectSpy.mockResolvedValue({}); - searchObjectsSpy.mockResolvedValue(undefined); + searchObjectsSpy.mockResolvedValue({ total: 0, data: [] }); getObjectPublicSpy.mockResolvedValue(true); const result = await service.syncObject(path, bucketId); @@ -529,7 +529,7 @@ describe('syncObject', () => { _deriveObjectIdSpy.mockResolvedValue(validUuidv4); createSpy.mockResolvedValue(comsObject); headObjectSpy.mockResolvedValue({}); - searchObjectsSpy.mockResolvedValue(undefined); + searchObjectsSpy.mockResolvedValue({ total: 0, data: [] }); getObjectPublicSpy.mockImplementation(() => { throw new Error(); }); const result = await service.syncObject(path, bucketId); @@ -574,7 +574,7 @@ describe('syncObject', () => { headObjectSpy.mockRejectedValue({}); pruneOrphanedMetadataSpy.mockResolvedValue(0); pruneOrphanedTagsSpy.mockResolvedValue(0); - searchObjectsSpy.mockResolvedValue([comsObject]); + searchObjectsSpy.mockResolvedValue({ total: 1, data: [comsObject] }); const result = await service.syncObject(path, bucketId); diff --git a/app/tests/unit/validators/bucket.spec.js b/app/tests/unit/validators/bucket.spec.js index 33c5eb6c..c446c6ee 100644 --- a/app/tests/unit/validators/bucket.spec.js +++ b/app/tests/unit/validators/bucket.spec.js @@ -147,7 +147,7 @@ describe('createBucketChild', () => { }); }); - describe.only('key', () => { + describe('key', () => { const subKey = body.keys.subKey; it('is a string', () => {