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

Add patch functionality to lists #691

Merged
merged 9 commits into from
Jan 15, 2021
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
15 changes: 14 additions & 1 deletion lib/routes/_lists.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const _ = require('lodash'),
responses = require('../responses'),
controller = require('../services/lists'),
{ withAuthLevel, authLevels } = require('amphora-auth');

/**
Expand All @@ -17,13 +18,24 @@ function onlyJSONLists(req, res, next) {
}
}

/**
* Modify a list, return JSON
* @param {Object} req
* @param {Object} res
*/
function patchList(req, res) {
responses.expectJSON(() => controller.patchList(req.uri, req.body, res.locals), res);
}

function routes(router) {
router.use(responses.varyWithoutExtension({varyBy: ['Accept']}));
router.all('*', responses.acceptJSONOnly);
router.all('/', responses.methodNotAllowed({allow: ['get']}));
router.get('/', responses.list());
router.all('/:name', responses.methodNotAllowed({allow: ['get', 'put']}));
router.all('/:name', responses.methodNotAllowed({allow: ['get', 'put', 'patch']}));
router.get('/:name', responses.getRouteFromDB);
router.patch('/:name', withAuthLevel(authLevels.WRITE));
router.patch('/:name', patchList);
router.use('/:name', onlyJSONLists);
router.put('/:name', withAuthLevel(authLevels.WRITE));
router.put('/:name', responses.putRouteFromDB);
Expand All @@ -32,3 +44,4 @@ function routes(router) {

module.exports = routes;
module.exports.onlyJSONLists = onlyJSONLists;
module.exports.patchList = patchList;
13 changes: 13 additions & 0 deletions lib/routes/_lists.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const _ = require('lodash'),
filename = __filename.split('/').pop().split('.').shift(),
lib = require('./' + filename),
responses = require('../responses'),
controller = require('../services/lists'),
sinon = require('sinon'),
expect = require('chai').expect;

Expand All @@ -12,12 +13,24 @@ describe(_.startCase(filename), function () {

beforeEach(function () {
sandbox = sinon.sandbox.create();

sandbox.stub(controller, 'patchList');
});

afterEach(function () {
sandbox.restore();
});

describe('patchList', function () {
const fn = lib[this.title];

it('calls expectjson and lists.patchList', function () {
fn({}, { json: item => item });

expect(controller.patchList.called).to.equal(true);
});
});

describe('onlyJSONLists', function () {
const fn = lib[this.title];

Expand Down
60 changes: 60 additions & 0 deletions lib/services/lists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
'use strict';

const _isEqual = require('lodash/isEqual');

let db = require('./db');

/**
* Appends an entry to a list and returns the updated list.
* @param {Array<Object>} list
* @param {Array<Object>} data
* @returns {Array<Object>}
*/
function addToList(list, data) {
return list.concat(data);
}

/**
* Removes an entry from a list and returns the updated list.
* @param {Array<Object>} list
* @param {Array<Object>} data
* @returns {Array<Object>}
*/
function removeFromList(list, data) {
for (const deletion of data) {
list = list.filter(entry => !_isEqual(entry, deletion));
}

return list;
}

/**
* Add or Remove an item from a list
* @param {string} uri
* @param {Array<Object>} data
* @returns {Promise}
*/
function patchList(uri, data) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be the first time I've seen PATCH implemented correctly haha

if (!Array.isArray(data.add) && !Array.isArray(data.remove)) {
throw new Error('Bad Request. List PATCH requires `add` or `remove` to be an array.');
}

return db.get(uri).then(list => {
if (Array.isArray(data.add)) {
list = addToList(list, data.add);
}

if (Array.isArray(data.remove)) {
list = removeFromList(list, data.remove);
}

// db.put wraps result in an object `{ _value: list }`, return list only
// hopefully db.put never does anything to the data bc we're just returning list
return db.put(uri, list).then(() => list);
});
}

module.exports.patchList = patchList;

// For testing
module.exports.setDb = mock => db = mock;
53 changes: 53 additions & 0 deletions lib/services/lists.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';

const _ = require('lodash'),
filename = __filename.split('/').pop().split('.').shift(),
lib = require('./' + filename),
sinon = require('sinon'),
expect = require('chai').expect,
storage = require('../../test/fixtures/mocks/storage');

describe(_.startCase(filename), function () {
let sandbox, db;

beforeEach(function () {
sandbox = sinon.sandbox.create();

db = storage();
lib.setDb(db);
});

afterEach(function () {
sandbox.restore();
});

describe('patchList', function () {
const fn = lib.patchList;

it('will throw if bad request body', function () {
try {
fn('domain.com/_lists/test', {});
} catch (e) {
expect(e.message).to.eql('Bad Request. List PATCH requires `add` or `remove` to be an array.');
}
});

it('adds to existing lists if has add property', function () {
db.get.resolves([]);
db.put.callsFake((uri, list) => Promise.resolve({ _value: list }));

return fn('domain.com/_lists/test', { add: [ 'hello' ] }).then(list => {
expect(list).to.eql([ 'hello' ]);
});
});

it('removes from existing lists if has remove property', function () {
db.get.resolves([ 'hello' ]);
db.put.callsFake((uri, list) => Promise.resolve({ _value: list }));

return fn('domain.com/_lists/test', { remove: [ 'hello' ] }).then(list => {
expect(list).to.eql([]);
});
});
});
});
38 changes: 38 additions & 0 deletions test/api/_lists/patch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';

const _ = require('lodash'),
apiAccepts = require('../../fixtures/api-accepts'),
endpointName = _.startCase(__dirname.split('/').pop()),
filename = _.startCase(__filename.split('/').pop().split('.')[0]),
sinon = require('sinon');

describe(endpointName, function () {
describe(filename, function () {
let sandbox,
hostname = 'localhost.example.com',
acceptsJsonBody = apiAccepts.acceptsJsonBody(_.camelCase(filename)),
start = ['item1', 'item2'],
data = {
add: ['item3', 'item4'],
remove: ['item1']
},
end = ['item2', 'item3', 'item4'];

beforeEach(function () {
sandbox = sinon.sandbox.create();
return apiAccepts.beforeEachTest({ sandbox, hostname, pathsAndData: {'/_lists/valid': start} });
});

afterEach(function () {
sandbox.restore();
});

describe('/_lists/:name', function () {
const path = this.title;

// overrides existing data
acceptsJsonBody(path, {name: 'valid'}, data, 200, end);
acceptsJsonBody(path, {name: 'missing'}, data, 404);
});
});
});