Skip to content

Commit cb9a54c

Browse files
author
Umed Khudoiberdiev
committed
fixed bugs with undefined result code behaviour
1 parent 06c49d4 commit cb9a54c

File tree

7 files changed

+55
-70
lines changed

7 files changed

+55
-70
lines changed

‎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.7.5",
4+
"version": "0.7.6",
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: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,11 @@ export class ExpressDriver extends BaseDriver {
248248
result = this.transformResult(result, action, options);
249249

250250
// set http status code
251-
if (result === undefined && action.undefinedResultCode && action.undefinedResultCode instanceof Function) {
252-
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);
253256
}
254257
else if (result === null) {
255258
if (action.nullResultCode) {
@@ -300,8 +303,6 @@ export class ExpressDriver extends BaseDriver {
300303
else if (result === undefined) { // throw NotFoundError on undefined response
301304

302305
if (action.undefinedResultCode) {
303-
options.response.status(action.undefinedResultCode);
304-
305306
if (action.isJsonTyped) {
306307
options.response.json();
307308
} else {

‎src/driver/koa/KoaDriver.ts

Lines changed: 29 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -221,18 +221,6 @@ export class KoaDriver extends BaseDriver {
221221
// transform result if needed
222222
result = this.transformResult(result, action, options);
223223

224-
// set http status code
225-
if (result === undefined && action.undefinedResultCode && action.undefinedResultCode instanceof Function) {
226-
throw new (action.undefinedResultCode as any)(options);
227-
}
228-
else if (result === null && action.nullResultCode) {
229-
if (action.nullResultCode instanceof Function) {
230-
throw new (action.nullResultCode as any)(options);
231-
}
232-
} else if (action.successHttpCode) {
233-
options.response.status = action.successHttpCode;
234-
}
235-
236224
if (action.redirect) { // if redirect is set then do it
237225
if (typeof result === "string") {
238226
options.response.redirect(result);
@@ -249,34 +237,18 @@ export class KoaDriver extends BaseDriver {
249237
});
250238
}
251239
else if (result === undefined) { // throw NotFoundError on undefined response
252-
if (action.undefinedResultCode) {
253-
options.response.status = action.undefinedResultCode;
254-
255-
if (action.isJsonTyped) {
256-
options.response.json();
257-
} else {
258-
options.response.send();
259-
}
260-
options.next();
240+
if (action.undefinedResultCode instanceof Function) {
241+
throw new (action.undefinedResultCode as any)(options);
261242

262-
} else {
243+
} else if (!action.undefinedResultCode) {
263244
throw new NotFoundError();
264245
}
265246
}
266247
else if (result === null) { // send null response
267-
if (action.isJsonTyped) {
268-
options.response.body = null;
269-
} else {
270-
options.response.body = null;
271-
}
272-
273-
// Setting `null` as a `response.body` means to koa that there is no content to return
274-
// so we must reset the status codes here.
275-
if (action.nullResultCode) {
276-
options.response.status = action.nullResultCode;
277-
} else {
278-
options.response.status = 204;
279-
}
248+
if (action.nullResultCode instanceof Function)
249+
throw new (action.nullResultCode as any)(options);
250+
251+
options.response.body = null;
280252
}
281253
else if (result instanceof Uint8Array) { // check if it's binary data (typed array)
282254
options.response.body = Buffer.from(result as any);
@@ -289,6 +261,21 @@ export class KoaDriver extends BaseDriver {
289261
}
290262
}
291263

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+
292279
// apply http headers
293280
Object.keys(action.headers).forEach(name => {
294281
options.response.set(name, action.headers[name]);
@@ -303,12 +290,6 @@ export class KoaDriver extends BaseDriver {
303290
handleError(error: any, action: ActionMetadata | undefined, options: Action) {
304291
return new Promise((resolve, reject) => {
305292
if (this.isDefaultErrorHandlingEnabled) {
306-
// set http status
307-
if (error instanceof HttpError && error.httpCode) {
308-
options.response.status = error.httpCode;
309-
} else {
310-
options.response.status = 500;
311-
}
312293

313294
// apply http headers
314295
if (action) {
@@ -324,6 +305,13 @@ export class KoaDriver extends BaseDriver {
324305
options.response.body = this.processTextError(error);
325306
}
326307

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+
327315
return resolve();
328316
}
329317
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/defaults.spec.ts

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
11
import "reflect-metadata";
2-
import { createExpressServer, getMetadataArgsStorage, createKoaServer } from '../../src/index';
3-
import {ExpressMiddlewareInterface} from "../../src/driver/express/ExpressMiddlewareInterface";
2+
import {createExpressServer, createKoaServer, getMetadataArgsStorage} from "../../src/index";
43
import {Controller} from "../../src/decorator/Controller";
54
import {Get} from "../../src/decorator/Get";
6-
import {UseBefore} from "../../src/decorator/UseBefore";
7-
import {Middleware} from "../../src/decorator/Middleware";
8-
import {UseAfter} from "../../src/decorator/UseAfter";
9-
import {NotAcceptableError} from "./../../src/http-error/NotAcceptableError";
10-
import {ExpressErrorMiddlewareInterface} from "./../../src/driver/express/ExpressErrorMiddlewareInterface";
11-
import { QueryParam } from '../../src/decorator/QueryParam';
12-
import { OnUndefined } from '../../src/decorator/OnUndefined';
13-
import { assertRequest } from './test-utils';
5+
import {QueryParam} from "../../src/decorator/QueryParam";
6+
import {OnUndefined} from "../../src/decorator/OnUndefined";
7+
import {assertRequest} from "./test-utils";
8+
149
const chakram = require("chakram");
1510
const expect = chakram.expect;
1611

@@ -33,8 +28,8 @@ describe("defaults", () => {
3328
}
3429

3530
@Get("/paramfunc")
36-
paramfunc(@QueryParam('x') x: number) {
37-
return { foo: 'bar' };
31+
paramfunc(@QueryParam("x") x: number) {
32+
return { foo: "bar" };
3833
}
3934

4035
@Get("/nullfunc")
@@ -47,8 +42,8 @@ describe("defaults", () => {
4742
overridefunc() { }
4843

4944
@Get("/overrideparamfunc")
50-
overrideparamfunc(@QueryParam('x', { required: false }) x: number) {
51-
return { foo: 'bar' };
45+
overrideparamfunc(@QueryParam("x", { required: false }) x: number) {
46+
return { foo: "bar" };
5247
}
5348
}
5449
});
@@ -79,37 +74,37 @@ describe("defaults", () => {
7974
after(done => kuaApp.close(done));
8075

8176
it("should return undefinedResultCode from defaults config for void function", () => {
82-
assertRequest([3001, 3002], 'get', 'voidfunc', res => {
77+
assertRequest([3001, 3002], "get", "voidfunc", res => {
8378
expect(res).to.have.status(defaultUndefinedResultCode);
8479
});
8580
});
8681

8782
it("should return undefinedResultCode from defaults config for promise void function", () => {
88-
assertRequest([3001, 3002], 'get', 'promisevoidfunc', res => {
83+
assertRequest([3001, 3002], "get", "promisevoidfunc", res => {
8984
expect(res).to.have.status(defaultUndefinedResultCode);
9085
});
9186
});
9287

9388
it("should return 400 from required paramOptions", () => {
94-
assertRequest([3001, 3002], 'get', 'paramfunc', res => {
89+
assertRequest([3001, 3002], "get", "paramfunc", res => {
9590
expect(res).to.have.status(400);
9691
});
9792
});
9893

9994
it("should return nullResultCode from defaults config", () => {
100-
assertRequest([3001, 3002], 'get', 'nullfunc', res => {
95+
assertRequest([3001, 3002], "get", "nullfunc", res => {
10196
expect(res).to.have.status(defaultNullResultCode);
10297
});
10398
});
10499

105100
it("should return status code from OnUndefined annotation", () => {
106-
assertRequest([3001, 3002], 'get', 'overridefunc', res => {
101+
assertRequest([3001, 3002], "get", "overridefunc", res => {
107102
expect(res).to.have.status(404);
108103
});
109104
});
110105

111106
it("should mark arg optional from QueryParam annotation", () => {
112-
assertRequest([3001, 3002], 'get', 'overrideparamfunc', res => {
107+
assertRequest([3001, 3002], "get", "overrideparamfunc", res => {
113108
expect(res).to.have.status(200);
114109
});
115110
});

‎test/functional/express-middlewares.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {UseBefore} from "../../src/decorator/UseBefore";
77
import {Middleware} from "../../src/decorator/Middleware";
88
import {UseAfter} from "../../src/decorator/UseAfter";
99
import {NotAcceptableError} from "./../../src/http-error/NotAcceptableError";
10-
import {ExpressErrorMiddlewareInterface} from "./../../src/driver/express/ExpressErrorMiddlewareInterface";
10+
1111
const chakram = require("chakram");
1212
const expect = chakram.expect;
1313

@@ -73,7 +73,7 @@ describe("express middlewares", () => {
7373
class TestCustomMiddlewareWhichThrows implements ExpressMiddlewareInterface {
7474

7575
use(request: any, response: any, next?: Function): any {
76-
throw new NotAcceptableError('TestCustomMiddlewareWhichThrows');
76+
throw new NotAcceptableError("TestCustomMiddlewareWhichThrows");
7777
}
7878

7979
}

‎test/functional/koa-middlewares.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {UseAfter} from "../../src/decorator/UseAfter";
77
import {createKoaServer, getMetadataArgsStorage} from "../../src/index";
88
import {KoaMiddlewareInterface} from "../../src/driver/koa/KoaMiddlewareInterface";
99
import {NotAcceptableError} from "./../../src/http-error/NotAcceptableError";
10+
1011
const chakram = require("chakram");
1112
const expect = chakram.expect;
1213

@@ -68,7 +69,7 @@ describe("koa middlewares", () => {
6869
class TestCustomMiddlewareWhichThrows implements KoaMiddlewareInterface {
6970

7071
use(request: any, response: any, next?: Function): any {
71-
throw new NotAcceptableError('TestCustomMiddlewareWhichThrows');
72+
throw new NotAcceptableError("TestCustomMiddlewareWhichThrows");
7273
}
7374

7475
}

0 commit comments

Comments
 (0)