Skip to content

Commit

Permalink
syncObject: delete 'coms-id' tag on new file if id already in use
Browse files Browse the repository at this point in the history
  • Loading branch information
@norrisng-bc
norrisng-bc committed Nov 28, 2024
1 parent 439dd44 commit 9827baf
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 1 deletion.
34 changes: 33 additions & 1 deletion app/src/services/sync.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
82 changes: 82 additions & 0 deletions app/tests/unit/services/sync.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand All @@ -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,
Expand Down Expand Up @@ -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);

Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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));
Expand Down

0 comments on commit 9827baf

Please sign in to comment.