Skip to content

Commit

Permalink
fix(rest): Improve rest metadata inheritance
Browse files Browse the repository at this point in the history 
Make sure metadata inherited from base classes are cloned to avoid
pollution of base information
  • Loading branch information
@raymondfeng
raymondfeng committed Nov 16, 2017
1 parent 76a08ee commit 3f124f3
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 35 deletions.
7 changes: 4 additions & 3 deletions packages/rest/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
"@loopback/context": "^4.0.0-alpha.20",
"@loopback/core": "^4.0.0-alpha.22",
"@loopback/openapi-spec": "^4.0.0-alpha.15",
"@types/http-errors": "^1.5.34",
"@types/js-yaml": "^3.9.1",
"body": "^5.1.0",
"debug": "^3.1.0",
"http-errors": "^1.6.1",
Expand All @@ -39,7 +37,10 @@
"devDependencies": {
"@loopback/build": "^4.0.0-alpha.5",
"@loopback/openapi-spec-builder": "^4.0.0-alpha.12",
"@loopback/testlab": "^4.0.0-alpha.14"
"@loopback/testlab": "^4.0.0-alpha.14",
"@types/http-errors": "^1.5.34",
"@types/js-yaml": "^3.9.1",
"@types/lodash": "^4.14.85"
},
"files": [
"README.md",
Expand Down
87 changes: 55 additions & 32 deletions packages/rest/src/router/metadata.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// License text available at https://opensource.org/licenses/MIT

import * as assert from 'assert';
import * as _ from 'lodash';

import {Reflector} from '@loopback/context';
import {
OperationObject,
Expand All @@ -21,6 +23,17 @@ const API_SPEC_KEY = 'rest:api-spec';

// tslint:disable:no-any

function cloneDeep<T>(val: T): T {
if (val === undefined) {
return {} as T;
}
return _.cloneDeepWith(val, v => {
// Do not clone functions
if (typeof v === 'function') return v;
return undefined;
});
}

export interface ControllerSpec {
/**
* The base path on which the Controller API is served.
Expand Down Expand Up @@ -73,6 +86,20 @@ interface RestEndpoint {
target: any;
}

function getEndpoints(
target: any,
): {[property: string]: Partial<RestEndpoint>} {
let endpoints = Reflector.getOwnMetadata(ENDPOINTS_KEY, target);
if (!endpoints) {
// Clone the endpoints so that subclasses won't mutate the metadata
// in the base class
const baseEndpoints = Reflector.getMetadata(ENDPOINTS_KEY, target);
endpoints = cloneDeep(baseEndpoints);
Reflector.defineMetadata(ENDPOINTS_KEY, endpoints, target);
}
return endpoints;
}

/**
* Build the api spec from class and method level decorations
* @param constructor Controller class
Expand All @@ -86,33 +113,39 @@ function resolveControllerSpec(

if (spec) {
debug(' using class-level spec defined via @api()', spec);
spec = Object.assign({}, spec);
spec = cloneDeep(spec);
} else {
spec = {paths: {}};
}

const endpoints =
Reflector.getMetadata(ENDPOINTS_KEY, constructor.prototype) || {};
const endpoints = getEndpoints(constructor.prototype);

for (const op in endpoints) {
const endpoint = endpoints[op];
const className =
endpoint.target.constructor.name ||
constructor.name ||
'<AnonymousClass>';
const fullMethodName = `${className}.${op}`;

const {verb, path} = endpoint;
const endpointName = `${fullMethodName} (${verb} ${path})`;
const verb = endpoint.verb!;
const path = endpoint.path!;

let endpointName = '';
if (debug.enabled) {
const className =
endpoint.target.constructor.name ||
constructor.name ||
'<AnonymousClass>';
const fullMethodName = `${className}.${op}`;
endpointName = `${fullMethodName} (${verb} ${path})`;
}

let operationSpec = endpoint.spec;
if (!operationSpec) {
// The operation was defined via @operation(verb, path) with no spec
operationSpec = {
responses: {},
};
endpoint.spec = operationSpec;
}

operationSpec['x-operation-name'] = op;

if (!spec.paths[path]) {
spec.paths[path] = {};
}
Expand All @@ -123,9 +156,7 @@ function resolveControllerSpec(
}

debug(` adding ${endpointName}`, operationSpec);
spec.paths[path][verb] = Object.assign({}, operationSpec, {
'x-operation-name': op,
});
spec.paths[path][verb] = operationSpec;
}
return spec;
}
Expand All @@ -135,7 +166,7 @@ function resolveControllerSpec(
* @param constructor Controller class
*/
export function getControllerSpec(constructor: Function): ControllerSpec {
let spec = Reflector.getMetadata(API_SPEC_KEY, constructor);
let spec = Reflector.getOwnMetadata(API_SPEC_KEY, constructor);
if (!spec) {
spec = resolveControllerSpec(constructor, spec);
Reflector.defineMetadata(API_SPEC_KEY, spec, constructor);
Expand Down Expand Up @@ -222,14 +253,9 @@ export function operation(verb: string, path: string, spec?: OperationObject) {
'@operation decorator can be applied to methods only',
);

let endpoints = Object.assign(
{},
Reflector.getMetadata(ENDPOINTS_KEY, target),
);
Reflector.defineMetadata(ENDPOINTS_KEY, endpoints, target);

let endpoint: Partial<RestEndpoint> = endpoints[propertyKey];
if (!endpoint) {
const endpoints = getEndpoints(target);
let endpoint = endpoints[propertyKey];
if (!endpoint || endpoint.target !== target) {
// Add the new endpoint metadata for the method
endpoint = {verb, path, spec, target};
endpoints[propertyKey] = endpoint;
Expand Down Expand Up @@ -305,16 +331,13 @@ export function param(paramSpec: ParameterObject) {
'@param decorator can be applied to methods only',
);

let endpoints = Object.assign(
{},
Reflector.getMetadata(ENDPOINTS_KEY, target),
);
Reflector.defineMetadata(ENDPOINTS_KEY, endpoints, target);

let endpoint: Partial<RestEndpoint> = endpoints[propertyKey];
if (!endpoint) {
const endpoints = getEndpoints(target);
let endpoint = endpoints[propertyKey];
if (!endpoint || endpoint.target !== target) {
const baseEndpoint = endpoint;
// Add the new endpoint metadata for the method
endpoint = {target};
endpoint = cloneDeep(baseEndpoint);
endpoint.target = target;
endpoints[propertyKey] = endpoint;
}

Expand Down
82 changes: 82 additions & 0 deletions packages/rest/test/unit/router/metadata.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
put,
patch,
del,
param,
} from '../../..';
import {expect} from '@loopback/testlab';
import {anOpenApiSpec, anOperationSpec} from '@loopback/openapi-spec-builder';
Expand Down Expand Up @@ -272,4 +273,85 @@ describe('Routing metadata', () => {
'getChildName',
);
});

it('allows children to override parent REST operations', () => {
const operationSpec = anOperationSpec()
.withStringResponse()
.build();

class Parent {
@get('/parent-name', operationSpec)
getName() {
return 'The Parent';
}
}

class Child extends Parent {
@get('/child-name', operationSpec)
getName() {
return 'The Child';
}
}

const childSpec = getControllerSpec(Child);
const parentSpec = getControllerSpec(Parent);

expect(childSpec.paths['/child-name']['get']).to.have.property(
'x-operation-name',
'getName',
);

// The parent endpoint has been overridden
expect(childSpec.paths).to.not.have.property('/parent-name');

expect(parentSpec.paths['/parent-name']['get']).to.have.property(
'x-operation-name',
'getName',
);

// The parent endpoint should not be polluted
expect(parentSpec.paths).to.not.have.property('/child-name');
});

it('allows children to override parent REST parameters', () => {
const operationSpec = anOperationSpec()
.withStringResponse()
.build();

class Parent {
@get('/greet', operationSpec)
greet(@param.query.string('msg') msg: string) {
return `Parent: ${msg}`;
}
}

class Child extends Parent {
greet(@param.query.string('message') msg: string) {
return `Child: ${msg}`;
}
}

const childSpec = getControllerSpec(Child);
const parentSpec = getControllerSpec(Parent);

const childGreet = childSpec.paths['/greet']['get'];
expect(childGreet).to.have.property('x-operation-name', 'greet');

expect(childGreet.parameters).to.have.property('length', 1);

expect(childGreet.parameters[0]).to.containEql({
name: 'message',
in: 'query',
});

const parentGreet = parentSpec.paths['/greet']['get'];
expect(parentGreet).to.have.property('x-operation-name', 'greet');

expect(parentGreet.parameters).to.have.property('length', 1);

expect(parentGreet.parameters[0]).to.containEql({
name: 'msg',
in: 'query',
});
});
});

0 comments on commit 3f124f3

Please sign in to comment.