One url make 2 action call

[soulski]

Hi, I have one Controller which have 2 method like below

import { JsonController, Get } from 'routing-controllers'

@JsonController('/api')
export class APIController {

    @Get('/user')
    public getByUser() {
        console.log('output2')
        return Promise.resolve({ say: 'user' })
    }

    @Get('/:id')
    public getById() {
        console.log('output1')
        return Promise.resolve({ say: 'hello1' })
    }

}

I run server and then send http request in path '/user'. The result in console log print follow below

output1
output2

and I receive response as json { say: 'user' }. I take a look at source code and found that after action was called, routing-controller call next method which I suspect it is the reason why 2 action which match path url call. I test by insert after after middleware that didn't call next. The result is only getByUser was called. Is this intend behavior? It use invoke only getByUser, is't it?

[NoNameProvided]

By default, route variables (aka your :id) will match alphanumeric characters, so your /:id endpoints matches the /user route. See #213 (comment) for details.

[marshall007]

@NoNameProvided this still seems like a bug though. It shouldn't be possible for controller methods to fall through like middleware. Once a controller method has handled the request that should be the end of it.

[NoNameProvided]

Yeah, it's not mentioned in the docs and this is what makes it seems like a bug the most, also that you can only return response from the first route which was hit. PR-s are welcomed for this as for any other issue, but first lets ask @pleerock if it was intentional or not?

[ArcanoxDragon]

I'm running into this issue as well. I have an action which listens on /js/common.js and one which listens on /js/:path*, with /js/common.js being first. In Express, if a route were to hit /js/common.js, it would immediately stop propagation and run that action. With routing-controllers, it's hitting /js/common.js first and then falling through to /js/:path*, causing an error when setting some headers, because they've already been sent by /js/common.js. Routes should not fall through.

[ArcanoxDragon]

I just made a pull request with a solution to this problem that I have now been using in my application (see #222). Place @Terminates() on your most specific route (and make sure to define it first).

If you don't feel like waiting for the PR to be accepted (assuming it is), clone my issue-220 branch, run npm install && gulp package, and replace the contents of your node_modules/routing-controllers folder with the new contents of build/package in the cloned repo (tedious, but it works).

Scratch this...it works, but after further consideration, I don't think that falling through should really be the default (after all, in Express routes, you have to explicitly fall through by accepting a next parameter and calling it in a route handler...falling through should be explicit in this library as well).

[NoNameProvided]

Actually I don’t think anyone takes advantage of falling through requests. So I think we can make this the default behavior without adding an extra option to re-enable it.

I would like to see this implemented as well but as this is a breaking change let’s wait until @pleerock let’s us know if it’s a bug or a feature :D

[ArcanoxDragon]

I just made a pull request (#223) which changes the default behavior to no-fallthrough, with an option to re-enable it. So, technically it's a breaking change, but it can be reverted per-server with a global option.

[pleerock]

okay, again we have issue with our next call after action execution. I think this will be fixed once we decide what to do with next call and most probably remove it. Anyone from contributors can take initiative to implement those fixes?

[MichalLytek]

When we remove next call, we have to build our own midleware stack handler to make after middlewares work. As now we are discussing the need of after middlewares in classic (req, res, next) way - it could be replaced with enhanced interceptors that have access also to action metadata and returned value.

[pleerock]

If we remove next call then I dont think we need after middleware...

[Diluka]

@soulski
There are something wrong with your API design. You should obey the RESTful patterns. APIs should distinguish from each other.

api/user and api/:id have ambiguity if the request uri is api/user. The server doesn't know the :id can't be user. And the order of method definition is not reliable.

[AntSworD]

I think koa router style is a good solution, according their code order, unless the first route has next