From a7ef75131a8e5510ac37f1320184900209ac5d48 Mon Sep 17 00:00:00 2001 From: Norris Ng <103449568+norrisng-bc@users.noreply.github.com> Date: Thu, 14 Nov 2024 17:41:17 -0800 Subject: [PATCH] _deriveObjectId(): delete coms-id tags if it conflicts with COMS --- app/src/services/object.js | 24 +++++++++++++++++++ app/src/services/sync.js | 21 +++++++++++++---- app/tests/unit/services/object.spec.js | 30 ++++++++++++++++++++++++ app/tests/unit/services/sync.spec.js | 32 +++++++++++++++++++++++++- 4 files changed, 101 insertions(+), 6 deletions(-) diff --git a/app/src/services/object.js b/app/src/services/object.js index 3b9c1735..f6a53ef0 100644 --- a/app/src/services/object.js +++ b/app/src/services/object.js @@ -210,6 +210,30 @@ const service = { } }, + /** + * @function exists + * Checks if an object db record exists + * @param {string} objId The object uuid to read + * @param {object} [etrx=undefined] An optional Objection Transaction object + * @returns {Promise} true if object exists in db, false otherwise + * @throws The error encountered upon db transaction failure + */ + exists: async (objId, etrx = undefined) => { + let trx; + try { + trx = etrx ? etrx : await ObjectModel.startTransaction(); + + const response = await ObjectModel.query(trx).findById(objId); + + if (!etrx) await trx.commit(); + + return response ? true : false; + } catch (err) { + if (!etrx && trx) await trx.rollback(); + throw err; + } + }, + /** * @function update * Update an object DB record diff --git a/app/src/services/sync.js b/app/src/services/sync.js index 1477e3d2..97640c67 100644 --- a/app/src/services/sync.js +++ b/app/src/services/sync.js @@ -21,23 +21,34 @@ const service = { * @function _deriveObjectId * Checks an S3 Object for any previous `coms-id` tag traces and returns it if found. * Otherwise it writes a new `coms-id` to the S3 Object if none were previously found. + * If the `coms-id` conflicts with an existing COMS object, it is replaced with a new one. * @param {object | boolean} s3Object The result of an S3 HeadObject operation * @param {string} path String representing the canonical path for the specified object * @param {string | null} bucketId uuid of bucket or `null` if syncing object in default bucket - * @returns {Promise} Resolves to an existing objectId or creates a new one + * @returns {Promise} Resolves to an existing objectId (if not already in COMS) + * or creates a new one */ _deriveObjectId: async (s3Object, path, bucketId) => { let objId = uuidv4(); if (typeof s3Object === 'object') { // If regular S3 Object - const TagSet = await storageService.getObjectTagging({ filePath: path, bucketId: bucketId }) + let TagSet = await storageService.getObjectTagging({ filePath: path, bucketId: bucketId }) .then(result => result.TagSet ?? []); + const s3ObjectComsId = TagSet.find(obj => (obj.Key === 'coms-id'))?.Value; - if (s3ObjectComsId && uuidValidate(s3ObjectComsId)) { + if (s3ObjectComsId + && uuidValidate(s3ObjectComsId) + && !(await objectService.exists(s3ObjectComsId))) { + // re-use existing coms-id (if no conflict) objId = s3ObjectComsId; - } else { // Update S3 Object if there is still remaining space in the TagSet - if (TagSet.length < 10) { // putObjectTagging replaces S3 tags so new TagSet must contain existing values + } else { + // remove `coms-id` tag since it conflicts with an existing COMS object + TagSet = TagSet.filter(x => x.Key != 'coms-id'); + + // Update S3 Object if there is still remaining space in the TagSet + if (TagSet.length < 10) { + // putObjectTagging replaces S3 tags so new TagSet must contain existing values await storageService.putObjectTagging({ filePath: path, bucketId: bucketId, diff --git a/app/tests/unit/services/object.spec.js b/app/tests/unit/services/object.spec.js index f13fcdef..60f4f722 100644 --- a/app/tests/unit/services/object.spec.js +++ b/app/tests/unit/services/object.spec.js @@ -226,6 +226,36 @@ describe('read', () => { }); }); +describe('exists', () => { + it('returns true if coms-id already exists in db ', async () => { + ObjectModel.findById.mockResolvedValueOnce(true); + + const result = await service.exists(data.id); + + expect(result).toBeTruthy(); + expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); + expect(ObjectModel.query).toHaveBeenCalledTimes(1); + expect(ObjectModel.query).toHaveBeenCalledWith(expect.anything()); + expect(ObjectModel.findById).toHaveBeenCalledTimes(1); + expect(ObjectModel.findById).toBeCalledWith(OBJECT_ID); + expect(objectModelTrx.commit).toHaveBeenCalledTimes(1); + }); + + it('returns false if coms-id does not exist in db', async () => { + ObjectModel.findById.mockResolvedValueOnce(false); + + const result = await service.exists(data.id); + + expect(result).toBeFalsy(); + expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); + expect(ObjectModel.query).toHaveBeenCalledTimes(1); + expect(ObjectModel.query).toHaveBeenCalledWith(expect.anything()); + expect(ObjectModel.findById).toHaveBeenCalledTimes(1); + expect(ObjectModel.findById).toBeCalledWith(OBJECT_ID); + expect(objectModelTrx.commit).toHaveBeenCalledTimes(1); + }); +}); + describe('update', () => { it('Update an object DB record', async () => { diff --git a/app/tests/unit/services/sync.spec.js b/app/tests/unit/services/sync.spec.js index c0d24a64..fca6405c 100644 --- a/app/tests/unit/services/sync.spec.js +++ b/app/tests/unit/services/sync.spec.js @@ -64,11 +64,15 @@ afterAll(() => { // Mockrestores must only happen after suite is completed }); describe('_deriveObjectId', () => { + + const existsSpy = jest.spyOn(objectService, 'exists'); + describe('Regular S3 Object', () => { - it('Returns an existing coms-id if valid', async () => { + it('Returns an existing coms-id if valid and doesn\'t already exist in COMS', async () => { getObjectTaggingSpy.mockResolvedValue({ TagSet: [{ Key: 'coms-id', Value: validUuidv4 }] }); + existsSpy.mockResolvedValue(false); const result = await service._deriveObjectId({}, path, bucketId); @@ -84,6 +88,32 @@ describe('_deriveObjectId', () => { expect(putObjectTaggingSpy).toHaveBeenCalledTimes(0); }); + it('Returns a new uuid and removes coms-id tag if it already exists in COMS', async () => { + getObjectTaggingSpy.mockResolvedValue({ + TagSet: [{ Key: 'coms-id', Value: validUuidv4 }] + }); + existsSpy.mockResolvedValue(true); + jest.spyOn(storageService, 'putObjectTagging').mockImplementation(() => Promise.resolve()); + + const result = await service._deriveObjectId({}, path, bucketId); + + expect(result).toBeTruthy(); + expect(typeof result).toBe('string'); + expect(result).not.toMatch(validUuidv4); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(getObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({ + filePath: path, + bucketId: bucketId + })); + expect(listAllObjectVersionsSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(putObjectTaggingSpy).toHaveBeenCalledWith({ + filePath: path, + bucketId: bucketId, + tags: [{ Key: 'coms-id', Value: expect.not.stringMatching(validUuidv4) }] + }); + }); + it('Returns a new uuid if invalid and pushes tags when less than 10 present', async () => { getObjectTaggingSpy.mockResolvedValue({ TagSet: [{ Key: 'coms-id', Value: 'garbage' }]