Skip to content

Commit

Permalink
Implement statusSync and statusDefault endpoint support
Browse files Browse the repository at this point in the history
In order to reduce code duplication, we reuse the syncBucket controller
with a few value tweaks so that it can handle both endpoint types. We also
move the sync* controller functions to the sync controller in order to
reduce controller pollution of the large object and bucket controllers, and
invoke these functions directly through the router instead. Lastly, some
database layer modifications are done in order to allow bucketId to accept
null fields.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
  • Loading branch information
jujaga committed Jul 27, 2023
1 parent 0baf1ca commit 6418f20
Show file tree
Hide file tree
Showing 14 changed files with 201 additions and 74 deletions.
34 changes: 1 addition & 33 deletions app/src/controllers/bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const {
} = require('../components/utils');
const { redactSecrets } = require('../db/models/utils');

const { bucketService, objectQueueService, objectService, storageService, userService } = require('../services');
const { bucketService, storageService, userService } = require('../services');

const SERVICE = 'BucketService';
const secretFields = ['accessKeyId', 'secretAccessKey'];
Expand Down Expand Up @@ -212,38 +212,6 @@ const controller = {
}
},

/**
* @function syncBucket
* Synchronizes a bucket
* @param {object} req Express request object
* @param {object} res Express response object
* @param {function} next The next callback function
* @returns {function} Express middleware function
*/
async syncBucket(req, res, next) {
try {
const bucketId = addDashesToUuid(req.params.bucketId);
const userId = await userService.getCurrentUserId(getCurrentIdentity(req.currentUser, SYSTEM_USER), SYSTEM_USER);

const [dbResponse, s3Response] = await Promise.all([
objectService.searchObjects({ bucketId: bucketId }),
storageService.listAllObjectVersions({ bucketId: bucketId, filterLatest: true })
]);

// Aggregate and dedupe all file paths to consider
const jobs = [...new Set([
...dbResponse.map(object => object.path),
...s3Response.DeleteMarkers.map(object => object.Key),
...s3Response.Versions.map(object => object.Key)
])].map(path => ({ path: path, bucketId: bucketId }));

const response = await objectQueueService.enqueue({ jobs: jobs, full: isTruthy(req.query.full), createdBy: userId });
res.status(202).json(response);
} catch (e) {
next(errorToProblem(SERVICE, e));
}
},

/**
* @function updateBucket
* Updates a bucket
Expand Down
1 change: 1 addition & 0 deletions app/src/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
metadataController: require('./metadata'),
objectController: require('./object'),
objectPermissionController: require('./objectPermission'),
syncController: require('./sync'),
userController: require('./user'),
tagController: require('./tag'),
versionController: require('./version')
Expand Down
27 changes: 0 additions & 27 deletions app/src/controllers/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ const {
bucketPermissionService,
metadataService,
objectService,
objectQueueService,
syncService,
storageService,
tagService,
userService,
Expand Down Expand Up @@ -891,31 +889,6 @@ const controller = {
}
},

/**
* @function syncObject
* Synchronizes an object
* @param {object} req Express request object
* @param {object} res Express response object
* @param {function} next The next callback function
* @returns {function} Express middleware function
*/
async syncObject(req, res, next) {
try {
const bucketId = req.currentObject?.bucketId;
const path = req.currentObject?.path;
const userId = await userService.getCurrentUserId(getCurrentIdentity(req.currentUser, SYSTEM_USER), SYSTEM_USER);

const response = await objectQueueService.enqueue({
jobs: [{ path: path, bucketId: bucketId }],
full: isTruthy(req.query.full),
createdBy: userId
});
res.status(202).json(response);
} catch (e) {
next(errorToProblem(SERVICE, e));
}
},

