From 4b30cd36b1c733a2d3ce63930a225318e9e43fb4 Mon Sep 17 00:00:00 2001 From: Jeremy Ho Date: Wed, 13 Dec 2023 15:04:58 -0800 Subject: [PATCH] Ensure sync operation does not fail when unable to write tags to s3 There are edge case scenarios where the PutObjectTaggingCommand will fail due to some upstream S3 configuration. We want to ensure that in the event sync is unable to write a coms-id tag to an object, the overall sync operation will still carry on instead of halting and failing out. Signed-off-by: Jeremy Ho --- app/src/services/sync.js | 28 ++++++++----- app/tests/unit/services/sync.spec.js | 63 ++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/app/src/services/sync.js b/app/src/services/sync.js index c35ad324..d80a83a4 100644 --- a/app/src/services/sync.js +++ b/app/src/services/sync.js @@ -42,6 +42,8 @@ const service = { filePath: path, bucketId: bucketId, tags: TagSet.concat([{ Key: 'coms-id', Value: objId }]) + }).catch((err) => { + log.warn(`Unable to add coms-id tag: ${err.message}`, { function: '_deriveObjectId' }); }); } } @@ -354,17 +356,21 @@ const service = { * NOTE: adding tags to a specified version (passing a `VersionId` parameter) will affect `Last Modified` * attribute of multiple versions on some s3 storage providors including Dell ECS */ - await storageService.putObjectTagging({ - filePath: path, - tags: (s3TagsForVersion?.TagSet ?? []).concat([{ - Key: 'coms-id', - Value: comsVersion.objectId - }]), - bucketId: bucketId, - // s3VersionId: comsVersion.s3VersionId, - }); - // add to our arrays for comaprison - s3Tags.push({ key: 'coms-id', value: comsVersion.objectId }); + try { + await storageService.putObjectTagging({ + filePath: path, + tags: (s3TagsForVersion?.TagSet ?? []).concat([{ + Key: 'coms-id', + Value: comsVersion.objectId + }]), + bucketId: bucketId, + // s3VersionId: comsVersion.s3VersionId, + }); + // add to our arrays for comaprison + s3Tags.push({ key: 'coms-id', value: comsVersion.objectId }); + } catch (err) { + log.warn(`Unable to add coms-id tag: ${err.message}`, { function: 'syncTags' }); + } } // Dissociate Tags not in S3 diff --git a/app/tests/unit/services/sync.spec.js b/app/tests/unit/services/sync.spec.js index a5088f98..2779f5c9 100644 --- a/app/tests/unit/services/sync.spec.js +++ b/app/tests/unit/services/sync.spec.js @@ -86,6 +86,7 @@ describe('_deriveObjectId', () => { getObjectTaggingSpy.mockResolvedValue({ TagSet: [{ Key: 'coms-id', Value: 'garbage' }] }); + putObjectTaggingSpy.mockResolvedValue(true); const result = await service._deriveObjectId({}, path, bucketId); @@ -106,8 +107,33 @@ describe('_deriveObjectId', () => { })); }); + it('Returns a new uuid if unavailable and pushes tags when less than 10 present', async () => { getObjectTaggingSpy.mockResolvedValue({ TagSet: [] }); + putObjectTaggingSpy.mockResolvedValue(true); + + const result = await service._deriveObjectId({}, path, bucketId); + + expect(result).toBeTruthy(); + expect(typeof result).toBe('string'); + expect(result).toMatch(uuidv4Regex); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(getObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({ + filePath: path, + bucketId: bucketId + })); + expect(listAllObjectVersionsSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(putObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({ + filePath: path, + bucketId: bucketId, + tags: expect.any(Array) + })); + }); + + it('Returns a new uuid if unavailable and continues when unable to write to S3', async () => { + getObjectTaggingSpy.mockResolvedValue({ TagSet: [] }); + putObjectTaggingSpy.mockRejectedValue({ message: 'nope' }); const result = await service._deriveObjectId({}, path, bucketId); @@ -1045,6 +1071,43 @@ describe('syncTags', () => { expect(versionTrx.commit).toHaveBeenCalledTimes(1); }); + it('should not write coms-id tag when coms version is latest and continues when unable to write to S3', async () => { + fetchTagsForVersionSpy.mockResolvedValue([{}]); + getObjectTaggingSpy.mockResolvedValue({}); + putObjectTaggingSpy.mockRejectedValue({ message: 'nope' }); + + const result = await service.syncTags(comsVersion, path, bucketId); + + expect(result).toBeTruthy(); + expect(Array.isArray(result)).toBeTruthy(); + expect(result).toHaveLength(0); + + expect(Version.startTransaction).toHaveBeenCalledTimes(1); + expect(associateTagsSpy).toHaveBeenCalledTimes(0); + expect(dissociateTagsSpy).toHaveBeenCalledTimes(0); + expect(fetchTagsForVersionSpy).toHaveBeenCalledTimes(1); + expect(fetchTagsForVersionSpy).toHaveBeenCalledWith(expect.objectContaining({ + versionIds: comsVersion.id + }), expect.any(Object)); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(getObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({ + filePath: path, + s3VersionId: comsVersion.s3VersionId, + bucketId: bucketId + })); + expect(getSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(putObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({ + filePath: path, + tags: expect.arrayContaining([{ + Key: 'coms-id', + Value: comsVersion.objectId + }]), + bucketId: bucketId, + })); + expect(versionTrx.commit).toHaveBeenCalledTimes(1); + }); + it('should not write coms-id tag when there are already 10 tags', async () => { fetchTagsForVersionSpy.mockResolvedValue([{}]); getObjectTaggingSpy.mockResolvedValue({