From f211a50c41fb3686fa7c5931670c5fe1662e74db Mon Sep 17 00:00:00 2001 From: groenroos Date: Sun, 20 Feb 2022 03:07:38 +0000 Subject: [PATCH] #171: Return record count from storage --- core/loadCustomTags.js | 2 +- drivers/db/Memory.js | 5 ++- hooks/sapling/user/recover.js | 4 +- hooks/sapling/user/register.js | 2 +- hooks/sapling/user/update.js | 2 +- lib/Response.js | 18 +++----- lib/Storage.js | 57 +++++++++++++++++++----- test/core/loadCustomTags.test.js | 15 ++++--- test/drivers/db/Memory.test.js | 2 +- test/lib/Response.test.js | 16 +++---- test/lib/Storage.test.js | 75 +++++++++++++++++--------------- 11 files changed, 118 insertions(+), 80 deletions(-) diff --git a/core/loadCustomTags.js b/core/loadCustomTags.js index f1045e3..4e17007 100644 --- a/core/loadCustomTags.js +++ b/core/loadCustomTags.js @@ -33,7 +33,7 @@ export default async function loadCustomTags(next) { /* Not allowed so give an empty array */ if (!allowed) { - return []; + return this.storage.formatResponse([]); } /* Request the data */ diff --git a/drivers/db/Memory.js b/drivers/db/Memory.js index 2e7a4a3..babf988 100644 --- a/drivers/db/Memory.js +++ b/drivers/db/Memory.js @@ -140,18 +140,21 @@ export default class Memory extends Interface { */ async remove(collection, conditions) { const records = this.memory[collection] || []; + let count = 0; if (Object.keys(conditions).length > 0) { for (const [index, record] of records.entries()) { if (this.isMatch(record, conditions) && this.memory[collection]) { this.memory[collection].splice(index, 1); + count++; } } } else { + count = collection in this.memory ? this.memory[collection].length : 0; this.memory[collection] = []; } - return [{ success: true }]; + return { data: true, count }; } diff --git a/hooks/sapling/user/recover.js b/hooks/sapling/user/recover.js index 35901b4..f3ab069 100644 --- a/hooks/sapling/user/recover.js +++ b/hooks/sapling/user/recover.js @@ -80,7 +80,7 @@ export default async function recover(app, request, response) { }); /* If there is no such user */ - if (user.length === 0) { + if (!user) { return new Response(app, request, response, new SaplingError({ status: '401', code: '4004', @@ -98,7 +98,7 @@ export default async function recover(app, request, response) { delete request.body.new_password; /* Update the new password and clear the key */ - const userData = await app.storage.post({ + const { data: userData } = await app.storage.post({ url: `/data/users/_id/${user._id}`, body: { password: hash[1], _salt: hash[0], _authkey: '' }, session: app.adminSession, diff --git a/hooks/sapling/user/register.js b/hooks/sapling/user/register.js index e63d38f..ad80749 100644 --- a/hooks/sapling/user/register.js +++ b/hooks/sapling/user/register.js @@ -77,7 +77,7 @@ export default async function register(app, request, response) { delete request.body.password_confirm; /* Save to the database */ - const userData = await app.storage.post({ + const { data: userData } = await app.storage.post({ url: '/data/users', session: request.session, permission: request.permission, diff --git a/hooks/sapling/user/update.js b/hooks/sapling/user/update.js index 3966e51..8a1ff0e 100644 --- a/hooks/sapling/user/update.js +++ b/hooks/sapling/user/update.js @@ -97,7 +97,7 @@ export default async function update(app, request, response) { } /* Send to the database */ - const userData = await app.storage.post({ + const { data: userData } = await app.storage.post({ url: `/data/users/_id/${user._id}`, body: request.body, session: request.session, diff --git a/lib/Response.js b/lib/Response.js index 4d2e35e..5916f20 100644 --- a/lib/Response.js +++ b/lib/Response.js @@ -122,19 +122,11 @@ export default class Response { /** * Return a string for number of records found/affected in an array. * - * @param {any} data Data to be analysed + * @param {any} response Response to be analysed */ - getRecordsFound(data) { - /* Coerce an object into an array */ - if (isobject(data)) { - data = [data]; - } - - if (Array.isArray(data)) { - return data.length + (data.length === 1 ? ' record ' : ' records ') + (this.request.method === 'GET' ? 'found' : 'affected'); - } - - return ''; + getRecordsFound(response) { + const count = (this.request.query.single || !('count' in response)) ? 1 : response.count; + return count + (count === 1 ? ' record ' : ' records ') + (this.request.method === 'GET' ? 'found' : 'affected'); } @@ -193,7 +185,7 @@ export default class Response { { request: this.request.method + ' ' + this.request.originalUrl, status: this.getRecordsFound(this.content), - data: this.convertArrayToTables(this.content), + data: this.convertArrayToTables(this.content.data), date: new Date(), }, )); diff --git a/lib/Storage.js b/lib/Storage.js index c4c6390..4bac61d 100755 --- a/lib/Storage.js +++ b/lib/Storage.js @@ -205,6 +205,41 @@ export default class Storage { } + /** + * Format the response from the database driver to be uniform + * + * @param {any} response Response from DB driver + * @returns {object} Formatted response + */ + formatResponse(response) { + const formattedResponse = { + data: [], + count: 0, + }; + + if (typeof response === 'boolean' || typeof response.data === 'boolean') { + /* If it's a data-less boolean response (i.e. deleting a record) */ + delete formattedResponse.data; + formattedResponse.count = 'count' in response ? response.count : 1; + formattedResponse.success = true; + } else if (isobject(response)) { + /* Format the object with some guesswork */ + formattedResponse.data = 'data' in response ? response.data : response; + formattedResponse.count = 'count' in response ? response.count : formattedResponse.data.length; + } else if (Array.isArray(response)) { + /* Assume the array is array of records */ + formattedResponse.data = response; + formattedResponse.count = response.length; + } else { + /* Fallback */ + formattedResponse.data = response; + formattedResponse.count = 1; + } + + return formattedResponse; + } + + /** * Serve an incoming GET request from the database * @@ -282,7 +317,7 @@ export default class Storage { /* Get it from the database */ try { - const array = await this.db.read(request.collection, conditions, options, references); + const array = this.formatResponse(await this.db.read(request.collection, conditions, options, references)); const rules = this.getRules(request.collection); /* Get the list of fields we should not be able to see */ @@ -292,24 +327,24 @@ export default class Storage { const ownerFields = this.app.user.ownerFields(rules); /* Process fields against both lists */ - for (let i = 0; i < array.length; ++i) { + for (let i = 0; i < array.data.length; ++i) { /* Omit fields from the disallowedFields */ - array[i] = _.omit(array[i], omit); + array.data[i] = _.omit(array.data[i], omit); /* Check for ownership */ - const owner = array[i]._creator || array[i]._id; + const owner = array.data[i]._creator || array.data[i]._id; if (role !== 'admin' && (!request.isLogged || owner !== request.session.user._id)) { for (const ownerField of ownerFields) { - delete array[i][ownerField]; + delete array.data[i][ownerField]; } } } - /* If we only have a single result, return it bare */ + /* If we only want a single result, return it bare */ /* Otherwise, an array */ - if (request.query.single && array.length > 0) { - return array[0]; + if (request.query.single) { + return array.data.length > 0 ? array.data[0] : false; } return array; @@ -406,7 +441,7 @@ export default class Storage { /* Send to the database */ try { - return await this.db.modify(request.collection, conditions, data); + return this.formatResponse(await this.db.modify(request.collection, conditions, data)); } catch (error) { return new Response(this.app, request, response, new SaplingError(error)); } @@ -423,7 +458,7 @@ export default class Storage { /* Send to the database */ try { - return await this.db.write(request.collection, data); + return this.formatResponse(await this.db.write(request.collection, data)); } catch (error) { return new Response(this.app, request, response, new SaplingError(error)); } @@ -451,6 +486,6 @@ export default class Storage { conditions = _.extend(conditions, this.app.request.getConstraints(request), this.app.request.getCreatorConstraint(request, role)); /* Send it to the database */ - return await this.db.remove(request.collection, conditions); + return this.formatResponse(await this.db.remove(request.collection, conditions)); } } diff --git a/test/core/loadCustomTags.test.js b/test/core/loadCustomTags.test.js index 2bf1463..56927a5 100644 --- a/test/core/loadCustomTags.test.js +++ b/test/core/loadCustomTags.test.js @@ -50,7 +50,8 @@ test('get tag fetches data', async t => { t.context.app.templating.renderer.registerTags = async (tags) => { const response = await tags.get.call(t.context.app, '/data/posts'); - t.is(response.length, 2); + t.is(response.count, 2); + t.is(response.data.length, 2); }; await loadCustomTags.call(t.context.app); @@ -67,7 +68,8 @@ test('get tag fetches data with given role', async t => { t.context.app.templating.renderer.registerTags = async (tags) => { const response = await tags.get.call(t.context.app, '/data/posts', 'admin'); - t.is(response.length, 2); + t.is(response.count, 2); + t.is(response.data.length, 2); }; await loadCustomTags.call(t.context.app); @@ -92,7 +94,8 @@ test('get tag fetches data with session role', async t => { t.context.app.templating.renderer.registerTags = async (tags) => { const response = await tags.get.call(t.context.app, '/data/posts'); - t.is(response.length, 2); + t.is(response.count, 2); + t.is(response.data.length, 2); }; await loadCustomTags.call(t.context.app); @@ -109,7 +112,8 @@ test('get tag returns empty data with insufficient given role', async t => { t.context.app.templating.renderer.registerTags = async (tags) => { const response = await tags.get.call(t.context.app, '/data/posts', 'member'); - t.is(response.length, 0); + t.is(response.count, 0); + t.is(response.data.length, 0); }; await loadCustomTags.call(t.context.app); @@ -134,7 +138,8 @@ test('get tag returns empty data with insufficient session role', async t => { t.context.app.templating.renderer.registerTags = async (tags) => { const response = await tags.get.call(t.context.app, '/data/posts'); - t.is(response.length, 0); + t.is(response.count, 0); + t.is(response.data.length, 0); }; await loadCustomTags.call(t.context.app); diff --git a/test/drivers/db/Memory.test.js b/test/drivers/db/Memory.test.js index 00f33fb..dbb7205 100644 --- a/test/drivers/db/Memory.test.js +++ b/test/drivers/db/Memory.test.js @@ -286,5 +286,5 @@ test.serial('deletes all records', async t => { test.serial('deletes nothing in a non-existent collection', async t => { const result = await t.context.memory.remove('fourth', {}); - t.true(result[0].success); + t.true(result.data); }); diff --git a/test/lib/Response.test.js b/test/lib/Response.test.js index 4e79987..a15026e 100644 --- a/test/lib/Response.test.js +++ b/test/lib/Response.test.js @@ -81,40 +81,36 @@ test('returns the proper HTML string for a number value', t => { /* getRecordsFound */ test('returns appropriate label for 1 record found', t => { - t.is((new Response(t.context.app, t.context.request)).getRecordsFound([{foo:'bar'}]), '1 record found'); + t.is((new Response(t.context.app, t.context.request)).getRecordsFound({data:[{foo:'bar'}],count:1}), '1 record found'); }); test('returns appropriate label for multiple records found', t => { - t.is((new Response(t.context.app, t.context.request)).getRecordsFound([{foo:'bar'},{foo:'bar'}]), '2 records found'); + t.is((new Response(t.context.app, t.context.request)).getRecordsFound({data:[{foo:'bar'},{foo:'bar'}],count:2}), '2 records found'); }); test('returns appropriate label for no records found', t => { - t.is((new Response(t.context.app, t.context.request)).getRecordsFound([]), '0 records found'); + t.is((new Response(t.context.app, t.context.request)).getRecordsFound({data:[],count:0}), '0 records found'); }); test('returns appropriate label for 1 record affected', t => { t.context.request.method = 'POST'; - t.is((new Response(t.context.app, t.context.request)).getRecordsFound([{foo:'bar'}]), '1 record affected'); + t.is((new Response(t.context.app, t.context.request)).getRecordsFound({data:[{foo:'bar'}],count:1}), '1 record affected'); }); test('returns appropriate label for multiple records affected', t => { t.context.request.method = 'POST'; - t.is((new Response(t.context.app, t.context.request)).getRecordsFound([{foo:'bar'},{foo:'bar'}]), '2 records affected'); + t.is((new Response(t.context.app, t.context.request)).getRecordsFound({data:[{foo:'bar'},{foo:'bar'}],count:2}), '2 records affected'); }); test('returns appropriate label for no records affected', t => { t.context.request.method = 'POST'; - t.is((new Response(t.context.app, t.context.request)).getRecordsFound([]), '0 records affected'); + t.is((new Response(t.context.app, t.context.request)).getRecordsFound({data:[],count:0}), '0 records affected'); }); test('returns appropriate label for a single record', t => { t.is((new Response(t.context.app, t.context.request)).getRecordsFound({foo:'bar'}), '1 record found'); }); -test('returns empty string for bad data', t => { - t.is((new Response(t.context.app, t.context.request)).getRecordsFound('bar'), ''); -}); - /* viewResponse */ diff --git a/test/lib/Storage.test.js b/test/lib/Storage.test.js index 7b5b4b9..e7c7620 100644 --- a/test/lib/Storage.test.js +++ b/test/lib/Storage.test.js @@ -183,11 +183,12 @@ test.serial('gets multiple records', async t => { session: { role: 'member' } }); - t.is(response.length, 2); - t.is(response[0].title, 'Hello'); - t.true('_id' in response[0]); - t.is(response[1].title, 'Hi'); - t.true('_id' in response[1]); + t.is(response.count, 2); + t.is(response.data.length, 2); + t.is(response.data[0].title, 'Hello'); + t.true('_id' in response.data[0]); + t.is(response.data[1].title, 'Hi'); + t.true('_id' in response.data[1]); }); test.serial('gets a single record', async t => { @@ -203,9 +204,10 @@ test.serial('gets a single record', async t => { session: { user: { role: 'member' } } }); - t.is(response.length, 1); - t.is(response[0].title, 'Hi'); - t.true('_id' in response[0]); + t.is(response.count, 1); + t.is(response.data.length, 1); + t.is(response.data[0].title, 'Hi'); + t.true('_id' in response.data[0]); }); @@ -222,11 +224,12 @@ test.serial('posts a record', async t => { } }); - t.is(response.length, 1); - t.is(response[0].title, 'Hello'); - t.is(response[0].body, 'This is a post'); - t.true('_id' in response[0]); - t.true('_created' in response[0]); + t.is(response.count, 1); + t.is(response.data.length, 1); + t.is(response.data[0].title, 'Hello'); + t.is(response.data[0].body, 'This is a post'); + t.true('_id' in response.data[0]); + t.true('_created' in response.data[0]); }); test.serial('attaches creator details', async t => { @@ -249,11 +252,12 @@ test.serial('attaches creator details', async t => { } }); - t.is(response.length, 1); - t.is(response[0].title, 'Howdy'); - t.is(response[0]._creator, '123'); - t.is(response[0]._creatorEmail, 'foo@example.com'); - t.true('_id' in response[0]); + t.is(response.count, 1); + t.is(response.data.length, 1); + t.is(response.data[0].title, 'Howdy'); + t.is(response.data[0]._creator, '123'); + t.is(response.data[0]._creatorEmail, 'foo@example.com'); + t.true('_id' in response.data[0]); }); test.serial('modifies a record', async t => { @@ -270,9 +274,10 @@ test.serial('modifies a record', async t => { } }); - t.is(response.length, 1); - t.is(response[0].title, 'Howdy'); - t.true('_id' in response[0]); + t.is(response.count, 1); + t.is(response.data.length, 1); + t.is(response.data[0].title, 'Howdy'); + t.true('_id' in response.data[0]); }); test.serial('attaches updator details', async t => { @@ -310,13 +315,14 @@ test.serial('attaches updator details', async t => { } }); - t.is(response.length, 1); - t.is(response[0].title, 'Hello'); - t.is(response[0]._creator, '123'); - t.is(response[0]._creatorEmail, 'foo@example.com'); - t.is(response[0]._lastUpdator, '345'); - t.is(response[0]._lastUpdatorEmail, 'bar@example.com'); - t.true('_id' in response[0]); + t.is(response.count, 1); + t.is(response.data.length, 1); + t.is(response.data[0].title, 'Hello'); + t.is(response.data[0]._creator, '123'); + t.is(response.data[0]._creatorEmail, 'foo@example.com'); + t.is(response.data[0]._lastUpdator, '345'); + t.is(response.data[0]._lastUpdatorEmail, 'bar@example.com'); + t.true('_id' in response.data[0]); }); test.serial('formats specific fields correctly', async t => { @@ -332,11 +338,12 @@ test.serial('formats specific fields correctly', async t => { } }); - t.is(response.length, 1); - t.is(response[0].posted, 823910400000); - t.is(typeof response[0].posted, 'number'); - t.is(response[0].published, true); - t.is(typeof response[0].published, 'boolean'); + t.is(response.count, 1); + t.is(response.data.length, 1); + t.is(response.data[0].posted, 823910400000); + t.is(typeof response.data[0].posted, 'number'); + t.is(response.data[0].published, true); + t.is(typeof response.data[0].published, 'boolean'); }); test.serial('handles request with files if file uploads are configured', async t => { @@ -432,5 +439,5 @@ test.serial('deletes a record', async t => { url: '/data/posts/title/Hi' }); - t.deepEqual(response, [ { success: true } ]); + t.true(response.success); });