/**
* @function togglePublic
* Sets the public flag of an object
Expand Down
92 changes: 92 additions & 0 deletions app/src/controllers/sync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
const { NIL: SYSTEM_USER } = require('uuid');

const errorToProblem = require('../components/errorToProblem');
const { addDashesToUuid, getCurrentIdentity, isTruthy } = require('../components/utils');
const { objectService, storageService, objectQueueService, userService } = require('../services');

const SERVICE = 'ObjectQueueService';

/**
* The Sync Controller
*/
const controller = {
/**
* @function syncBucket
* Synchronizes a bucket
* @param {object} req Express request object
* @param {object} res Express response object
* @param {function} next The next callback function
* @returns {function} Express middleware function
*/
async syncBucket(req, res, next) {
try {
const allMode = isTruthy(req.query.all);
const bucketId = addDashesToUuid(req.params.bucketId);
const userId = await userService.getCurrentUserId(getCurrentIdentity(req.currentUser, SYSTEM_USER), SYSTEM_USER);

const dbParams = {};
if (!allMode) dbParams.bucketId = bucketId;

const [dbResponse, s3Response] = await Promise.all([
objectService.searchObjects(dbParams),
storageService.listAllObjectVersions({ bucketId: bucketId, filterLatest: true })
]);

// Aggregate and dedupe all file paths to consider
const jobs = [...new Set([
...dbResponse.map(object => object.path),
...s3Response.DeleteMarkers.map(object => object.Key),
...s3Response.Versions.map(object => object.Key)
])].map(path => ({ path: path, bucketId: bucketId }));

const response = await objectQueueService.enqueue({ jobs: jobs, full: isTruthy(req.query.full), createdBy: userId });
res.status(202).json(response);
} catch (e) {
next(errorToProblem(SERVICE, e));
}
},

/**
* @function syncObject
* Synchronizes an object
* @param {object} req Express request object
* @param {object} res Express response object
* @param {function} next The next callback function
* @returns {function} Express middleware function
*/
async syncObject(req, res, next) {
try {
const bucketId = req.currentObject?.bucketId;
const path = req.currentObject?.path;
const userId = await userService.getCurrentUserId(getCurrentIdentity(req.currentUser, SYSTEM_USER), SYSTEM_USER);

const response = await objectQueueService.enqueue({
jobs: [{ path: path, bucketId: bucketId }],
full: isTruthy(req.query.full),
createdBy: userId
});
res.status(202).json(response);
} catch (e) {
next(errorToProblem(SERVICE, e));
}
},

/**
* @function syncStatus
* Reports on current sync queue size
* @param {object} req Express request object
* @param {object} res Express response object
* @param {function} next The next callback function
* @returns {function} Express middleware function
*/
async syncStatus(_req, res, next) {
try {
const response = await objectQueueService.queueSize();
res.status(200).json(response);
} catch (e) {
next(errorToProblem(SERVICE, e));
}
}
};

