From 0a734d4a87b365c75bd6dcde30e05815d8d17525 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Mon, 19 Mar 2018 12:21:21 +0000 Subject: [PATCH 1/4] Use a custom Serializer method to solve repetition 1. Override normalizeResponse in the application serializer 2. Create custom normalizePayload to be overridden by individual serializers --- app/adapters/kv.js | 11 ++++--- app/adapters/node.js | 6 ++-- app/adapters/service.js | 8 ++++-- app/serializers/acl.js | 52 ++++++---------------------------- app/serializers/application.js | 17 ++++++++++- app/serializers/dc.js | 18 +++++------- app/serializers/kv.js | 49 ++++++-------------------------- app/serializers/session.js | 27 ++++-------------- 8 files changed, 61 insertions(+), 127 deletions(-) diff --git a/app/adapters/kv.js b/app/adapters/kv.js index 5326ea52e6ca..926664a2b276 100644 --- a/app/adapters/kv.js +++ b/app/adapters/kv.js @@ -10,6 +10,9 @@ const makeAttrable = function(obj) { const keyToArray = function(key) { return (key === '/' ? '' : key).split('/'); }; +const PRIMARY_KEY = 'Key'; +const DATACENTER_KEY = 'Datacenter'; + export default Adapter.extend({ urlForQuery: function(query, modelName) { const parts = keyToArray(query.key); @@ -26,7 +29,7 @@ export default Adapter.extend({ }, urlForDeleteRecord: function(id, modelName, snapshot) { const query = { - dc: snapshot.attr('Datacenter'), + dc: snapshot.attr(DATACENTER_KEY), }; if (isFolder(id)) { query.recurse = null; @@ -35,7 +38,7 @@ export default Adapter.extend({ }, urlForCreateRecord: function(modelName, snapshot) { return this.appendURL('kv', keyToArray(snapshot.attr('Key')), { - dc: snapshot.attr('Datacenter'), + dc: snapshot.attr(DATACENTER_KEY), }); }, urlForUpdateRecord: function(id, modelName, snapshot) { @@ -58,11 +61,11 @@ export default Adapter.extend({ // isBoolean? should error on false const url = requestData.url.split('?')[0]; const kv = { - Key: url + [PRIMARY_KEY]: url .split('/') .splice(3) .join('/'), - Datacenter: '', + DATACENTER_KEY: '', }; // TODO: separator? // safest way to check this is a create? if (this.urlForCreateRecord(null, makeAttrable(kv)).split('?')[0] === url) { diff --git a/app/adapters/node.js b/app/adapters/node.js index 70f799ca86be..3f71734e0d7e 100644 --- a/app/adapters/node.js +++ b/app/adapters/node.js @@ -8,7 +8,7 @@ export default Adapter.extend({ delete query.id; return this.appendURL('internal/ui/node', [id]); }, - handleResponse: function(status, headers, payload, requestData) { - return this._super(status, headers, { nodes: payload }, requestData); - }, + // handleResponse: function(status, headers, payload, requestData) { + // return this._super(status, headers, { nodes: payload }, requestData); + // }, }); diff --git a/app/adapters/service.js b/app/adapters/service.js index fbf42df446b0..2e4c2855d81e 100644 --- a/app/adapters/service.js +++ b/app/adapters/service.js @@ -1,5 +1,6 @@ import Adapter from './application'; import { assign } from '@ember/polyfills'; +const PRIMARY_KEY = 'Id'; export default Adapter.extend({ urlForQuery: function(query, modelName) { return this.appendURL('internal/ui/services'); @@ -21,17 +22,18 @@ export default Adapter.extend({ const parts = requestData.url.split('/'); if (this.isQueryRecord(parts)) { response = { - Id: parts.pop(), + [PRIMARY_KEY]: parts.pop(), Nodes: response, }; } else { // isQuery response = response.map(function(item, i, arr) { return assign({}, item, { - Id: item.Name, + [PRIMARY_KEY]: item.Name, }); }); } - return this._super(status, headers, { services: response }, requestData); + return this._super(status, headers, response, requestData); + // return this._super(status, headers, {services: response}, requestData); }, }); diff --git a/app/serializers/acl.js b/app/serializers/acl.js index bbd997a21d15..d244e83a7fcb 100644 --- a/app/serializers/acl.js +++ b/app/serializers/acl.js @@ -2,49 +2,13 @@ import Serializer from './application'; import { typeOf } from '@ember/utils'; export default Serializer.extend({ primaryKey: 'ID', - normalizeQueryResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload, - }, - id, - requestType - ); - }, - normalizeQueryRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: typeOf(payload) === 'array' ? payload[0] : payload, - }, - id, - requestType - ); - }, - normalizeUpdateRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this.normalizeQueryResponse(...arguments); - }, - normalizeDeleteRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this.normalizeQueryResponse( - store, - primaryModelClass, - { [this.get('primaryKey')]: id }, - id, - requestType - ); - }, - normalizeCreateRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload, - }, - id, - requestType - ); + normalizePayload: function(payload, id, requestType) { + switch (requestType) { + case 'deleteRecord': + return { [this.get('primaryKey')]: id }; + case 'queryRecord': + return typeOf(payload) === 'array' ? payload[0] : payload; + } + return this._super(...arguments); }, }); diff --git a/app/serializers/application.js b/app/serializers/application.js index 73ccc8454e84..790dcd45a213 100644 --- a/app/serializers/application.js +++ b/app/serializers/application.js @@ -1,3 +1,18 @@ import Serializer from 'ember-data/serializers/rest'; -export default Serializer.extend({}); +export default Serializer.extend({ + normalizeResponse: function(store, primaryModelClass, payload, id, requestType) { + return this._super( + store, + primaryModelClass, + { + [primaryModelClass.modelName]: this.normalizePayload(payload, id, requestType), + }, + id, + requestType + ); + }, + normalizePayload: function(payload, id, requestType) { + return payload; + }, +}); diff --git a/app/serializers/dc.js b/app/serializers/dc.js index c3e29df299f1..819f8d62ffa7 100644 --- a/app/serializers/dc.js +++ b/app/serializers/dc.js @@ -1,19 +1,15 @@ import Serializer from './application'; export default Serializer.extend({ primaryKey: 'Name', - normalizeFindAllResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload.map(item => { + normalizePayload: function(payload, id, requestType) { + switch (requestType) { + case 'findAll': + return payload.map(item => { return { [this.get('primaryKey')]: item, }; - }), - }, - id, - requestType - ); + }); + } + return this._super(...arguments); }, }); diff --git a/app/serializers/kv.js b/app/serializers/kv.js index 4c54600a4efd..046662326053 100644 --- a/app/serializers/kv.js +++ b/app/serializers/kv.js @@ -1,48 +1,17 @@ import Serializer from './application'; - export default Serializer.extend({ primaryKey: 'Key', - normalizeQueryResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload.map(function(item, i, arr) { + normalizePayload: function(payload, id, requestType) { + switch (requestType) { + case 'query': + return payload.map(item => { return { Key: item, }; - }), - }, - id, - requestType - ); - }, - normalizeCreateRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload, - }, - id, - requestType - ); - }, - normalizeUpdateRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this.normalizeCreateRecordResponse(...arguments); - }, - normalizeDeleteRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this.normalizeCreateRecordResponse(...arguments); - }, - normalizeQueryRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload[0], - }, - id, - requestType - ); + }); + case 'queryRecord': + return payload[0]; + } + return this._super(...arguments); }, }); diff --git a/app/serializers/session.js b/app/serializers/session.js index 60c873cbabd2..d78f1cb5fe4c 100644 --- a/app/serializers/session.js +++ b/app/serializers/session.js @@ -2,26 +2,11 @@ import Serializer from './application'; export default Serializer.extend({ primaryKey: 'ID', - normalizeQueryResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload, - }, - id, - requestType - ); - }, - normalizeQueryRecordResponse: function(store, primaryModelClass, payload, id, requestType) { - return this._super( - store, - primaryModelClass, - { - [primaryModelClass.modelName]: payload[0], - }, - id, - requestType - ); + normalizePayload: function(payload, id, requestType) { + switch (requestType) { + case 'queryRecord': + return payload[0]; + } + return this._super(...arguments); }, }); From a765184889213fe1b0e7af24cc5a6e6ee853a83b Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 20 Mar 2018 11:40:57 +0000 Subject: [PATCH 2/4] Breaking test for DATACENTER_KEY usage --- tests/unit/adapters/kv-test.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/unit/adapters/kv-test.js b/tests/unit/adapters/kv-test.js index 8d5f38b412cf..ccdcda8b2332 100644 --- a/tests/unit/adapters/kv-test.js +++ b/tests/unit/adapters/kv-test.js @@ -6,7 +6,21 @@ module('Unit | Adapter | kv', function(hooks) { // Replace this with your real tests. test('it exists', function(assert) { - let adapter = this.owner.lookup('adapter:kv'); + const adapter = this.owner.lookup('adapter:kv'); assert.ok(adapter); }); + test('handleResponse returns a Kv-like object when the request is a createRecord', function(assert) { + const adapter = this.owner.lookup('adapter:kv'); + // unflake, this is also going through _super and + // using urlForCreateRecord and makeAttrable so not the unit + const expected = 'key/name'; + const actual = adapter.handleResponse(200, {}, true, { + method: 'GET', + url: `/v1/kv/${expected}`, + }); + assert.deepEqual(actual, { + Key: expected, + Datacenter: '', + }); + }); }); From ee6661ca3e469f82c97e2108d9441ab5fe2513b4 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 20 Mar 2018 11:42:41 +0000 Subject: [PATCH 3/4] Add square brackets around DATACENTER_KEY --- app/adapters/kv.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/adapters/kv.js b/app/adapters/kv.js index d8d91e1af6b4..3133f8c4aa22 100644 --- a/app/adapters/kv.js +++ b/app/adapters/kv.js @@ -93,7 +93,7 @@ export default Adapter.extend({ .split('/') .splice(3) .join('/'), - DATACENTER_KEY: '', + [DATACENTER_KEY]: '', }; // TODO: separator? // safest way to check this is a create? if (this.urlForCreateRecord(null, makeAttrable(kv)).split('?')[0] === url) { From 52edc11a182205c1109cbfa5abbd9e74c6f1bbd4 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Thu, 22 Mar 2018 12:21:58 +0000 Subject: [PATCH 4/4] Make sure the returns are consistent in normalizePayload, also Add some todo's in to remind me to think consider this further at a later date. For example, is normalizePayload to be a hook or an overridable method --- app/serializers/acl.js | 2 +- app/serializers/application.js | 4 ++++ app/serializers/dc.js | 2 +- app/serializers/kv.js | 2 +- app/serializers/session.js | 2 +- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/serializers/acl.js b/app/serializers/acl.js index d244e83a7fcb..62de4ac76624 100644 --- a/app/serializers/acl.js +++ b/app/serializers/acl.js @@ -9,6 +9,6 @@ export default Serializer.extend({ case 'queryRecord': return typeOf(payload) === 'array' ? payload[0] : payload; } - return this._super(...arguments); + return payload; }, }); diff --git a/app/serializers/application.js b/app/serializers/application.js index 790dcd45a213..94653e78dd1a 100644 --- a/app/serializers/application.js +++ b/app/serializers/application.js @@ -1,6 +1,9 @@ import Serializer from 'ember-data/serializers/rest'; export default Serializer.extend({ + // this could get confusing if you tried to override + // say `normalizeQueryResponse` + // TODO: consider creating a method for each one of the `normalize...Response` family normalizeResponse: function(store, primaryModelClass, payload, id, requestType) { return this._super( store, @@ -12,6 +15,7 @@ export default Serializer.extend({ requestType ); }, + // TODO: decide on whether this is a 'hook' or an 'overridable method' normalizePayload: function(payload, id, requestType) { return payload; }, diff --git a/app/serializers/dc.js b/app/serializers/dc.js index 819f8d62ffa7..28cd44eb9c44 100644 --- a/app/serializers/dc.js +++ b/app/serializers/dc.js @@ -10,6 +10,6 @@ export default Serializer.extend({ }; }); } - return this._super(...arguments); + return payload; }, }); diff --git a/app/serializers/kv.js b/app/serializers/kv.js index 046662326053..fce5184eb674 100644 --- a/app/serializers/kv.js +++ b/app/serializers/kv.js @@ -12,6 +12,6 @@ export default Serializer.extend({ case 'queryRecord': return payload[0]; } - return this._super(...arguments); + return payload; }, }); diff --git a/app/serializers/session.js b/app/serializers/session.js index d78f1cb5fe4c..6db937efe41f 100644 --- a/app/serializers/session.js +++ b/app/serializers/session.js @@ -7,6 +7,6 @@ export default Serializer.extend({ case 'queryRecord': return payload[0]; } - return this._super(...arguments); + return payload; }, });