From ac14e8786d63a4a29aa51a481ff0ef9f09f82520 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Tue, 18 Jan 2022 13:44:27 -0500 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20support=20isNestedAt?= =?UTF-8?q?tribute=20in=20models?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../models/fragment-target-attributes.js | 9 ----- addons/api/addon/models/target.js | 13 ++++--- addons/api/addon/serializers/application.js | 36 +++++++++++++++++- .../app/models/fragment-target-attributes.js | 1 - .../api/tests/unit/serializers/target-test.js | 37 ++++++++++++++++++- ui/admin/app/components/form/target/index.hbs | 2 +- 6 files changed, 79 insertions(+), 19 deletions(-) delete mode 100644 addons/api/addon/models/fragment-target-attributes.js delete mode 100644 addons/api/app/models/fragment-target-attributes.js diff --git a/addons/api/addon/models/fragment-target-attributes.js b/addons/api/addon/models/fragment-target-attributes.js deleted file mode 100644 index 44f1a94aa..000000000 --- a/addons/api/addon/models/fragment-target-attributes.js +++ /dev/null @@ -1,9 +0,0 @@ -import Fragment from 'ember-data-model-fragments/fragment'; -import { attr } from '@ember-data/model'; - -export default class FragmentTargetAttributesModel extends Fragment { - @attr('number', { - description: 'The default port a target should use if present.', - }) - default_port; -} diff --git a/addons/api/addon/models/target.js b/addons/api/addon/models/target.js index 9b36a5749..65baaffea 100644 --- a/addons/api/addon/models/target.js +++ b/addons/api/addon/models/target.js @@ -1,15 +1,16 @@ import GeneratedTargetModel from '../generated/models/target'; -import { fragment, fragmentArray } from 'ember-data-model-fragments/attributes'; +import { attr } from '@ember-data/model'; +import { fragmentArray } from 'ember-data-model-fragments/attributes'; import { computed } from '@ember/object'; export default class TargetModel extends GeneratedTargetModel { // =attributes - /** - * Attributes of this resource, if any, represented as a JSON fragment. - * @type {FragmentTargetAttributesModel} - */ - @fragment('fragment-target-attributes', { defaultValue: {} }) attributes; + @attr('number', { + isNestedAttribute: true, + description: 'The default port a target should use if present.', + }) + default_port; /** * @type {[FragmentHostSourceModel]} diff --git a/addons/api/addon/serializers/application.js b/addons/api/addon/serializers/application.js index 08b59eebc..6223f5df8 100644 --- a/addons/api/addon/serializers/application.js +++ b/addons/api/addon/serializers/application.js @@ -50,6 +50,12 @@ export default class ApplicationSerializer extends RESTSerializer { // Do not serialize `disabled` fields. // TODO: disabled is temporarily disabled if (key === 'disabled') delete json[key]; + // Push nested attributes down into the attributes key + if (options.isNestedAttribute && json[key]) { + if (!json.attributes) json.attributes = {}; + json.attributes[key] = json[key]; + delete json[key]; + } return value; } @@ -195,9 +201,37 @@ export default class ApplicationSerializer extends RESTSerializer { * @return {Object} */ normalize(typeClass, hash) { - const normalizedHash = copy(hash, true); + let normalizedHash = copy(hash, true); const scopeID = get(normalizedHash, 'scope.id'); if (scopeID) normalizedHash.scope.scope_id = scopeID; + normalizedHash = this.normalizeNestedAttributes(typeClass, normalizedHash); return super.normalize(typeClass, normalizedHash); } + + /** + * Certain typed resources in the API have type-specific fields nested under + * an `attributes` key in the payload + * (e.g. `attributes: { fieldName: '123' }`). Since Ember Data lacks + * first-class support for nested attributes, these fields must be hoisted up + * into the main body of the payload. + * + * In order to annotate a field as being a nested attribute, add the key name + * to the `nestedAttributes` field on the serializer: + * `@attr('string', { isNestedAttribute: true }) fieldName;` + * + * @param {Model} typeClass + * @param {Object} hash + * @return {Object} + */ + normalizeNestedAttributes(typeClass, hash) { + const normalizedHash = copy(hash, true); + typeClass.attributes.forEach((attribute) => { + const { isNestedAttribute } = attribute.options; + const attributeValue = normalizedHash.attributes?.[attribute.name]; + if (isNestedAttribute && attributeValue) { + normalizedHash[attribute.name] = attributeValue; + } + }); + return normalizedHash; + } } diff --git a/addons/api/app/models/fragment-target-attributes.js b/addons/api/app/models/fragment-target-attributes.js deleted file mode 100644 index 41eb7d88f..000000000 --- a/addons/api/app/models/fragment-target-attributes.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from 'api/models/fragment-target-attributes'; diff --git a/addons/api/tests/unit/serializers/target-test.js b/addons/api/tests/unit/serializers/target-test.js index 0c1bd089c..d384e2cee 100644 --- a/addons/api/tests/unit/serializers/target-test.js +++ b/addons/api/tests/unit/serializers/target-test.js @@ -20,6 +20,7 @@ module('Unit | Serializer | target', function (hooks) { scope_id: 'org_1', type: 'org', }, + default_port: 1234, version: 1, type: 'tcp', }); @@ -35,7 +36,7 @@ module('Unit | Serializer | target', function (hooks) { session_max_seconds: 28800, session_connection_limit: 1, attributes: { - default_port: null, + default_port: 1234, }, }); }); @@ -123,6 +124,40 @@ module('Unit | Serializer | target', function (hooks) { }); }); + test('it normalizes records with nested attributes', function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('target'); + const targetModelClass = store.createRecord('target').constructor; + const payload = { + id: '1', + name: 'Target 1', + attributes: { + default_port: 1234, + }, + }; + const normalized = serializer.normalizeSingleResponse( + store, + targetModelClass, + payload + ); + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + type: 'target', + attributes: { + application_credential_source_ids: [], + authorized_actions: [], + host_sources: [], + name: 'Target 1', + default_port: 1234, + }, + relationships: {}, + }, + }); + }); + test('it normalizes missing host_sources and application_credential_source_ids to empty array', function (assert) { assert.expect(1); const store = this.owner.lookup('service:store'); diff --git a/ui/admin/app/components/form/target/index.hbs b/ui/admin/app/components/form/target/index.hbs index efdda831f..fc8757041 100644 --- a/ui/admin/app/components/form/target/index.hbs +++ b/ui/admin/app/components/form/target/index.hbs @@ -94,7 +94,7 @@ Date: Tue, 18 Jan 2022 14:32:03 -0500 Subject: [PATCH 2/3] =?UTF-8?q?chore:=20=F0=9F=A4=96=20add=20support=20for?= =?UTF-8?q?=20nested=20attributes=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/adapters/application.js | 38 +++++++++++++++++++----- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 04a9ebe6f..1c4149511 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -263,14 +263,36 @@ export default class ApplicationAdapter extends RESTAdapter.extend( source: { pointer: '/data' }, }; // Normalize field-specific errors, if any. - const errors = fieldErrors.map((error) => ({ - detail: error.description, - // Ember Data cannot handle errors on nested attributes, so these are - // flattened (e.g. `attributes.foobar` becomes `attributes_foobar`). - // To capture these errors, add read-only fields on the model - // corresponding to the underscored field name. - source: { pointer: `/data/attributes/${error.name.replace('.', '_')}` }, - })); + const errors = fieldErrors.flatMap((error) => { + const newValue = [ + { + detail: error.description, + // Ember Data cannot handle errors on nested attributes, so these are + // flattened (e.g. `attributes.foobar` becomes `attributes_foobar`). + // To capture these errors, add read-only fields on the model + // corresponding to the underscored field name. + source: { + pointer: `/data/attributes/${error.name.replace('.', '_')}`, + }, + }, + ]; + if (error.name.includes('attributes.')) { + newValue.push({ + detail: error.description, + // Going forward, nested attributes will be hoisted. Thus "attributes." + // must be stripped from the error name key. Until all models are + // transitioned away from model fragments, however, both this and the + // prior error key mappings are necessary. + source: { + pointer: `/data/attributes/${error.name.replace( + 'attributes.', + '' + )}`, + }, + }); + } + return newValue; + }); // Return a list of JSON API errors rooted under the `errors` key. return { errors: [baseError, ...errors], From 121129ffd2594ee4fb91308a03a607bd96fcc404 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 19 Jan 2022 12:16:57 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20=F0=9F=90=9B=20include=20nulls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/serializers/application.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/api/addon/serializers/application.js b/addons/api/addon/serializers/application.js index 6223f5df8..cb61f1a34 100644 --- a/addons/api/addon/serializers/application.js +++ b/addons/api/addon/serializers/application.js @@ -51,7 +51,7 @@ export default class ApplicationSerializer extends RESTSerializer { // TODO: disabled is temporarily disabled if (key === 'disabled') delete json[key]; // Push nested attributes down into the attributes key - if (options.isNestedAttribute && json[key]) { + if (options.isNestedAttribute && json[key] !== undefined) { if (!json.attributes) json.attributes = {}; json.attributes[key] = json[key]; delete json[key];