diff --git a/app/src/services/sync.js b/app/src/services/sync.js index 1477e3d2..47743d17 100644 --- a/app/src/services/sync.js +++ b/app/src/services/sync.js @@ -175,7 +175,39 @@ const service = { // Case: not in COMS - insert new COMS object else { - const objId = await service._deriveObjectId(s3Object, path, bucketId); + let objId = await service._deriveObjectId(s3Object, path, bucketId); + let existingComsId; + try { + existingComsId = await objectService.read(objId); + } catch { + // Derived object id not currently in use by COMS + } + + // Object id in 'coms-id' S3 tag already in use in COMS: + // Delete the 'coms-id' tag on the new object, + // so that we don't attempt to re-use an existing COMS object id. + if (existingComsId) { + const sourceObject = await storageService.getObjectTagging({ + filePath: path, + bucketId: bucketId + }); + + // Workaround for deleting just the 'coms-id' tag (S3 can only delete ALL tags on an object): + // Retrieve all tags, delete all tags on object, then re-add all tags except 'coms-id'. + const objectTags = sourceObject.TagSet.filter( + x => !sourceObject.TagSet.includes(x.Key) && x.Key != 'coms-id' + ); + const data = { + bucketId, + filePath: path, + tags: objectTags + }; + await storageService.putObjectTagging(data); + + // _deriveObjectId() doesn't care if it conflicts with existing coms-id's, + // so generate a new one after deleting the S3 tag + objId = uuidv4(); + } response = await objectService.create({ id: objId, diff --git a/app/tests/unit/services/sync.spec.js b/app/tests/unit/services/sync.spec.js index c0d24a64..a1c64442 100644 --- a/app/tests/unit/services/sync.spec.js +++ b/app/tests/unit/services/sync.spec.js @@ -348,9 +348,11 @@ describe('syncObject', () => { const _deriveObjectIdSpy = jest.spyOn(service, '_deriveObjectId'); const createSpy = jest.spyOn(objectService, 'create'); const deleteSpy = jest.spyOn(objectService, 'delete'); + const deleteObjectTaggingSpy = jest.spyOn(storageService, 'deleteObjectTagging'); const getObjectPublicSpy = jest.spyOn(storageService, 'getObjectPublic'); const pruneOrphanedMetadataSpy = jest.spyOn(metadataService, 'pruneOrphanedMetadata'); const pruneOrphanedTagsSpy = jest.spyOn(tagService, 'pruneOrphanedTags'); + const readSpy = jest.spyOn(objectService, 'read'); const searchObjectsSpy = jest.spyOn(objectService, 'searchObjects'); const updateSpy = jest.spyOn(objectService, 'update'); @@ -391,6 +393,10 @@ describe('syncObject', () => { expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledTimes(0); + expect(readSpy).toHaveBeenCalledTimes(0); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(deleteObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(0); expect(createSpy).toHaveBeenCalledTimes(0); expect(deleteSpy).toHaveBeenCalledTimes(0); expect(getObjectPublicSpy).toHaveBeenCalledTimes(1); @@ -429,6 +435,10 @@ describe('syncObject', () => { expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledTimes(0); + expect(readSpy).toHaveBeenCalledTimes(0); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(deleteObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(0); expect(createSpy).toHaveBeenCalledTimes(0); expect(deleteSpy).toHaveBeenCalledTimes(0); expect(getObjectPublicSpy).toHaveBeenCalledTimes(1); @@ -467,6 +477,10 @@ describe('syncObject', () => { expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledTimes(0); + expect(readSpy).toHaveBeenCalledTimes(0); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(deleteObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(0); expect(createSpy).toHaveBeenCalledTimes(0); expect(deleteSpy).toHaveBeenCalledTimes(0); expect(getObjectPublicSpy).toHaveBeenCalledTimes(1); @@ -494,6 +508,7 @@ describe('syncObject', () => { headObjectSpy.mockResolvedValue({}); searchObjectsSpy.mockResolvedValue({ total: 0, data: [] }); getObjectPublicSpy.mockResolvedValue(true); + readSpy.mockImplementation(() => { throw new Error(); }); const result = await service.syncObject(path, bucketId); @@ -504,6 +519,10 @@ describe('syncObject', () => { expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledWith(expect.any(Object), path, bucketId); + expect(readSpy).toHaveBeenCalledTimes(1); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(deleteObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(0); expect(createSpy).toHaveBeenCalledTimes(1); expect(createSpy).toHaveBeenCalledWith(expect.objectContaining({ id: validUuidv4, @@ -538,6 +557,7 @@ describe('syncObject', () => { headObjectSpy.mockResolvedValue({}); searchObjectsSpy.mockResolvedValue({ total: 0, data: [] }); getObjectPublicSpy.mockImplementation(() => { throw new Error(); }); + readSpy.mockImplementation(() => { throw new Error(); }); const result = await service.syncObject(path, bucketId); @@ -548,6 +568,10 @@ describe('syncObject', () => { expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledWith(expect.any(Object), path, bucketId); + expect(readSpy).toHaveBeenCalledTimes(1); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(deleteObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(0); expect(createSpy).toHaveBeenCalledTimes(1); expect(createSpy).toHaveBeenCalledWith(expect.objectContaining({ id: validUuidv4, @@ -575,6 +599,60 @@ describe('syncObject', () => { expect(objectModelTrx.commit).toHaveBeenCalledTimes(1); }); + it('should insert new object when not in COMS when it has a conflicting coms-id S3 tag', async () => { + const comsObject = {}; + _deriveObjectIdSpy.mockResolvedValue(validUuidv4); + createSpy.mockResolvedValue(comsObject); + headObjectSpy.mockResolvedValue({}); + searchObjectsSpy.mockResolvedValue({ total: 0, data: [] }); + getObjectPublicSpy.mockResolvedValue(true); + readSpy.mockResolvedValue(comsObject); + getObjectTaggingSpy.mockResolvedValue({ + TagSet: [{}, { Key: 'coms-id', Value: validUuidv4 }] + }); + deleteObjectTaggingSpy.mockImplementation(jest.fn()); + + const result = await service.syncObject(path, bucketId); + + expect(result).toBeTruthy(); + expect(result.modified).toBeTruthy(); + expect(result.object).toEqual(comsObject); + + expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); + expect(_deriveObjectIdSpy).toHaveBeenCalledTimes(1); + expect(_deriveObjectIdSpy).toHaveBeenCalledWith(expect.any(Object), path, bucketId); + expect(readSpy).toHaveBeenCalledTimes(1); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(deleteObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(1); + expect(createSpy).toHaveBeenCalledTimes(1); + expect(createSpy).toHaveBeenCalledWith(expect.objectContaining({ + id: expect.stringMatching(uuidv4Regex), + name: path.match(/(?!.*\/)(.*)$/)[0], + path: path, + public: true, + bucketId: bucketId, + userId: expect.any(String), + }), expect.any(Object)); + expect(createSpy.mock.calls[0]['id']).not.toEqual(validUuidv4); + expect(deleteSpy).toHaveBeenCalledTimes(0); + expect(getObjectPublicSpy).toHaveBeenCalledTimes(1); + expect(getObjectPublicSpy).toHaveBeenCalledWith(expect.objectContaining({ + filePath: path, bucketId: bucketId + })); + expect(headObjectSpy).toHaveBeenCalledTimes(1); + expect(headObjectSpy).toHaveBeenCalledWith(expect.objectContaining({ + filePath: path, bucketId: bucketId + })); + expect(pruneOrphanedMetadataSpy).toHaveBeenCalledTimes(0); + expect(pruneOrphanedTagsSpy).toHaveBeenCalledTimes(0); + expect(searchObjectsSpy).toHaveBeenCalledTimes(1); + expect(searchObjectsSpy).toHaveBeenCalledWith(expect.objectContaining({ + path: path, bucketId: bucketId + }), expect.any(Object)); + expect(objectModelTrx.commit).toHaveBeenCalledTimes(1); + }); + it('should drop COMS object when not in S3', async () => { const comsObject = { id: validUuidv4 }; deleteSpy.mockResolvedValue(comsObject); @@ -591,6 +669,10 @@ describe('syncObject', () => { expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1); expect(_deriveObjectIdSpy).toHaveBeenCalledTimes(0); + expect(readSpy).toHaveBeenCalledTimes(0); + expect(getObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(deleteObjectTaggingSpy).toHaveBeenCalledTimes(0); + expect(putObjectTaggingSpy).toHaveBeenCalledTimes(0); expect(createSpy).toHaveBeenCalledTimes(0); expect(deleteSpy).toHaveBeenCalledTimes(1); expect(deleteSpy).toHaveBeenCalledWith(validUuidv4, expect.any(Object));