Merge pull request #234 from bcgov/bugfix/soft-sync-tags

Ensure sync operation does not fail when unable to write tags to s3
bcgov · Dec 14, 2023 · 687361d · 687361d
2 parents 4904784 + 4b30cd3
commit 687361d
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 11 deletions.
diff --git 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
@@ -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({