Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Populate lastSyncedDate on object create/update when syncing object #247

Merged
merged 3 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/src/docs/v1.api-spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,8 @@ paths:
This endpoint does not guarantee immediately synchronized results.
Synchronization latency will be affected by the remaining number of
objects awaiting synchronization.
The `lastSyncedDate` attribute will be asynchronously updated when it
performs a successful synchronization with the S3 bucket
tags:
- Sync
operationId: syncObject
Expand Down
5 changes: 4 additions & 1 deletion app/src/services/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ const service = {
* @param {string} data.path The relative S3 key/path of the object
* @param {boolean} [data.public] The optional public flag - defaults to true if undefined
* @param {boolean} [data.active] The optional active flag - defaults to true if undefined
* @param {string} [data.lastSyncedDate] The last time a sync request was made for the object.
* Should be left undefined if not part of a sync operation
* @param {object} [etrx=undefined] An optional Objection Transaction object
* @returns {Promise<object>} The result of running the patch operation
* @throws The error encountered upon db transaction failure
Expand All @@ -232,7 +234,8 @@ const service = {
path: data.path,
public: data.public,
active: data.active,
updatedBy: data.userId ?? SYSTEM_USER
updatedBy: data.userId ?? SYSTEM_USER,
lastSyncedDate: data.lastSyncedDate
});

if (!etrx) await trx.commit();
Expand Down
17 changes: 13 additions & 4 deletions app/src/services/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,19 @@ const service = {
// Case: already synced - record & update public status as needed
if (comsObject) {
if (s3Public === undefined || s3Public === comsObject.public) {
response = comsObject;
response = await objectService.update({
id: comsObject.id,
userId: userId,
lastSyncedDate: new Date().toISOString()
}, trx);
} else {
response = await objectService.update({
id: comsObject.id, userId: userId, path: comsObject.path, public: s3Public
});
id: comsObject.id,
userId: userId,
path: comsObject.path,
public: s3Public,
lastSyncedDate: new Date().toISOString()
}, trx);
modified = true;
}
}
Expand All @@ -179,7 +187,8 @@ const service = {
path: path,
public: s3Public,
bucketId: bucketId,
userId: userId
userId: userId,
lastSyncedDate: new Date().toISOString()
}, trx);

modified = true;
Expand Down
26 changes: 24 additions & 2 deletions app/tests/unit/services/object.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const data = {
public: 'true',
active: 'true',
createdBy: SYSTEM_USER,
userId: SYSTEM_USER
userId: SYSTEM_USER,
lastSyncedDate: undefined
};

beforeEach(() => {
Expand Down Expand Up @@ -227,6 +228,7 @@ describe('read', () => {

describe('update', () => {
it('Update an object DB record', async () => {

await service.update({ ...data });

expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1);
Expand All @@ -236,7 +238,27 @@ describe('update', () => {
path: data.path,
public: data.public,
active: data.active,
updatedBy: data.userId
updatedBy: data.userId,
lastSyncedDate: undefined
});
expect(objectModelTrx.commit).toHaveBeenCalledTimes(1);
});

it('Update an object DB record as part of a sync operation', async () => {

const testDateString = new Date('2024-01-01T00:00:00').toISOString();

await service.update({ ...data, lastSyncedDate: testDateString });

expect(ObjectModel.startTransaction).toHaveBeenCalledTimes(1);
expect(ObjectModel.query).toHaveBeenCalledTimes(1);
expect(ObjectModel.patchAndFetchById).toHaveBeenCalledTimes(1);
expect(ObjectModel.patchAndFetchById).toBeCalledWith(data.id, {
path: data.path,
public: data.public,
active: data.active,
updatedBy: data.userId,
lastSyncedDate: testDateString
});
expect(objectModelTrx.commit).toHaveBeenCalledTimes(1);
});
Expand Down
17 changes: 12 additions & 5 deletions app/tests/unit/services/sync.spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { NIL: SYSTEM_USER } = require('uuid');

const { resetModel, trxBuilder } = require('../../common/helper');
const utils = require('../../../src/db/models/utils');
const ObjectModel = require('../../../src/db/models/tables/objectModel');
Expand Down Expand Up @@ -371,14 +373,15 @@ describe('syncObject', () => {
pruneOrphanedMetadataSpy.mockRestore();
pruneOrphanedTagsSpy.mockRestore();
searchObjectsSpy.mockRestore();
updateSpy.mockReset();
updateSpy.mockRestore();
});

it('should return object when already synced', async () => {
const comsObject = { id: validUuidv4, path: path, public: true };
headObjectSpy.mockResolvedValue({});
searchObjectsSpy.mockResolvedValue({ total: 1, data: [comsObject] });
getObjectPublicSpy.mockResolvedValue(true);
updateSpy.mockResolvedValue(comsObject);

const result = await service.syncObject(path, bucketId);

Expand All @@ -405,7 +408,10 @@ describe('syncObject', () => {
path: path, bucketId: bucketId
}), expect.any(Object));
expect(objectModelTrx.commit).toHaveBeenCalledTimes(1);
expect(updateSpy).toHaveBeenCalledTimes(0);
expect(updateSpy).toHaveBeenCalledTimes(1);
expect(updateSpy).toHaveBeenCalledWith(expect.objectContaining({
id: comsObject.id, lastSyncedDate: expect.anything()
}), expect.any(Object));
});

it('should return object when already synced but public mismatch', async () => {
Expand Down Expand Up @@ -442,15 +448,16 @@ describe('syncObject', () => {
expect(objectModelTrx.commit).toHaveBeenCalledTimes(1);
expect(updateSpy).toHaveBeenCalledTimes(1);
expect(updateSpy).toHaveBeenCalledWith(expect.objectContaining({
id: validUuidv4, path: path, public: false
}));
id: validUuidv4, path: path, public: false, lastSyncedDate: expect.anything(), userId: SYSTEM_USER
}), expect.any(Object));
});

it('should return object when already synced but S3 ACL errors out', async () => {
const comsObject = { id: validUuidv4, path: path, public: true };
headObjectSpy.mockResolvedValue({});
searchObjectsSpy.mockResolvedValue({ total: 1, data: [comsObject] });
getObjectPublicSpy.mockImplementation(() => { throw new Error(); });
updateSpy.mockResolvedValue(comsObject);

const result = await service.syncObject(path, bucketId);

Expand All @@ -477,7 +484,7 @@ describe('syncObject', () => {
path: path, bucketId: bucketId
}), expect.any(Object));
expect(objectModelTrx.commit).toHaveBeenCalledTimes(1);
expect(updateSpy).toHaveBeenCalledTimes(0);
expect(updateSpy).toHaveBeenCalledTimes(1);
});

it('should insert new object when not in COMS', async () => {
Expand Down
Loading