From 0e6ec391387571f3317de4c4830c450b22b4a95e Mon Sep 17 00:00:00 2001 From: Zachary Golba Date: Tue, 27 Sep 2016 10:48:56 -0400 Subject: [PATCH] refactor: cleanup unecessary controller props (#420) * refactor: cleanup unecessary controller props * test: test against custom routes and model-less controllers --- src/packages/controller/index.js | 74 ++---------- src/packages/router/route/index.js | 109 +++++++----------- .../router/route/params/test/params.test.js | 103 +++++++++++++++++ .../utils/get-default-collection-params.js | 26 ++--- .../params/utils/get-default-member-params.js | 22 ++-- .../route/params/utils/get-query-params.js | 17 ++- src/packages/router/route/test/route.test.js | 53 --------- 7 files changed, 189 insertions(+), 215 deletions(-) create mode 100644 src/packages/router/route/params/test/params.test.js diff --git a/src/packages/controller/index.js b/src/packages/controller/index.js index 8bfa38f0..e5d07ace 100644 --- a/src/packages/controller/index.js +++ b/src/packages/controller/index.js @@ -1,7 +1,7 @@ // @flow import { Model } from '../database'; import { getDomain } from '../server'; -import { freezeProps, deepFreezeProps } from '../freezeable'; +import { freezeProps } from '../freezeable'; import findOne from './utils/find-one'; import findMany from './utils/find-many'; @@ -170,30 +170,6 @@ class Controller { */ hasSerializer: boolean; - /** - * @property modelName - * @memberof Controller - * @instance - * @private - */ - modelName: string; - - /** - * @property attributes - * @memberof Controller - * @instance - * @private - */ - attributes: Array; - - /** - * @property relationships - * @memberof Controller - * @instance - * @private - */ - relationships: Array; - /** * @property serializer * @memberof Controller @@ -211,37 +187,13 @@ class Controller { controllers: Map; constructor({ model, namespace, serializer }: Controller$opts) { - const hasModel = Boolean(model); - const hasNamespace = Boolean(namespace); - const hasSerializer = Boolean(serializer); - let attributes = []; - let relationships = []; - - if (hasModel && hasSerializer) { - const { primaryKey, attributeNames, relationshipNames } = model; - const { attributes: serializedAttributes } = serializer; - - const serializedRelationships = [ - ...serializer.hasOne, - ...serializer.hasMany - ]; - - attributes = attributeNames.filter(attr => { - return attr === primaryKey || serializedAttributes.indexOf(attr) >= 0; - }); - - relationships = relationshipNames.filter(relationship => { - return serializedRelationships.indexOf(relationship) >= 0; - }); - - Object.freeze(attributes); - Object.freeze(relationships); - } - Object.assign(this, { model, namespace, - serializer + serializer, + hasModel: Boolean(model), + hasNamespace: Boolean(namespace), + hasSerializer: Boolean(serializer) }); freezeProps(this, true, @@ -250,22 +202,10 @@ class Controller { 'serializer' ); - Object.assign(this, { - hasModel, - hasNamespace, - hasSerializer, - attributes, - relationships, - modelName: hasModel ? model.modelName : '', - }); - - deepFreezeProps(this, false, + freezeProps(this, false, 'hasModel', 'hasNamespace', - 'hasSerializer', - 'attributes', - 'relationships', - 'modelName' + 'hasSerializer' ); } diff --git a/src/packages/router/route/index.js b/src/packages/router/route/index.js index bd9f66f3..e805dc3c 100644 --- a/src/packages/router/route/index.js +++ b/src/packages/router/route/index.js @@ -1,7 +1,7 @@ // @flow import { ID_PATTERN } from './constants'; -import { FreezeableSet } from '../../freezeable'; +import { FreezeableSet, freezeProps, deepFreezeProps } from '../../freezeable'; import { createAction } from './action'; import { paramsFor, defaultParamsFor, validateResourceId } from './params'; @@ -33,6 +33,8 @@ class Route extends FreezeableSet> { staticPath: string; + defaultParams: Object; + dynamicSegments: Array; constructor({ @@ -55,65 +57,44 @@ class Route extends FreezeableSet> { dynamicSegments }); + const staticPath = getStaticPath(path, dynamicSegments); + + const defaultParams = defaultParamsFor({ + type, + controller + }); + super(createAction(type, handler, controller)); - Object.defineProperties(this, { - type: { - value: type, - writable: false, - enumerable: true, - configurable: false - }, - - path: { - value: path, - writable: false, - enumerable: true, - configurable: false - }, - - params: { - value: params, - writable: false, - enumerable: false, - configurable: false - }, - - action: { - value: action, - writable: false, - enumerable: true, - configurable: false - }, - - method: { - value: method, - writable: false, - enumerable: true, - configurable: false - }, - - controller: { - value: controller, - writable: false, - enumerable: false, - configurable: false - }, - - dynamicSegments: { - value: dynamicSegments, - writable: false, - enumerable: false, - configurable: false - }, - - staticPath: { - value: getStaticPath(path, dynamicSegments), - writable: false, - enumerable: false, - configurable: false - } + Object.assign(this, { + type, + path, + params, + action, + method, + controller, + staticPath, + defaultParams, + dynamicSegments }); + + freezeProps(this, true, + 'type', + 'path' + ); + + freezeProps(this, false, + 'action', + 'params', + 'method', + 'controller', + 'staticPath' + ); + + deepFreezeProps(this, false, + 'defaultParams', + 'dynamicSegments' + ); } else { const { constructor: { @@ -151,15 +132,6 @@ class Route extends FreezeableSet> { }, {}); } - getDefaultParams() { - const { type, controller } = this; - - return defaultParamsFor({ - type, - controller - }); - } - async execHandlers(req: Request, res: Response): Promise { for (const handler of this) { const data = await handler(req, res); @@ -171,9 +143,10 @@ class Route extends FreezeableSet> { } async visit(req: Request, res: Response): Promise { - Object.assign(req, { - defaultParams: this.getDefaultParams(), + const { defaultParams } = this; + Object.assign(req, { + defaultParams, params: this.params.validate({ ...req.params, ...this.parseParams(req.url.pathname) diff --git a/src/packages/router/route/params/test/params.test.js b/src/packages/router/route/params/test/params.test.js new file mode 100644 index 00000000..24519960 --- /dev/null +++ b/src/packages/router/route/params/test/params.test.js @@ -0,0 +1,103 @@ +// @flow +import { expect } from 'chai'; +import { it, describe, before } from 'mocha'; + +import { defaultParamsFor } from '../index'; + +import setType from '../../../../../utils/set-type'; +import { getTestApp } from '../../../../../../test/utils/get-test-app'; + +import type Controller from '../../../../controller'; + +describe('module "router/route/params"', () => { + describe('#defaultParamsFor()', () => { + let getController; + + before(async () => { + const { controllers } = await getTestApp(); + + getController = (name: string): Controller => setType(() => { + return controllers.get(name); + }); + }); + + describe('with collection route', () => { + let params; + let controller; + + before(() => { + controller = getController('posts'); + params = defaultParamsFor({ + controller, + type: 'collection' + }); + }); + + it('contains sort', () => { + expect(params).to.include.keys('sort'); + }); + + it('contains page cursor', () => { + expect(params).to.include.keys('page'); + expect(params.page).to.include.keys('size', 'number'); + }); + + it('contains model fields', () => { + const { model, serializer: { attributes } } = controller; + + expect(params.fields).to.include.keys(model.resourceName); + expect(params.fields[model.resourceName]).to.deep.equal(attributes); + }); + }); + + describe('with member route', () => { + let params; + let controller; + + before(() => { + controller = getController('posts'); + params = defaultParamsFor({ + controller, + type: 'member' + }); + }); + + it('contains model fields', () => { + const { model, serializer: { attributes } } = controller; + + expect(params.fields).to.include.keys(model.resourceName); + expect(params.fields[model.resourceName]).to.deep.equal(attributes); + }); + }); + + describe('with custom route', () => { + let params; + + before(() => { + params = defaultParamsFor({ + type: 'custom', + controller: getController('posts') + }); + }); + + it('is an empty object literal', () => { + expect(params).to.deep.equal({}); + }); + }); + + describe('with model-less controller', () => { + let params; + + before(() => { + params = defaultParamsFor({ + type: 'custom', + controller: getController('health') + }); + }); + + it('is an empty object literal', () => { + expect(params).to.deep.equal({}); + }); + }); + }); +}); diff --git a/src/packages/router/route/params/utils/get-default-collection-params.js b/src/packages/router/route/params/utils/get-default-collection-params.js index 3c9e0fcd..b6fa6ab4 100644 --- a/src/packages/router/route/params/utils/get-default-collection-params.js +++ b/src/packages/router/route/params/utils/get-default-collection-params.js @@ -6,31 +6,31 @@ import type Controller from '../../../../controller'; */ export default function getDefaultCollectionParams({ model, - attributes, - relationships, - defaultPerPage + defaultPerPage, + serializer: { + hasOne, + hasMany, + attributes + } }: Controller): Object { return { sort: 'createdAt', filter: {}, - fields: { [model.resourceName]: attributes, - - ...relationships.reduce((include, key) => { + ...[...hasOne, ...hasMany].reduce((include, key) => { const opts = model.relationshipFor(key); - if (opts) { - return { - ...include, - [opts.model.resourceName]: [opts.model.primaryKey] - }; - } else { + if (!opts) { return include; } + + return { + ...include, + [opts.model.resourceName]: [opts.model.primaryKey] + }; }, {}) }, - page: { size: defaultPerPage, number: 1 diff --git a/src/packages/router/route/params/utils/get-default-member-params.js b/src/packages/router/route/params/utils/get-default-member-params.js index aade715a..af493d4a 100644 --- a/src/packages/router/route/params/utils/get-default-member-params.js +++ b/src/packages/router/route/params/utils/get-default-member-params.js @@ -6,24 +6,26 @@ import type Controller from '../../../../controller'; */ export default function getDefaultMemberParams({ model, - attributes, - relationships + serializer: { + hasOne, + hasMany, + attributes + } }: Controller): Object { return { fields: { [model.resourceName]: attributes, - - ...relationships.reduce((include, key) => { + ...[...hasOne, ...hasMany].reduce((include, key) => { const opts = model.relationshipFor(key); - if (opts) { - return { - ...include, - [opts.model.resourceName]: [opts.model.primaryKey] - }; - } else { + if (!opts) { return include; } + + return { + ...include, + [opts.model.resourceName]: [opts.model.primaryKey] + }; }, {}) } }; diff --git a/src/packages/router/route/params/utils/get-query-params.js b/src/packages/router/route/params/utils/get-query-params.js index 2f3e871a..f578c6b9 100644 --- a/src/packages/router/route/params/utils/get-query-params.js +++ b/src/packages/router/route/params/utils/get-query-params.js @@ -55,9 +55,14 @@ function getFilterParam({ */ function getFieldsParam({ model, - attributes, - relationships + serializer: { + hasOne, + hasMany, + attributes + } }: Controller): [string, ParameterLike] { + const relationships = [...hasOne, ...hasMany]; + return ['fields', new ParameterGroup([ [model.resourceName, new Parameter({ path: `fields.${model.resourceName}`, @@ -65,7 +70,6 @@ function getFieldsParam({ values: attributes, sanitize: true })], - ...relationships.reduce((result, relationship) => { const opts = model.relationshipFor(relationship); @@ -98,8 +102,13 @@ function getFieldsParam({ * @private */ function getIncludeParam({ - relationships + serializer: { + hasOne, + hasMany + } }: Controller): [string, ParameterLike] { + const relationships = [...hasOne, ...hasMany]; + return ['include', new Parameter({ path: 'include', type: 'array', diff --git a/src/packages/router/route/test/route.test.js b/src/packages/router/route/test/route.test.js index a68c0483..1702c622 100644 --- a/src/packages/router/route/test/route.test.js +++ b/src/packages/router/route/test/route.test.js @@ -56,58 +56,5 @@ describe('module "router/route"', () => { expect(dynamicRoute.parseParams('/posts/1/2')).to.deep.equal({ id: 1 }); }); }); - - describe('#getDefaultParams()', () => { - describe('with collection route', () => { - let params: Object; - - before(() => { - params = staticRoute.getDefaultParams(); - }); - - it('contains sort', () => { - expect(params).to.include.keys('sort'); - }); - - it('contains page cursor', () => { - expect(params).to.include.keys('page'); - expect(params.page).to.include.keys('size', 'number'); - }); - - it('contains model fields', () => { - const { controller: { attributes, model } } = staticRoute; - expect(params.fields).to.include.keys(model.resourceName); - expect(params.fields[model.resourceName]).to.deep.equal(attributes); - }); - }); - - describe('with member route', () => { - let params: Object; - - before(() => { - params = dynamicRoute.getDefaultParams(); - }); - - it('contains model fields', () => { - const { controller: { attributes, model } } = staticRoute; - expect(params.fields).to.include.keys(model.resourceName); - expect(params.fields[model.resourceName]).to.deep.equal(attributes); - }); - }); - - describe('with data route', () => { - let params: Object; - - before(() => { - params = dynamicRoute.getDefaultParams(); - }); - - it('contains model fields', () => { - const { controller: { attributes, model } } = dataRoute; - expect(params.fields).to.include.keys(model.resourceName); - expect(params.fields[model.resourceName]).to.deep.equal(attributes); - }); - }); - }); }); });