Skip to content

Commit 673dda9

Browse files
committed
Merge branch 'master' into next
2 parents bac90a1 + cb9a54c commit 673dda9

13 files changed

+224
-88
lines changed

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@
77
- enhance params normalization - converting from string to primitive types is now more strict and can throw ParamNormalizationError,
88
e.g. when number is expected but the invalid string (NaN) has been received (#289)
99

10+
### 0.7.3
11+
12+
- FIXED: Directly calling response bug - #286
13+
- FIXED: Missing parameter in @BodyParam error message - #284
14+
- FIXED: Sync and async auth checker bug - #283
15+
- FIXED: Handling different content-type responses in JsonController - #277
16+
- ADDED: Support for returning Buffer and streams from action handler (controller's method) - #285
17+
- ADDED: Custom driver support - #276
18+
1019
### 0.7.2
1120

1221
- FIXED: Using `@Authorization` decorator with Koa caused 404 responses (ref [#240](https://github.com/pleerock/routing-controllers/pull/240))

README.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,8 @@ export class UserController {
246246

247247
#### Using Request and Response objects
248248

249-
You can use framework's request and response objects this way:
249+
You can use framework's request and response objects directly. If you want to handle the response by yourself,
250+
just make sure you return the response object itself from the action.
250251

251252
```typescript
252253
import {Controller, Req, Res, Get} from "routing-controllers";
@@ -256,7 +257,7 @@ export class UserController {
256257
257258
@Get("/users")
258259
getAll(@Req() request: any, @Res() response: any) {
259-
response.send("Hello response!");
260+
return response.send("Hello response!");
260261
}
261262
262263
}
@@ -274,7 +275,7 @@ export class UserController {
274275
275276
@Get("/users")
276277
getAll(@Req() request: Request, @Res() response: Response) {
277-
response.send("Hello response!");
278+
return response.send("Hello response!");
278279
}
279280
280281
}

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "routing-controllers",
33
"private": true,
4-
"version": "0.8.0-beta.1",
4+
"version": "0.8.0",
55
"description": "Create structured, declarative and beautifully organized class-based controllers with heavy decorators usage for Express / Koa using TypeScript.",
66
"license": "MIT",
77
"readmeFilename": "README.md",

src/driver/express/ExpressDriver.ts

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {UseMetadata} from "../../metadata/UseMetadata";
22
import {MiddlewareMetadata} from "../../metadata/MiddlewareMetadata";
33
import {ActionMetadata} from "../../metadata/ActionMetadata";
44
import {Action} from "../../Action";
5-
import {classToPlain} from "class-transformer";
65
import {ParamMetadata} from "../../metadata/ParamMetadata";
76
import {BaseDriver} from "../BaseDriver";
87
import {ExpressMiddlewareInterface} from "./ExpressMiddlewareInterface";
@@ -12,7 +11,8 @@ import {AuthorizationCheckerNotDefinedError} from "../../error/AuthorizationChec
1211
import {isPromiseLike} from "../../util/isPromiseLike";
1312
import {getFromContainer} from "../../container";
1413
import {AuthorizationRequiredError} from "../../error/AuthorizationRequiredError";
15-
import { NotFoundError } from "../../index";
14+
import {NotFoundError} from "../../index";
15+
1616
const cookie = require("cookie");
1717
const templateUrl = require("template-url");
1818

@@ -105,7 +105,7 @@ export class ExpressDriver extends BaseDriver {
105105
const action: Action = { request, response, next };
106106
try {
107107
const checkResult = this.authorizationChecker(action, actionMetadata.authorizedRoles);
108-
108+
109109
const handleError = (result: any) => {
110110
if (!result) {
111111
let error = actionMetadata.authorizedRoles.length === 0 ? new AuthorizationRequiredError(action) : new AccessDeniedError(action);
@@ -114,7 +114,7 @@ export class ExpressDriver extends BaseDriver {
114114
next();
115115
}
116116
};
117-
117+
118118
if (isPromiseLike(checkResult)) {
119119
checkResult
120120
.then(result => handleError(result))
@@ -150,6 +150,14 @@ export class ExpressDriver extends BaseDriver {
150150
// prepare route and route handler function
151151
const route = ActionMetadata.appendBaseRoute(this.routePrefix, actionMetadata.fullRoute);
152152
const routeHandler = function routeHandler(request: any, response: any, next: Function) {
153+
// Express calls the "get" route automatically when we call the "head" route:
154+
// Reference: https://expressjs.com/en/4x/api.html#router.METHOD
155+
// This causes a double action execution on our side, which results in an unhandled rejection,
156+
// saying: "Can't set headers after they are sent".
157+
// The following line skips action processing when the request method does not match the action method.
158+
if (request.method.toLowerCase() !== actionMetadata.type)
159+
return next();
160+
153161
return executeCallback({request, response, next});
154162
};
155163

@@ -230,12 +238,21 @@ export class ExpressDriver extends BaseDriver {
230238
*/
231239
handleSuccess(result: any, action: ActionMetadata, options: Action): void {
232240

241+
// if the action returned the response object itself, short-circuits
242+
if (result && result === options.response) {
243+
options.next();
244+
return;
245+
}
246+
233247
// transform result if needed
234248
result = this.transformResult(result, action, options);
235249

236250
// set http status code
237-
if (result === undefined && action.undefinedResultCode && action.undefinedResultCode instanceof Function) {
238-
throw new (action.undefinedResultCode as any)(options);
251+
if (result === undefined && action.undefinedResultCode) {
252+
if (action.undefinedResultCode instanceof Function) {
253+
throw new (action.undefinedResultCode as any)(options);
254+
}
255+
options.response.status(action.undefinedResultCode);
239256
}
240257
else if (result === null) {
241258
if (action.nullResultCode) {
@@ -284,11 +301,18 @@ export class ExpressDriver extends BaseDriver {
284301
});
285302
}
286303
else if (result === undefined) { // throw NotFoundError on undefined response
287-
const notFoundError = new NotFoundError();
304+
288305
if (action.undefinedResultCode) {
289-
notFoundError.httpCode = action.undefinedResultCode as number;
306+
if (action.isJsonTyped) {
307+
options.response.json();
308+
} else {
309+
options.response.send();
310+
}
311+
options.next();
312+
313+
} else {
314+
throw new NotFoundError();
290315
}
291-
throw notFoundError;
292316
}
293317
else if (result === null) { // send null response
294318
if (action.isJsonTyped) {

src/driver/koa/KoaDriver.ts

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ import {BaseDriver} from "../BaseDriver";
44
import {MiddlewareMetadata} from "../../metadata/MiddlewareMetadata";
55
import {ParamMetadata} from "../../metadata/ParamMetadata";
66
import {UseMetadata} from "../../metadata/UseMetadata";
7-
import {classToPlain} from "class-transformer";
87
import {KoaMiddlewareInterface} from "./KoaMiddlewareInterface";
98
import {AuthorizationCheckerNotDefinedError} from "../../error/AuthorizationCheckerNotDefinedError";
109
import {AccessDeniedError} from "../../error/AccessDeniedError";
1110
import {isPromiseLike} from "../../util/isPromiseLike";
1211
import {getFromContainer} from "../../container";
1312
import {RoleChecker} from "../../RoleChecker";
1413
import {AuthorizationRequiredError} from "../../error/AuthorizationRequiredError";
15-
import { NotFoundError, HttpError } from "../../index";
14+
import {HttpError, NotFoundError} from "../../index";
15+
1616
const cookie = require("cookie");
1717
const templateUrl = require("template-url");
1818

@@ -81,7 +81,7 @@ export class KoaDriver extends BaseDriver {
8181
const checkResult = actionMetadata.authorizedRoles instanceof Function ?
8282
getFromContainer<RoleChecker>(actionMetadata.authorizedRoles).check(action) :
8383
this.authorizationChecker(action, actionMetadata.authorizedRoles);
84-
84+
8585
const handleError = (result: any) => {
8686
if (!result) {
8787
let error = actionMetadata.authorizedRoles.length === 0 ? new AuthorizationRequiredError(action) : new AccessDeniedError(action);
@@ -90,7 +90,7 @@ export class KoaDriver extends BaseDriver {
9090
return next();
9191
}
9292
};
93-
93+
9494
if (isPromiseLike(checkResult)) {
9595
return checkResult
9696
.then(result => handleError(result))
@@ -213,21 +213,14 @@ export class KoaDriver extends BaseDriver {
213213
*/
214214
handleSuccess(result: any, action: ActionMetadata, options: Action): void {
215215

216+
// if the action returned the context or the response object itself, short-circuits
217+
if (result && (result === options.response || result === options.context)) {
218+
return options.next();
219+
}
220+
216221
// transform result if needed
217222
result = this.transformResult(result, action, options);
218223

219-
// set http status code
220-
if (result === undefined && action.undefinedResultCode && action.undefinedResultCode instanceof Function) {
221-
throw new (action.undefinedResultCode as any)(options);
222-
}
223-
else if (result === null && action.nullResultCode) {
224-
if (action.nullResultCode instanceof Function) {
225-
throw new (action.nullResultCode as any)(options);
226-
}
227-
} else if (action.successHttpCode) {
228-
options.response.status = action.successHttpCode;
229-
}
230-
231224
if (action.redirect) { // if redirect is set then do it
232225
if (typeof result === "string") {
233226
options.response.redirect(result);
@@ -238,32 +231,24 @@ export class KoaDriver extends BaseDriver {
238231
}
239232
} else if (action.renderedTemplate) { // if template is set then render it // TODO: not working in koa
240233
const renderOptions = result && result instanceof Object ? result : {};
241-
234+
242235
this.koa.use(async function (ctx: any, next: any) {
243236
await ctx.render(action.renderedTemplate, renderOptions);
244237
});
245238
}
246239
else if (result === undefined) { // throw NotFoundError on undefined response
247-
const notFoundError = new NotFoundError();
248-
if (action.undefinedResultCode) {
249-
notFoundError.httpCode = action.undefinedResultCode as number;
240+
if (action.undefinedResultCode instanceof Function) {
241+
throw new (action.undefinedResultCode as any)(options);
242+
243+
} else if (!action.undefinedResultCode) {
244+
throw new NotFoundError();
250245
}
251-
throw notFoundError;
252246
}
253247
else if (result === null) { // send null response
254-
if (action.isJsonTyped) {
255-
options.response.body = null;
256-
} else {
257-
options.response.body = null;
258-
}
259-
260-
// Setting `null` as a `response.body` means to koa that there is no content to return
261-
// so we must reset the status codes here.
262-
if (action.nullResultCode) {
263-
options.response.status = action.nullResultCode;
264-
} else {
265-
options.response.status = 204;
266-
}
248+
if (action.nullResultCode instanceof Function)
249+
throw new (action.nullResultCode as any)(options);
250+
251+
options.response.body = null;
267252
}
268253
else if (result instanceof Uint8Array) { // check if it's binary data (typed array)
269254
options.response.body = Buffer.from(result as any);
@@ -275,7 +260,22 @@ export class KoaDriver extends BaseDriver {
275260
options.response.body = result;
276261
}
277262
}
278-
263+
264+
// set http status code
265+
if (result === undefined && action.undefinedResultCode) {
266+
console.log(action.undefinedResultCode);
267+
options.response.status = action.undefinedResultCode;
268+
}
269+
else if (result === null && action.nullResultCode) {
270+
options.response.status = action.nullResultCode;
271+
272+
} else if (action.successHttpCode) {
273+
options.response.status = action.successHttpCode;
274+
275+
} else if (options.response.body === null) {
276+
options.response.status = 204;
277+
}
278+
279279
// apply http headers
280280
Object.keys(action.headers).forEach(name => {
281281
options.response.set(name, action.headers[name]);
@@ -290,12 +290,6 @@ export class KoaDriver extends BaseDriver {
290290
handleError(error: any, action: ActionMetadata | undefined, options: Action) {
291291
return new Promise((resolve, reject) => {
292292
if (this.isDefaultErrorHandlingEnabled) {
293-
// set http status
294-
if (error instanceof HttpError && error.httpCode) {
295-
options.response.status = error.httpCode;
296-
} else {
297-
options.response.status = 500;
298-
}
299293

300294
// apply http headers
301295
if (action) {
@@ -311,6 +305,13 @@ export class KoaDriver extends BaseDriver {
311305
options.response.body = this.processTextError(error);
312306
}
313307

308+
// set http status
309+
if (error instanceof HttpError && error.httpCode) {
310+
options.response.status = error.httpCode;
311+
} else {
312+
options.response.status = 500;
313+
}
314+
314315
return resolve();
315316
}
316317
return reject(error);

src/util/importClassesFromDirectories.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export function importClassesFromDirectories(directories: string[], formats = ["
1010
allLoaded.push(exported);
1111
} else if (exported instanceof Array) {
1212
exported.forEach((i: any) => loadFileClasses(i, allLoaded));
13-
} else if (exported instanceof Object || typeof exported === 'object') {
13+
} else if (exported instanceof Object || typeof exported === "object") {
1414
Object.keys(exported).forEach(key => loadFileClasses(exported[key], allLoaded));
1515
}
1616

test/functional/action-params.spec.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {assertRequest} from "./test-utils";
66
import {User} from "../fakes/global-options/User";
77
import {Controller} from "../../src/decorator/Controller";
88
import {Get} from "../../src/decorator/Get";
9+
import {Ctx} from "../../src/decorator/Ctx";
910
import {Req} from "../../src/decorator/Req";
1011
import {Res} from "../../src/decorator/Res";
1112
import {Param} from "../../src/decorator/Param";
@@ -107,6 +108,26 @@ describe("action parameters", () => {
107108
return "<html><body>hello</body></html>";
108109
}
109110

111+
@Get("/users-direct")
112+
getUsersDirect(@Res() response: any): any {
113+
if (typeof response.send === "function")
114+
return response.status(201).contentType("custom/x-sample").send("hi, I was written directly to the response");
115+
else {
116+
response.status = 201;
117+
response.type = "custom/x-sample; charset=utf-8";
118+
response.body = "hi, I was written directly to the response";
119+
return response;
120+
}
121+
}
122+
123+
@Get("/users-direct/ctx")
124+
getUsersDirectKoa(@Ctx() ctx: any): any {
125+
ctx.response.status = 201;
126+
ctx.response.type = "custom/x-sample; charset=utf-8";
127+
ctx.response.body = "hi, I was written directly to the response using Koa Ctx";
128+
return ctx;
129+
}
130+
110131
@Get("/users/:userId")
111132
getUser(@Param("userId") userId: number) {
112133
paramUserId = userId;
@@ -383,6 +404,22 @@ describe("action parameters", () => {
383404
});
384405
});
385406

407+
describe("writing directly to the response using @Res should work", () => {
408+
assertRequest([3001, 3002], "get", "users-direct", response => {
409+
expect(response).to.be.status(201);
410+
expect(response.body).to.be.equal("hi, I was written directly to the response");
411+
expect(response).to.have.header("content-type", "custom/x-sample; charset=utf-8");
412+
});
413+
});
414+
415+
describe("writing directly to the response using @Ctx should work", () => {
416+
assertRequest([3002], "get", "users-direct/ctx", response => {
417+
expect(response).to.be.status(201);
418+
expect(response.body).to.be.equal("hi, I was written directly to the response using Koa Ctx");
419+
expect(response).to.have.header("content-type", "custom/x-sample; charset=utf-8");
420+
});
421+
});
422+
386423
describe("@Param should give a param from route", () => {
387424
assertRequest([3001, 3002], "get", "users/1", response => {
388425
expect(paramUserId).to.be.equal(1);

0 commit comments

Comments
 (0)