module.exports = controller;
8 changes: 6 additions & 2 deletions app/src/db/models/tables/objectModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ class ObjectModel extends Timestamps(Model) {
filterOneOrMany(query, value, 'object.id');
},
filterBucketIds(query, value) {
filterOneOrMany(query, value, 'object.bucketId');
if (value === null) {
query.whereNull('object.bucketId');
} else {
filterOneOrMany(query, value, 'object.bucketId');
}
},
filterName(query, value) {
filterILike(query, value, 'object.name');
Expand Down Expand Up @@ -192,7 +196,7 @@ class ObjectModel extends Timestamps(Model) {
path: { type: 'string', minLength: 1, maxLength: 1024 },
public: { type: 'boolean' },
active: { type: 'boolean' },
bucketId: { type: 'string', maxLength: 255 },
bucketId: { type: 'string', maxLength: 255, nullable: true },
name: { type: 'string', maxLength: 1024 },
...stamps
},
Expand Down
2 changes: 1 addition & 1 deletion app/src/db/models/tables/objectQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ObjectModel extends Timestamps(Model) {
required: ['path', 'full', 'retries'],
properties: {
id: { type: 'integer' },
bucketId: { type: 'string', maxLength: 255 },
bucketId: { type: 'string', maxLength: 255, nullable: true },
path: { type: 'string', minLength: 1, maxLength: 1024 },
full: { type: 'boolean' },
retries: { type: 'integer' },
Expand Down
4 changes: 2 additions & 2 deletions app/src/routes/v1/bucket.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const router = require('express').Router();

const { Permissions } = require('../../components/constants');
const { bucketController } = require('../../controllers');
const { bucketController, syncController } = require('../../controllers');
const { bucketValidator } = require('../../validators');
const { requireSomeAuth } = require('../../middleware/featureToggle');
const { checkAppMode, hasPermission } = require('../../middleware/authorization');
Expand Down Expand Up @@ -46,7 +46,7 @@ router.delete('/:bucketId', bucketValidator.deleteBucket, hasPermission(Permissi

/** Synchronizes a bucket */
router.get('/:bucketId/sync', bucketValidator.syncBucket, hasPermission(Permissions.READ), (req, res, next) => {
bucketController.syncBucket(req, res, next);
syncController.syncBucket(req, res, next);
});

module.exports = router;
5 changes: 5 additions & 0 deletions app/src/routes/v1/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ router.get('/', (_req, res) => {
'/metadata',
'/object',
'/permission',
'/sync',
'/tagging',
'/user',
'/version'
]
Expand All @@ -33,6 +35,9 @@ router.use('/object', require('./object'));
/** Permission Router */
router.use('/permission', require('./permission'));

/** Sync Router */
router.use('/sync', require('./sync'));

/** Tagging Router */
router.use('/tagging', require('./tag'));

Expand Down
4 changes: 2 additions & 2 deletions app/src/routes/v1/object.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const router = require('express').Router();

const { Permissions } = require('../../components/constants');
const { objectController } = require('../../controllers');
const { objectController, syncController } = require('../../controllers');
const { objectValidator } = require('../../validators');
const { requireSomeAuth } = require('../../middleware/featureToggle');
const { checkAppMode, currentObject, hasPermission } = require('../../middleware/authorization');
Expand Down Expand Up @@ -76,7 +76,7 @@ router.delete('/:objectId/metadata', objectValidator.deleteMetadata, requireSome

/** Synchronizes an object */
router.get('/:objectId/sync', objectValidator.syncObject, requireSomeAuth, currentObject, hasPermission(Permissions.READ), (req, res, next) => {
objectController.syncObject(req, res, next);
syncController.syncObject(req, res, next);
});

/** Add tags to an object */
Expand Down
22 changes: 22 additions & 0 deletions app/src/routes/v1/sync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const router = require('express').Router();

const { syncController } = require('../../controllers');
const { syncValidator } = require('../../validators');
const { checkAppMode } = require('../../middleware/authorization');
const { requireBasicAuth, requireSomeAuth } = require('../../middleware/featureToggle');

router.use(checkAppMode);
router.use(requireSomeAuth);

/** Synchronizes the default bucket */
router.get('/', syncValidator.syncDefault, requireBasicAuth, (req, res, next) => {
req.params.bucketId = null;
syncController.syncBucket(req, res, next);
});

/** Check sync queue size */
router.get('/status', (req, res, next) => {
syncController.syncStatus(req, res, next);
});

module.exports = router;
1 change: 1 addition & 0 deletions app/src/validators/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
metadataValidator: require('./metadata'),
objectValidator: require('./object'),
objectPermissionValidator: require('./objectPermission'),
syncValidator: require('./sync'),
tagValidator: require('./tag'),
userValidator: require('./user'),
versionValidator: require('./version'),
Expand Down
18 changes: 18 additions & 0 deletions app/src/validators/sync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const { validate, Joi } = require('express-validation');
const { type } = require('./common');


const schema = {
syncDefault: {
query: Joi.object({
full: type.truthy
})
}
};

const validator = {
syncDefault: validate(schema.syncDefault, { statusCode: 422 })
};

module.exports = validator;
module.exports.schema = schema;
19 changes: 12 additions & 7 deletions app/tests/unit/routes/v1.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,18 @@ describe(`GET ${basePath}`, () => {
expect(response.statusCode).toBe(200);
expect(response.body).toBeTruthy();
expect(Array.isArray(response.body.endpoints)).toBeTruthy();
expect(response.body.endpoints).toHaveLength(7);
expect(response.body.endpoints).toContain('/bucket');
expect(response.body.endpoints).toContain('/docs');
expect(response.body.endpoints).toContain('/metadata');
expect(response.body.endpoints).toContain('/object');
expect(response.body.endpoints).toContain('/permission');
expect(response.body.endpoints).toContain('/user');
expect(response.body.endpoints).toHaveLength(9);
expect(response.body.endpoints).toEqual(expect.arrayContaining([
'/bucket',
'/docs',
'/metadata',
'/object',
'/permission',
'/sync',
'/tagging',
'/user',
'/version'
]));
});
});

38 changes: 38 additions & 0 deletions app/tests/unit/validators/sync.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const jestJoi = require('jest-joi');
expect.extend(jestJoi.matchers);

const { schema } = require('../../../src/validators/sync');

describe('syncDefault', () => {

describe('query', () => {
const query = schema.syncDefault.query.describe();

describe('full', () => {
const full = query.keys.full;

it('is a boolean', () => {
expect(full).toBeTruthy();
expect(full.type).toEqual('boolean');
});

it('contains truthy array', () => {
expect(Array.isArray(full.truthy)).toBeTruthy();
expect(full.truthy).toHaveLength(12);
});

it.each([
true, 1, 'true', 'TRUE', 't', 'T', 'yes', 'yEs', 'y', 'Y', '1',
false, 0, 'false', 'FALSE', 'f', 'F', 'no', 'nO', 'n', 'N', '0'
])('accepts the schema given %j', (value) => {
const req = {
query: {
full: value
}
};

expect(req).toMatchSchema(schema.syncDefault);
});
});
});
});

0 comments on commit 6418f20

Please sign in to comment.