Skip to content
This repository was archived by the owner on Jun 9, 2020. It is now read-only.

Commit ee7dbdf

Browse files
authored
fix(route-generation): Correctly use controller route prefix for id generation (#167)
1 parent 86e0b2f commit ee7dbdf

5 files changed

Lines changed: 28 additions & 10 deletions

File tree

src/core/routes/GiuseppeBaseRoute.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ export type FunctionMethodDecorator = (
2222
* Route decorator. Creates a route definition that reacts to a specified request. The method needs to be specified.
2323
* Can define one or multiple middlewares that are registered for that route. Also, a route name can be specified
2424
* that creates the url for express.
25-
*
25+
*
2626
* @export
2727
* @param {HttpMethod} method The http method to use.
2828
* @param {(string | RequestHandler)} [routeOrMiddleware] Either a string that represents the url for this route, or
2929
* an optional middleware if no specific route is needed.
3030
* @param {...RequestHandler[]} middlewares Other middlewares that are used for this route.
31-
* @returns {FunctionMethodDecorator}
31+
* @returns {FunctionMethodDecorator}
3232
*/
3333
export function Route(
3434
method: HttpMethod,
@@ -50,7 +50,7 @@ export function Route(
5050
/**
5151
* Default core base route of giuseppe. Is configurable with all defined http methods of express (from the source).
5252
* Specifies it's http method via the enum {@link HttpMethod}.
53-
*
53+
*
5454
* @export
5555
* @class GiuseppeBaseRoute
5656
* @implements {RouteDefinition}
@@ -74,7 +74,7 @@ export class GiuseppeBaseRoute implements RouteDefinition {
7474
): GiuseppeRoute[] {
7575
return [
7676
{
77-
id: `${HttpMethod[this.httpMethod]}_${this.route}`,
77+
id: `${HttpMethod[this.httpMethod]}_${baseUrl}_${this.route}`,
7878
name: this.name,
7979
method: this.httpMethod,
8080
url: UrlHelper.buildUrl(baseUrl, this.route),

test/Giuseppe.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,24 @@ describe('Giuseppe', () => {
385385
expect(fn).toThrowErrorMatchingSnapshot();
386386
});
387387

388+
it('should not throw on a similar route in a different controller', () => {
389+
@Controller('a')
390+
class CtrlA {
391+
@Get('getFunc')
392+
public func(): void { }
393+
}
394+
395+
@Controller('b')
396+
class CtrlB {
397+
@Get('getFunc')
398+
public func(): void { }
399+
}
400+
401+
const fn = () => giuseppe.configureRouter('baseUrl');
402+
403+
expect(fn).not.toThrow();
404+
});
405+
388406
it('should order the urls correctly (all cases)', () => {
389407
@Controller()
390408
class Ctrl {

test/__snapshots__/Giuseppe.spec.ts.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,4 @@ Array [
5353
]
5454
`;
5555

56-
exports[`Giuseppe Core configureRouter() should throw on a duplicate route 1`] = `"A route with the ID 'get_getFunc' (method name: 'func2', url: 'baseUrl/getFunc') is already registered."`;
56+
exports[`Giuseppe Core configureRouter() should throw on a duplicate route 1`] = `"A route with the ID 'get_baseUrl_getFunc' (method name: 'func2', url: 'baseUrl/getFunc') is already registered."`;

test/core/controller/CoreController.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ describe('Core controller', () => {
106106

107107
const ctrl = Giuseppe.registrar.controller[0];
108108
const route = ctrl.createRoutes('')[0];
109-
expect(route.id).toBe('get_');
109+
expect(route.id).toBe('get__');
110110
expect(route.name).toBe('get');
111111
});
112112

@@ -127,15 +127,15 @@ describe('Core controller', () => {
127127
const routes = ctrl.createRoutes('');
128128

129129
let route = routes[0];
130-
expect(route.id).toBe('get_foobar');
130+
expect(route.id).toBe('get_api_foobar');
131131
expect(route.url).toBe('api/foobar');
132132

133133
route = routes[1];
134-
expect(route.id).toBe('get_/barfoo');
134+
expect(route.id).toBe('get_api_/barfoo');
135135
expect(route.url).toBe('api/barfoo');
136136

137137
route = routes[2];
138-
expect(route.id).toBe('get_/rootfoo');
138+
expect(route.id).toBe('get_api_/rootfoo');
139139
expect(route.url).toBe('api/rootfoo');
140140
});
141141

test/core/routes/CoreRoutes.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ describe('Core routes', () => {
211211
const route = meta.routes()[0];
212212
const generated = route.createRoutes(meta, '', [])[0];
213213

214-
expect(generated.id).toBe(`${run.name}_TheUrl`);
214+
expect(generated.id).toBe(`${run.name}__TheUrl`);
215215
});
216216

217217
it('should use the correct name', () => {

0 commit comments

Comments
 (0)