Skip to content

Commit 3f124f3

Browse files
committed
fix(rest): Improve rest metadata inheritance
Make sure metadata inherited from base classes are cloned to avoid pollution of base information
1 parent 76a08ee commit 3f124f3

File tree

3 files changed

+141
-35
lines changed

3 files changed

+141
-35
lines changed

packages/rest/package.json

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
"@loopback/context": "^4.0.0-alpha.20",
2727
"@loopback/core": "^4.0.0-alpha.22",
2828
"@loopback/openapi-spec": "^4.0.0-alpha.15",
29-
"@types/http-errors": "^1.5.34",
30-
"@types/js-yaml": "^3.9.1",
3129
"body": "^5.1.0",
3230
"debug": "^3.1.0",
3331
"http-errors": "^1.6.1",
@@ -39,7 +37,10 @@
3937
"devDependencies": {
4038
"@loopback/build": "^4.0.0-alpha.5",
4139
"@loopback/openapi-spec-builder": "^4.0.0-alpha.12",
42-
"@loopback/testlab": "^4.0.0-alpha.14"
40+
"@loopback/testlab": "^4.0.0-alpha.14",
41+
"@types/http-errors": "^1.5.34",
42+
"@types/js-yaml": "^3.9.1",
43+
"@types/lodash": "^4.14.85"
4344
},
4445
"files": [
4546
"README.md",

packages/rest/src/router/metadata.ts

Lines changed: 55 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// License text available at https://opensource.org/licenses/MIT
55

66
import * as assert from 'assert';
7+
import * as _ from 'lodash';
8+
79
import {Reflector} from '@loopback/context';
810
import {
911
OperationObject,
@@ -21,6 +23,17 @@ const API_SPEC_KEY = 'rest:api-spec';
2123

2224
// tslint:disable:no-any
2325

26+
function cloneDeep<T>(val: T): T {
27+
if (val === undefined) {
28+
return {} as T;
29+
}
30+
return _.cloneDeepWith(val, v => {
31+
// Do not clone functions
32+
if (typeof v === 'function') return v;
33+
return undefined;
34+
});
35+
}
36+
2437
export interface ControllerSpec {
2538
/**
2639
* The base path on which the Controller API is served.
@@ -73,6 +86,20 @@ interface RestEndpoint {
7386
target: any;
7487
}
7588

89+
function getEndpoints(
90+
target: any,
91+
): {[property: string]: Partial<RestEndpoint>} {
92+
let endpoints = Reflector.getOwnMetadata(ENDPOINTS_KEY, target);
93+
if (!endpoints) {
94+
// Clone the endpoints so that subclasses won't mutate the metadata
95+
// in the base class
96+
const baseEndpoints = Reflector.getMetadata(ENDPOINTS_KEY, target);
97+
endpoints = cloneDeep(baseEndpoints);
98+
Reflector.defineMetadata(ENDPOINTS_KEY, endpoints, target);
99+
}
100+
return endpoints;
101+
}
102+
76103
/**
77104
* Build the api spec from class and method level decorations
78105
* @param constructor Controller class
@@ -86,33 +113,39 @@ function resolveControllerSpec(
86113

87114
if (spec) {
88115
debug(' using class-level spec defined via @api()', spec);
89-
spec = Object.assign({}, spec);
116+
spec = cloneDeep(spec);
90117
} else {
91118
spec = {paths: {}};
92119
}
93120

94-
const endpoints =
95-
Reflector.getMetadata(ENDPOINTS_KEY, constructor.prototype) || {};
121+
const endpoints = getEndpoints(constructor.prototype);
96122

97123
for (const op in endpoints) {
98124
const endpoint = endpoints[op];
99-
const className =
100-
endpoint.target.constructor.name ||
101-
constructor.name ||
102-
'<AnonymousClass>';
103-
const fullMethodName = `${className}.${op}`;
104-
105-
const {verb, path} = endpoint;
106-
const endpointName = `${fullMethodName} (${verb} ${path})`;
125+
const verb = endpoint.verb!;
126+
const path = endpoint.path!;
127+
128+
let endpointName = '';
129+
if (debug.enabled) {
130+
const className =
131+
endpoint.target.constructor.name ||
132+
constructor.name ||
133+
'<AnonymousClass>';
134+
const fullMethodName = `${className}.${op}`;
135+
endpointName = `${fullMethodName} (${verb} ${path})`;
136+
}
107137

108138
let operationSpec = endpoint.spec;
109139
if (!operationSpec) {
110140
// The operation was defined via @operation(verb, path) with no spec
111141
operationSpec = {
112142
responses: {},
113143
};
144+
endpoint.spec = operationSpec;
114145
}
115146

147+
operationSpec['x-operation-name'] = op;
148+
116149
if (!spec.paths[path]) {
117150
spec.paths[path] = {};
118151
}
@@ -123,9 +156,7 @@ function resolveControllerSpec(
123156
}
124157

125158
debug(` adding ${endpointName}`, operationSpec);
126-
spec.paths[path][verb] = Object.assign({}, operationSpec, {
127-
'x-operation-name': op,
128-
});
159+
spec.paths[path][verb] = operationSpec;
129160
}
130161
return spec;
131162
}
@@ -135,7 +166,7 @@ function resolveControllerSpec(
135166
* @param constructor Controller class
136167
*/
137168
export function getControllerSpec(constructor: Function): ControllerSpec {
138-
let spec = Reflector.getMetadata(API_SPEC_KEY, constructor);
169+
let spec = Reflector.getOwnMetadata(API_SPEC_KEY, constructor);
139170
if (!spec) {
140171
spec = resolveControllerSpec(constructor, spec);
141172
Reflector.defineMetadata(API_SPEC_KEY, spec, constructor);
@@ -222,14 +253,9 @@ export function operation(verb: string, path: string, spec?: OperationObject) {
222253
'@operation decorator can be applied to methods only',
223254
);
224255

225-
let endpoints = Object.assign(
226-
{},
227-
Reflector.getMetadata(ENDPOINTS_KEY, target),
228-
);
229-
Reflector.defineMetadata(ENDPOINTS_KEY, endpoints, target);
230-
231-
let endpoint: Partial<RestEndpoint> = endpoints[propertyKey];
232-
if (!endpoint) {
256+
const endpoints = getEndpoints(target);
257+
let endpoint = endpoints[propertyKey];
258+
if (!endpoint || endpoint.target !== target) {
233259
// Add the new endpoint metadata for the method
234260
endpoint = {verb, path, spec, target};
235261
endpoints[propertyKey] = endpoint;
@@ -305,16 +331,13 @@ export function param(paramSpec: ParameterObject) {
305331
'@param decorator can be applied to methods only',
306332
);
307333

308-
let endpoints = Object.assign(
309-
{},
310-
Reflector.getMetadata(ENDPOINTS_KEY, target),
311-
);
312-
Reflector.defineMetadata(ENDPOINTS_KEY, endpoints, target);
313-
314-
let endpoint: Partial<RestEndpoint> = endpoints[propertyKey];
315-
if (!endpoint) {
334+
const endpoints = getEndpoints(target);
335+
let endpoint = endpoints[propertyKey];
336+
if (!endpoint || endpoint.target !== target) {
337+
const baseEndpoint = endpoint;
316338
// Add the new endpoint metadata for the method
317-
endpoint = {target};
339+
endpoint = cloneDeep(baseEndpoint);
340+
endpoint.target = target;
318341
endpoints[propertyKey] = endpoint;
319342
}
320343

packages/rest/test/unit/router/metadata.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
put,
1313
patch,
1414
del,
15+
param,
1516
} from '../../..';
1617
import {expect} from '@loopback/testlab';
1718
import {anOpenApiSpec, anOperationSpec} from '@loopback/openapi-spec-builder';
@@ -272,4 +273,85 @@ describe('Routing metadata', () => {
272273
'getChildName',
273274
);
274275
});
276+
277+
it('allows children to override parent REST operations', () => {
278+
const operationSpec = anOperationSpec()
279+
.withStringResponse()
280+
.build();
281+
282+
class Parent {
283+
@get('/parent-name', operationSpec)
284+
getName() {
285+
return 'The Parent';
286+
}
287+
}
288+
289+
class Child extends Parent {
290+
@get('/child-name', operationSpec)
291+
getName() {
292+
return 'The Child';
293+
}
294+
}
295+
296+
const childSpec = getControllerSpec(Child);
297+
const parentSpec = getControllerSpec(Parent);
298+
299+
expect(childSpec.paths['/child-name']['get']).to.have.property(
300+
'x-operation-name',
301+
'getName',
302+
);
303+
304+
// The parent endpoint has been overridden
305+
expect(childSpec.paths).to.not.have.property('/parent-name');
306+
307+
expect(parentSpec.paths['/parent-name']['get']).to.have.property(
308+
'x-operation-name',
309+
'getName',
310+
);
311+
312+
// The parent endpoint should not be polluted
313+
expect(parentSpec.paths).to.not.have.property('/child-name');
314+
});
315+
316+
it('allows children to override parent REST parameters', () => {
317+
const operationSpec = anOperationSpec()
318+
.withStringResponse()
319+
.build();
320+
321+
class Parent {
322+
@get('/greet', operationSpec)
323+
greet(@param.query.string('msg') msg: string) {
324+
return `Parent: ${msg}`;
325+
}
326+
}
327+
328+
class Child extends Parent {
329+
greet(@param.query.string('message') msg: string) {
330+
return `Child: ${msg}`;
331+
}
332+
}
333+
334+
const childSpec = getControllerSpec(Child);
335+
const parentSpec = getControllerSpec(Parent);
336+
337+
const childGreet = childSpec.paths['/greet']['get'];
338+
expect(childGreet).to.have.property('x-operation-name', 'greet');
339+
340+
expect(childGreet.parameters).to.have.property('length', 1);
341+
342+
expect(childGreet.parameters[0]).to.containEql({
343+
name: 'message',
344+
in: 'query',
345+
});
346+
347+
const parentGreet = parentSpec.paths['/greet']['get'];
348+
expect(parentGreet).to.have.property('x-operation-name', 'greet');
349+
350+
expect(parentGreet.parameters).to.have.property('length', 1);
351+
352+
expect(parentGreet.parameters[0]).to.containEql({
353+
name: 'msg',
354+
in: 'query',
355+
});
356+
});
275357
});

0 commit comments

Comments
 (0)