restify / node-restify

The future of Node.js REST development
http://restify.com
MIT License
10.72k stars 982 forks source link

Support server.use('/route', handler) and server.all() #289

Closed arcmode closed 7 years ago

arcmode commented 11 years ago

Edited

Would be very nice to have server.use('/route', handler) and server.all()functions in restify.

Thanks Falco and Mark.

Cheers, David

fnogatz commented 11 years ago

I think the express way would be to add a server.use('/api', validate.Credentials). I would prefer this way as the server.use() is already implemented in the new restify version - and the nested routes look a little bit weird ;) All it needs is to add the optional parameter to server.use() to restrict the middleware to specific resources.

mcavage commented 11 years ago

I think Falco's suggestion is a nicer API - that gives you the same thing - it'll just be a bit weird to go tack that in in the code, but from a library perspective I like that.

arcmode commented 11 years ago

Thanks for pointing that. Much better that way.

fnogatz commented 11 years ago

Reopen as being not implemented yet?

arcmode commented 11 years ago

Yes, thanks!.

jo commented 11 years ago

+1 for extended use, like in http://expressjs.com/api.html#app.use

kvnneff commented 11 years ago

+1. I was originally using Express for a large, distributed service and was able to link routes within my sub-modules into my main API's routes like so:

var users = require('module-users');
app.use('/api/users', users);

Being able to do this with Restify would keep me using Restify.

djensen47 commented 10 years ago

Any updates on this?

Seems like it should be two separate Github issues:

  1. server.use([path], function)
  2. server.all(…)
mieky commented 9 years ago

I would love to see the server.use(path, middleware) syntax implemented, as it would enable more modular routes. Had to revert to Express for now.

micahr commented 9 years ago

The trouble with supporting this pattern is that if you've already added a route to the route list, this new handler would have to be added to the end or beginning, which could make it too late or too early for the expected behavior. When you are forced to build the handler chain for your route, you can ensure that handlers are executed in the expected order.

roblav96 commented 9 years ago

+1

aaronroberson commented 9 years ago

+1

I'm currently using restify-namespace (https://github.com/mpareja/node-restify-namespace) to accomplish similar functionality.

fullstackwebdev commented 9 years ago

Yes I would also like this feature!

erikmueller commented 9 years ago

Is this still being considered? Im also using restify-namespace for now to check wether restify could replace an express approach. Is there any other mechanic to let's say expose every route under '/api' for example?

Cheers.

sulhome commented 9 years ago

What is the status of this issue? It has been suggested 3 years ago!

This is very important because it saves us from doing:

api.use(req,res,next) => {
   if(req.url.startsWith('')){
       //etc
   }
}
roblav96 commented 9 years ago

I don't think this will be implemented because it adds too much overhead. I love restify because of how basic and barbones it is. I hope it stays that way.

sulhome commented 9 years ago

I totally disagree with @roblav96 The case for this feature is justified and indeed very useful. Imagine 5 routes which need the same object (for example book) based on an id from the url. Instead of every route (method) executing the same code to get the same book, you can with this feature supply that book and add it to the req object

roblav96 commented 9 years ago

Or do this:

server.post( '/api/public/prekey',
    require( './middle/public/throttle.js' ).bind( envThrottle.pre ),
    require( './middle/public/bytes.js' ),
    require( './routes/public/pre-key.js' )
)
server.post( '/api/public/verifycaptcha',
    require( './middle/public/throttle.js' ).bind( envThrottle.pre ),
    require( './middle/public/bytes.js' ),
    require( './routes/public/verify-captcha.js' )
)
server.post( '/api/public/register',
    require( './middle/public/throttle.js' ).bind( envThrottle.pub ),
    require( './middle/public/bytes.js' ),
    require( './middle/public/verify-prekey.js' ),
    require( './routes/public/register.js' )
)
server.post( '/api/public/login',
    require( './middle/public/throttle.js' ).bind( envThrottle.pub ),
    require( './middle/public/bytes.js' ),
    require( './middle/public/verify-prekey.js' ),
    require( './routes/public/login.js' )
)

It only adds those middlewares to my public routes.

micahr commented 9 years ago

