Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add @Fallthrough() decorator and global option to enable route fallthrough, which is now off by default #223

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/RoutingControllersOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ export interface RoutingControllersOptions {
*/
routePrefix?: string;

/**
* Enable automatic fallthrough from action handlers to future actions or middleware
*/
automaticFallthrough?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fallthrough is better then automaticFallthrough . Also TBH fallthrough does not indicate for me what this option does, but I can't find a good name for it. Maybe this terminology is highly used, simply Im not familiar with it, adding other thoughts @19majkel94 @NoNameProvided . Is this word better then something like callNext: boolean ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also TBH fallthrough does not indicate for me what this option does, but I can't find a good name for it. Maybe this terminology is highly used, simply Im not familiar with it

It should be FallThrough because there is no such word as fallthrough, it is fall through. It is used in programming so I am cool with it, however we can make it more explicit via naming it as routeFallThrough.


/**
* List of controllers to register in the framework or directories from where to import all your controllers.
*/
Expand Down
15 changes: 15 additions & 0 deletions src/decorator/Fallthrough.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {getMetadataArgsStorage} from "../index";

/**
* Indicates that this route should fall through to further route handlers or middleware after being executed.
* This is equivalent to a route handler calling next() with no parameters in Express.
*/
export function Fallthrough(): Function {
return function (object: Object, methodName: string) {
getMetadataArgsStorage().responseHandlers.push({
type: "fallthrough",
target: object.constructor,
method: methodName
});
};
}
5 changes: 5 additions & 0 deletions src/driver/BaseDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ export class BaseDriver {
*/
routePrefix: string = "";

/**
* Enable automatic fallthrough from action handlers to future actions or middleware
*/
automaticFallthrough: boolean;

/**
* Indicates if cors are enabled.
* This requires installation of additional module (cors for express and kcors for koa).
Expand Down
5 changes: 5 additions & 0 deletions src/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ export interface Driver {
*/
routePrefix: string;

/**
* Enable automatic fallthrough from action handlers to future actions or middleware
*/
automaticFallthrough: boolean;

/**
* Indicates if cors are enabled.
* This requires installation of additional module (cors for express and kcors for koa).
Expand Down
16 changes: 11 additions & 5 deletions src/driver/express/ExpressDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ export class ExpressDriver extends BaseDriver implements Driver {
options.response.redirect(action.redirect);
}

options.next();
if (this.automaticFallthrough || action.fallthrough)
options.next();

} else if (action.renderedTemplate) { // if template is set then render it
const renderOptions = result && result instanceof Object ? result : {};
Expand All @@ -273,7 +274,9 @@ export class ExpressDriver extends BaseDriver implements Driver {
} else if (html) {
options.response.send(html);
}
options.next();

if (this.automaticFallthrough || action.fallthrough)
options.next();
});

} else if (result !== undefined || action.undefinedResultCode) { // send regular result
Expand All @@ -287,18 +290,21 @@ export class ExpressDriver extends BaseDriver implements Driver {
} else {
options.response.send();
}
options.next();
if (this.automaticFallthrough || action.fallthrough)
options.next();
} else {
if (action.isJsonTyped) {
options.response.json(result);
} else {
options.response.send(result);
}
options.next();
if (this.automaticFallthrough || action.fallthrough)
options.next();
}

} else {
options.next();
if (this.automaticFallthrough || action.fallthrough)
options.next();
}
}

Expand Down
25 changes: 20 additions & 5 deletions src/driver/koa/KoaDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,10 @@ export class KoaDriver extends BaseDriver implements Driver {
options.response.redirect(action.redirect);
}

return options.next();
if (this.automaticFallthrough || action.fallthrough)
return options.next();
else
return;

} else if (action.renderedTemplate) { // if template is set then render it // todo: not working in koa
const renderOptions = result && result instanceof Object ? result : {};
Expand All @@ -249,7 +252,10 @@ export class KoaDriver extends BaseDriver implements Driver {
await ctx.render(action.renderedTemplate, renderOptions);
});

return options.next();
if (this.automaticFallthrough || action.fallthrough)
return options.next();
else
return;

} else if (result !== undefined || action.undefinedResultCode) { // send regular result
if (result === null || (result === undefined && action.undefinedResultCode)) {
Expand All @@ -268,18 +274,27 @@ export class KoaDriver extends BaseDriver implements Driver {
options.response.status = action.undefinedResultCode;
}

return options.next();
if (this.automaticFallthrough || action.fallthrough)
return options.next();
else
return;
} else {
if (result instanceof Object) {
options.response.body = result;
} else {
options.response.body = result;
}
return options.next();
if (this.automaticFallthrough || action.fallthrough)
return options.next();
else
return;
}

} else {
return options.next();
if(action.fallthrough)
return options.next();
else
return;
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export * from "./decorator/CookieParams";
export * from "./decorator/Ctx";
export * from "./decorator/CurrentUser";
export * from "./decorator/Delete";
export * from "./decorator/Fallthrough";
export * from "./decorator/Get";
export * from "./decorator/Head";
export * from "./decorator/Header";
Expand Down Expand Up @@ -179,6 +180,12 @@ export function createExecutor(driver: Driver, options: RoutingControllersOption
driver.isDefaultErrorHandlingEnabled = true;
}

if (options.automaticFallthrough !== undefined) {
driver.automaticFallthrough = options.automaticFallthrough;
} else {
driver.automaticFallthrough = false;
}

if (options.classTransformer !== undefined) {
driver.useClassTransformer = options.classTransformer;
} else {
Expand Down
8 changes: 8 additions & 0 deletions src/metadata/ActionMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ export class ActionMetadata {
*/
successHttpCode: number;

/**
* Specifies if this action should fall through to future handlers or middleware or not.
*/
fallthrough: boolean;

/**
* Specifies redirection url for this action.
*/
Expand Down Expand Up @@ -173,6 +178,7 @@ export class ActionMetadata {
const redirectHandler = responseHandlers.find(handler => handler.type === "redirect");
const renderedTemplateHandler = responseHandlers.find(handler => handler.type === "rendered-template");
const authorizedHandler = responseHandlers.find(handler => handler.type === "authorized");
const fallthroughHandler = responseHandlers.find(handler => handler.type === "fallthrough");
const bodyParam = this.params.find(param => param.type === "body");

if (classTransformerResponseHandler)
Expand All @@ -187,6 +193,8 @@ export class ActionMetadata {
this.redirect = redirectHandler.value;
if (renderedTemplateHandler)
this.renderedTemplate = renderedTemplateHandler.value;
if (fallthroughHandler)
this.fallthrough = true;

this.bodyExtraOptions = bodyParam ? bodyParam.extraOptions : undefined;
this.isBodyUsed = !!this.params.find(param => param.type === "body" || param.type === "body-param");
Expand Down
3 changes: 2 additions & 1 deletion src/metadata/types/ResponseHandlerType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ export type ResponseHandlerType = "success-code"
|"on-null"
|"on-undefined"
|"response-class-transform-options"
|"authorized";
|"authorized"
|"fallthrough";
2 changes: 1 addition & 1 deletion test/functional/express-middlewares.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ describe("express middlewares", () => {
});

let app: any;
before(done => app = createExpressServer().listen(3001, done));
before(done => app = createExpressServer({ automaticFallthrough: true }).listen(3001, done));
after(done => app.close(done));

it("should call a global middlewares", () => {
Expand Down
50 changes: 50 additions & 0 deletions test/functional/fallthrough-decorator.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import "reflect-metadata";
import {Controller} from "../../src/decorator/Controller";
import {Fallthrough} from "../../src/decorator/Fallthrough";
import {UseAfter} from "../../src/decorator/UseAfter";
import {Get} from "../../src/decorator/Get";
import {createExpressServer, createKoaServer, getMetadataArgsStorage} from "../../src/index";
import {assertRequest} from "./test-utils";
const chakram = require("chakram");
const expect = chakram.expect;

describe("fallthrough-decorator behavior", () => {
let useAfter: boolean;

beforeEach(() => {
useAfter = undefined;
});

before(() => {
// reset metadata args storage
getMetadataArgsStorage().reset();

@Controller()
class FallthroughController {
@Get("/books")
@Fallthrough()
@UseAfter(function(req: any, res: any, next: (err?: any) => any) {
useAfter = true;
})
getAll() {
return "<html><body>All books</body></html>";
}
}
});

let expressApp: any, koaApp: any;
before(done => expressApp = createExpressServer().listen(3001, done));
after(done => expressApp.close(done));
before(done => koaApp = createKoaServer().listen(3002, done));
after(done => koaApp.close(done));

describe("get should respond with proper status code, headers and body content, and should call after middleware", () => {
assertRequest([3001, 3002], "get", "books", response => {
expect(response).to.have.status(200);
expect(response).to.have.header("content-type", "text/html; charset=utf-8");
expect(response.body).to.be.equal("<html><body>All books</body></html>");
expect(useAfter).to.be.equal(true);
});
});

});
2 changes: 1 addition & 1 deletion test/functional/koa-middlewares.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe("koa middlewares", () => {
});

let app: any;
before(done => app = createKoaServer().listen(3001, done));
before(done => app = createKoaServer({ automaticFallthrough: true }).listen(3001, done));
after(done => app.close(done));

it("should call a global middlewares", () => {
Expand Down