From 7d5bef44400925e74bcbb584202eb2577509247e Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Sat, 12 Sep 2020 10:42:23 -0400 Subject: [PATCH 1/5] =?UTF-8?q?fix:=20=F0=9F=90=9B=20add=20support=20for?= =?UTF-8?q?=20emptyArrayIfMissing=20on=20model=20attrs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/models/target.js | 5 ++- addons/api/addon/serializers/application.js | 30 +++++++++++++++++- .../unit/serializers/application-test.js | 31 +++++++++++++++++++ .../api/tests/unit/serializers/target-test.js | 31 +++++++++++++++++++ ui/core/mirage/serializers/target.js | 12 ++++--- 5 files changed, 102 insertions(+), 7 deletions(-) diff --git a/addons/api/addon/models/target.js b/addons/api/addon/models/target.js index 90b3df16b..03a3ad5d7 100644 --- a/addons/api/addon/models/target.js +++ b/addons/api/addon/models/target.js @@ -9,7 +9,10 @@ export default class TargetModel extends GeneratedTargetModel { /** * @type {[FragmentHostSetModel]} */ - @fragmentArray('fragment-host-set', {readOnly: true}) host_sets; + @fragmentArray('fragment-host-set', { + readOnly: true, + emptyArrayIfMissing: true + }) host_sets; /** * An array of resolved host set and host catalog instances. Model instances diff --git a/addons/api/addon/serializers/application.js b/addons/api/addon/serializers/application.js index 65743c707..6dd0b84bd 100644 --- a/addons/api/addon/serializers/application.js +++ b/addons/api/addon/serializers/application.js @@ -117,6 +117,7 @@ export default class ApplicationSerializer extends RESTSerializer { * This method makes this transformation to accommodate Ember Data. * * @method normalizeSingleResponse + * @override * @param {Store} store * @param {Model} primaryModelClass * @param {Object} payload @@ -125,6 +126,10 @@ export default class ApplicationSerializer extends RESTSerializer { * @return {Object} JSON-API Document */ normalizeSingleResponse(store, primaryModelClass, payload, id, requestType) { + // Copy payload (mostly to prevent mocking issues) + payload = copy(payload, true); + // Check for and normalize missing arrays + payload = this.normalizeMissingArrays(store, primaryModelClass, payload); // Setup a new payload data structure. const transformedPayload = {}; // Find the Ember-data-expected root key name. @@ -132,7 +137,7 @@ export default class ApplicationSerializer extends RESTSerializer { primaryModelClass.modelName ); // Copy the unrooted payload under the expected root key name. - transformedPayload[payloadKey] = copy(payload, true); + transformedPayload[payloadKey] = payload; // Return the result of normalizing the transformed payload. return super.normalizeSingleResponse( store, @@ -143,6 +148,29 @@ export default class ApplicationSerializer extends RESTSerializer { ); } + /** + * Resets missing array fields to an empty array if they are annotated by + * `normalizeToEmptyArray: true` in the associated model attribute + * declaration. Our API excludes arrays when they are empty from + * singular responses. + * + * @param {Store} store + * @param {Model} primaryModelClass + * @param {object} payload + * @return {object} + */ + normalizeMissingArrays(store, primaryModelClass, payload) { + const attrDefs = store._attributesDefinitionFor(primaryModelClass.modelName); + if (attrDefs) { + Object.keys(attrDefs).forEach(key => { + if (!payload[key] && attrDefs[key]?.options?.emptyArrayIfMissing) { + payload[key] = []; + } + }); + } + return payload; + } + /** * One consequence of using model fragments to represent the embedded scope * is that they cannot have `id` fields. We still need to know scope IDs, diff --git a/addons/api/tests/unit/serializers/application-test.js b/addons/api/tests/unit/serializers/application-test.js index df6dca9db..f1962477b 100644 --- a/addons/api/tests/unit/serializers/application-test.js +++ b/addons/api/tests/unit/serializers/application-test.js @@ -138,4 +138,35 @@ module('Unit | Serializer | application', function (hooks) { }, }); }); + + test('it normalizes missing arrays in single responses if annotated in model attributes', function (assert) { + assert.expect(2); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('target'); + const target = store.createRecord('target').constructor; + const payload = { + id: '1', + name: 'Target 1', + scope: { id: 'o_123' } + }; + const normalized = serializer.normalizeSingleResponse( + store, + target, + payload + ); + assert.notOk(payload.bogusArrayField); + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + type: 'target', + attributes: { + name: 'Target 1', + scope: { scope_id: 'o_123' }, + host_sets: [] + }, + relationships: {}, + }, + }); + }); }); diff --git a/addons/api/tests/unit/serializers/target-test.js b/addons/api/tests/unit/serializers/target-test.js index 62af72737..ab1a07635 100644 --- a/addons/api/tests/unit/serializers/target-test.js +++ b/addons/api/tests/unit/serializers/target-test.js @@ -94,4 +94,35 @@ module('Unit | Serializer | target', function(hooks) { }); }); + test('it normalizes missing host_sets to empty array', function (assert) { + assert.expect(2); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('target'); + const target = store.createRecord('target').constructor; + const payload = { + id: '1', + name: 'Target 1', + scope: { id: 'o_123' } + }; + const normalized = serializer.normalizeSingleResponse( + store, + target, + payload + ); + assert.notOk(payload.bogusArrayField); + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + type: 'target', + attributes: { + name: 'Target 1', + scope: { scope_id: 'o_123' }, + host_sets: [] + }, + relationships: {}, + }, + }); + }); + }); diff --git a/ui/core/mirage/serializers/target.js b/ui/core/mirage/serializers/target.js index 70850f457..2d2a0d974 100644 --- a/ui/core/mirage/serializers/target.js +++ b/ui/core/mirage/serializers/target.js @@ -13,11 +13,13 @@ export default ApplicationSerializer.extend({ _hashForModel(model) { const json = ApplicationSerializer.prototype._hashForModel.apply(this, arguments); const { hostSets } = this.schema; - json.host_sets = model.hostSetIds.map(host_set_id => { - const hostSet = hostSets.find(host_set_id); - const host_catalog_id = hostSet?.hostCatalog?.id; - return { id: host_set_id, host_catalog_id }; - }); + if (model.hostSetIds?.length) { + json.host_sets = model.hostSetIds.map(host_set_id => { + const hostSet = hostSets.find(host_set_id); + const host_catalog_id = hostSet?.hostCatalog?.id; + return { id: host_set_id, host_catalog_id }; + }); + } return json; } From 048e202343c01094dc06c96e70a43cc4aec7bcff Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Sat, 12 Sep 2020 11:20:38 -0400 Subject: [PATCH 2/5] =?UTF-8?q?fix:=20=F0=9F=90=9B=20handle=20missing=20ho?= =?UTF-8?q?st=5Fids=20array=20in=20host=20sets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/models/host-set.js | 5 ++- .../tests/unit/serializers/host-set-test.js | 32 +++++++++++++++++++ ui/core/mirage/serializers/host-set.js | 4 ++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/addons/api/addon/models/host-set.js b/addons/api/addon/models/host-set.js index ffa0f7906..018f95f64 100644 --- a/addons/api/addon/models/host-set.js +++ b/addons/api/addon/models/host-set.js @@ -9,7 +9,10 @@ export default class HostSetModel extends GeneratedHostSetModel { * Grants is read-only _most_ under normal circumstances. But grants can * be persisted via a dedicated call to `saveGrants()`. */ - @fragmentArray('fragment-string', {readOnly: true}) host_ids; + @fragmentArray('fragment-string', { + readOnly: true, + emptyArrayIfMissing: true + }) host_ids; // =methods diff --git a/addons/api/tests/unit/serializers/host-set-test.js b/addons/api/tests/unit/serializers/host-set-test.js index c10cbe915..41d5505fd 100644 --- a/addons/api/tests/unit/serializers/host-set-test.js +++ b/addons/api/tests/unit/serializers/host-set-test.js @@ -74,4 +74,36 @@ module('Unit | Serializer | host set', function(hooks) { }, }); }); + + test('it normalizes missing host_ids to empty array', function (assert) { + assert.expect(2); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('host-set'); + const hostSet = store.createRecord('host-set').constructor; + const payload = { + id: '1', + name: 'Host Set 1', + scope: { id: 'o_123' } + }; + const normalized = serializer.normalizeSingleResponse( + store, + hostSet, + payload + ); + assert.notOk(payload.bogusArrayField); + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + type: 'host-set', + attributes: { + name: 'Host Set 1', + scope: { scope_id: 'o_123' }, + host_ids: [] + }, + relationships: {}, + }, + }); + }); + }); diff --git a/ui/core/mirage/serializers/host-set.js b/ui/core/mirage/serializers/host-set.js index bd680326e..7acf251b3 100644 --- a/ui/core/mirage/serializers/host-set.js +++ b/ui/core/mirage/serializers/host-set.js @@ -12,8 +12,10 @@ export default ApplicationSerializer.extend({ _hashForModel(model) { const json = ApplicationSerializer.prototype._hashForModel.apply(this, arguments); - json.host_ids = model.hostIds; json.host_catalog_id = model.hostCatalogId; + if (model.hostIds?.length) { + json.host_ids = model.hostIds; + } return json; } From bcd46cec1f78d1aaaaed75223c3fd8f5db9c1d0c Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Sat, 12 Sep 2020 11:40:21 -0400 Subject: [PATCH 3/5] =?UTF-8?q?fix:=20=F0=9F=90=9B=20handle=20missing=20pr?= =?UTF-8?q?incipals=20array=20in=20role=20response?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/models/role.js | 5 ++- .../unit/serializers/application-test.js | 3 +- .../tests/unit/serializers/host-set-test.js | 3 +- .../api/tests/unit/serializers/role-test.js | 31 +++++++++++++++++++ .../api/tests/unit/serializers/target-test.js | 3 +- ui/core/mirage/serializers/role.js | 1 + 6 files changed, 39 insertions(+), 7 deletions(-) diff --git a/addons/api/addon/models/role.js b/addons/api/addon/models/role.js index dd7e8bb2b..83d1a99a7 100644 --- a/addons/api/addon/models/role.js +++ b/addons/api/addon/models/role.js @@ -12,7 +12,10 @@ export default class RoleModel extends GeneratedRoleModel { * see obvious. Instead, the application layer is expected to load referenced * users and groups as needed. */ - @fragmentArray('fragment-principal', {readOnly: true}) principals; + @fragmentArray('fragment-principal', { + readOnly: true, + emptyArrayIfMissing: true + }) principals; /** * Grants is read-only _most_ under normal circumstances. But grants can diff --git a/addons/api/tests/unit/serializers/application-test.js b/addons/api/tests/unit/serializers/application-test.js index f1962477b..094e0ffb3 100644 --- a/addons/api/tests/unit/serializers/application-test.js +++ b/addons/api/tests/unit/serializers/application-test.js @@ -140,7 +140,7 @@ module('Unit | Serializer | application', function (hooks) { }); test('it normalizes missing arrays in single responses if annotated in model attributes', function (assert) { - assert.expect(2); + assert.expect(1); const store = this.owner.lookup('service:store'); const serializer = store.serializerFor('target'); const target = store.createRecord('target').constructor; @@ -154,7 +154,6 @@ module('Unit | Serializer | application', function (hooks) { target, payload ); - assert.notOk(payload.bogusArrayField); assert.deepEqual(normalized, { included: [], data: { diff --git a/addons/api/tests/unit/serializers/host-set-test.js b/addons/api/tests/unit/serializers/host-set-test.js index 41d5505fd..be9d51436 100644 --- a/addons/api/tests/unit/serializers/host-set-test.js +++ b/addons/api/tests/unit/serializers/host-set-test.js @@ -76,7 +76,7 @@ module('Unit | Serializer | host set', function(hooks) { }); test('it normalizes missing host_ids to empty array', function (assert) { - assert.expect(2); + assert.expect(1); const store = this.owner.lookup('service:store'); const serializer = store.serializerFor('host-set'); const hostSet = store.createRecord('host-set').constructor; @@ -90,7 +90,6 @@ module('Unit | Serializer | host set', function(hooks) { hostSet, payload ); - assert.notOk(payload.bogusArrayField); assert.deepEqual(normalized, { included: [], data: { diff --git a/addons/api/tests/unit/serializers/role-test.js b/addons/api/tests/unit/serializers/role-test.js index 1293d28b4..7a3d6e59d 100644 --- a/addons/api/tests/unit/serializers/role-test.js +++ b/addons/api/tests/unit/serializers/role-test.js @@ -68,6 +68,37 @@ module('Unit | Serializer | role', function(hooks) { attributes: { name: 'Role 1', grants: [{value: '*'}, {value: '*'}], + principals: [] + }, + relationships: {}, + }, + }); + }); + + test('it normalizes missing principals to empty array', function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('role'); + const role = store.createRecord('role').constructor; + const payload = { + id: '1', + name: 'Role 1', + scope: { id: 'o_123' } + }; + const normalized = serializer.normalizeSingleResponse( + store, + role, + payload + ); + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + type: 'role', + attributes: { + name: 'Role 1', + scope: { scope_id: 'o_123' }, + principals: [] }, relationships: {}, }, diff --git a/addons/api/tests/unit/serializers/target-test.js b/addons/api/tests/unit/serializers/target-test.js index ab1a07635..94cab05e5 100644 --- a/addons/api/tests/unit/serializers/target-test.js +++ b/addons/api/tests/unit/serializers/target-test.js @@ -95,7 +95,7 @@ module('Unit | Serializer | target', function(hooks) { }); test('it normalizes missing host_sets to empty array', function (assert) { - assert.expect(2); + assert.expect(1); const store = this.owner.lookup('service:store'); const serializer = store.serializerFor('target'); const target = store.createRecord('target').constructor; @@ -109,7 +109,6 @@ module('Unit | Serializer | target', function(hooks) { target, payload ); - assert.notOk(payload.bogusArrayField); assert.deepEqual(normalized, { included: [], data: { diff --git a/ui/core/mirage/serializers/role.js b/ui/core/mirage/serializers/role.js index 159bdc56c..06009a6f3 100644 --- a/ui/core/mirage/serializers/role.js +++ b/ui/core/mirage/serializers/role.js @@ -16,6 +16,7 @@ export default ApplicationSerializer.extend({ type: 'group' })); json.principals = [ ...serializedUsers, ...serializedGroups ]; + if (!json.principals.length) delete json.principals; return json; } }); From 156dc08ff5c3b8a93b7c491bf8a08d47c4aa4c8a Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Sat, 12 Sep 2020 11:47:27 -0400 Subject: [PATCH 4/5] =?UTF-8?q?fix:=20=F0=9F=90=9B=20handle=20empty=20memb?= =?UTF-8?q?er=5Fids=20in=20group=20responses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/models/group.js | 6 +++- .../api/tests/unit/serializers/group-test.js | 30 +++++++++++++++++++ ui/core/mirage/serializers/group.js | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/addons/api/addon/models/group.js b/addons/api/addon/models/group.js index 8489fcd68..6b0da1b1a 100644 --- a/addons/api/addon/models/group.js +++ b/addons/api/addon/models/group.js @@ -11,7 +11,11 @@ export default class GroupModel extends GeneratedGroupModel { * Members is read-only under normal circumstances. But members can * be persisted via calls to `addMembers()` or `removeMembers()`. */ - @attr({readOnly: true, defaultValue: () => A()}) member_ids; + @attr({ + readOnly: true, + defaultValue: () => A(), + emptyArrayIfMissing: true + }) member_ids; /** * An array of resolved user instances. diff --git a/addons/api/tests/unit/serializers/group-test.js b/addons/api/tests/unit/serializers/group-test.js index 2a7da2b06..8c80a1b50 100644 --- a/addons/api/tests/unit/serializers/group-test.js +++ b/addons/api/tests/unit/serializers/group-test.js @@ -45,4 +45,34 @@ module('Unit | Serializer | group', function(hooks) { }); }); + test('it normalizes missing member_ids to empty array', function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('group'); + const group = store.createRecord('group').constructor; + const payload = { + id: '1', + name: 'Group 1', + scope: { id: 'o_123' } + }; + const normalized = serializer.normalizeSingleResponse( + store, + group, + payload + ); + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + type: 'group', + attributes: { + name: 'Group 1', + scope: { scope_id: 'o_123' }, + member_ids: [] + }, + relationships: {}, + }, + }); + }); + }); diff --git a/ui/core/mirage/serializers/group.js b/ui/core/mirage/serializers/group.js index 2854224f8..1ea1605f7 100644 --- a/ui/core/mirage/serializers/group.js +++ b/ui/core/mirage/serializers/group.js @@ -12,7 +12,7 @@ export default ApplicationSerializer.extend({ _hashForModel(model) { const json = ApplicationSerializer.prototype._hashForModel.apply(this, arguments); - json.member_ids = model.memberIds; + if (model.memberIds.length) json.member_ids = model.memberIds; return json; } From 254ec2203eba6638c83df2e51665663ed0390d98 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Sat, 12 Sep 2020 12:03:52 -0400 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20=F0=9F=90=9B=20add=20missing=20set-g?= =?UTF-8?q?rants=20method=20to=20mocks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ui/core/mirage/config.js | 24 +++++++++++++++++++----- ui/core/mirage/factories/role.js | 2 +- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index aee4a5b3b..3b9e24cce 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -109,17 +109,18 @@ export default function() { const id = idMethod.split(':')[0]; const method = idMethod.split(':')[1]; const role = roles.find(id); - const updatedAttrs = { - version: attrs.version, - userIds: role.userIds, - groupIds: role.groupIds - }; + let updatedAttrs = {}; // Principals is a combined list of users and groups, but in Mirage we've // internally modelled them as separate lists. Therefore we must check // the type by looking up the user or group first, then determine which // list to add the principal to. if (method === 'add-principals') { + updatedAttrs = { + version: attrs.version, + userIds: role.userIds, + groupIds: role.groupIds + }; attrs.principalIds.forEach(id => { const isUser = users.find(id); const isGroup = groups.find(id); @@ -134,6 +135,11 @@ export default function() { // If deleting principals, filter them out of both users and groups, // and in this case don't care about the type if (method === 'remove-principals') { + updatedAttrs = { + version: attrs.version, + userIds: role.userIds, + groupIds: role.groupIds + }; updatedAttrs.userIds = updatedAttrs.userIds.filter(id => { return !attrs.principalIds.includes(id); }); @@ -141,6 +147,14 @@ export default function() { return !attrs.principalIds.includes(id); }); } + + if (method === 'set-grants') { + updatedAttrs = { + version: attrs.version, + grant_strings: attrs.grant_strings + }; + } + return role.update(updatedAttrs); }); diff --git a/ui/core/mirage/factories/role.js b/ui/core/mirage/factories/role.js index ea5734835..87eb39092 100644 --- a/ui/core/mirage/factories/role.js +++ b/ui/core/mirage/factories/role.js @@ -4,7 +4,7 @@ import { trait } from 'ember-cli-mirage'; export default factory.extend({ // eslint-disable-next-line ember/avoid-leaking-state-in-ember-objects - grant_strings: [ + grant_strings: () => [ 'id=*;action=*', 'id=*;type=host-catalog;actions=create,read' ],