We are working on a solution to the issue of route inheritance and sharing of common middleware. You can take a look at: restify/conductor. It is still a work in progress, but is a system we actively use to manage our routes and their dependencies. The biggest benefit we get is to have a set of common route middlewares that we can share across many different routes, without having to resort to adding them to every route manually.

The API is still in flux, but will hopefully firm up soon. The documentation is very minimal right now, but will be improved over time as well.

DonutEspresso commented 9 years ago

Yes, please check out restify-conductor. It is a different take on the abstraction approach for reusable routes, without explicitly tying them to a specific URL(s).

We would love to hear any feedback you may have.

omnip620 commented 8 years ago

+1

caomu commented 8 years ago

+1

michaelfavia commented 8 years ago

I started converting everything to restify this morning and it went really well until i hit this roadblock. We were previously using the middleware solution for route inheritance as well in express.

CreativeManix commented 8 years ago

+1000 This is required to do some common logic, please add

tbranyen commented 8 years ago

If you're working with simple middleware, you might be able to get away with this 8 line replacement for the server.use('/prefix', middleware) signature:

function scopeMiddlewareTo(prefix, middleware) {
  return function(req, res, next) {
    if (req.url.indexOf(prefix) === 0) {
      req.url = req.url.slice(prefix.length);
      return middleware.call(this, req, res, next);
    }
    else { return next(); }
  };
}

And use with:

server.use(scopeMiddlewareTo('/prefix', myMiddleware));

This works for very simple middleware that are only checking the req.url, but hey that's most cases, and will be easier to add than getting Restify to change or opt'ing for the overkill Restify conductor.

scippio commented 8 years ago

bump :thinking:

FrikkieRetro commented 8 years ago

@tbranyen Why do you slice the req.url? If you wish to have more than one handler on the same prefix, this slice makes that impossible.

Good workaround though, thanks.

saidkholov commented 7 years ago

+1 Has this been implemented yet? Is anyone working on it?

cocoya23 commented 7 years ago

Maybe this will help https://www.npmjs.com/package/restify-router

mwthink commented 7 years ago

+1

chrisenytc commented 7 years ago

+1

flowl commented 7 years ago

This is essential, especially when using versioning via GET instead of headers and with many routes.

mossy02 commented 7 years ago

+1 - would be great to have for shared middleware used across versioned routes

minhchu commented 7 years ago

+1 - will this feature be added in v5.x ?

alber70g commented 7 years ago

@tbranyen https://github.com/restify/node-restify/issues/289#issuecomment-202445094

This doesn't work, since, restify only routes through middleware when it knows about a registered route on the prefix. You will get ResourceNotFound if the requested http verb on the route isn't found.

tbranyen commented 7 years ago

@Alber70g absolutely correct, that's something that has bitten me many times using Restify. I've since abandoned Restify in favor of Express, but our team would like that to be a choice. I met with current maintainers of Restify @DonutEspresso and @yunong and we all agreed this feature should be implemented. However, we need to spec out the work and figure out a less ad-hoc way of introducing the change.

The work here would need to include (at a minimum):

This weekend I'll put together an idea of what could be implemented and mock out the intended behavior. If anyone else wants to help contribute to this effort, please think of potential gotchas and wants along with implementation concerns.

alber70g commented 7 years ago

@tbranyen I'm not sure, but if you'd use a regex on a get,post,put,delete it would trigger all middleware since the regex would potentially match on all requests. Although I think there's a reason why this shouldn't work, as it gives a huge performance penalty on routes that don't exist (probably?).

Maybe that's why we wouldn't want it in this way. A structural solution for scoped routes would be better. Maybe all routes nested under a server.use(<prefix>, middleware), should be 'registered' to the pipeline explicitly (e.g. api/ -users -posts -things would be registered as api/users api/posts api/things). In a way server.use(<prefix>, middleware) becomes then more of syntactic sugar

MartinMuzatko commented 5 years ago

There is https://www.npmjs.com/package/restify-router to organize routes/subroutes. Not sure if this works for middlewares.

abhikk commented 2 years ago

https://www.npmjs.com/package/restify-prefix-route

Use this to add prefix in all routes like... server.pre(applyPrefix('/v1'));