Support for auto-generated Accept-Patch header

[Risto-Stevcev]

The auto-generated OPTIONS response works well, but it there currently isn't any option that you can set to include Accept-Patch for the media type(s) that your API's PATCH request accepts.

Creating a middleware function like this doesn't work because the middleware executes before the auto-generated OPTIONS response:

let acceptPatch = function(req, res, next) {
  if (/PATCH/.test(req.get('Allow')))
    res.set('Accept-Patch', 'application/json')
  next()
}
router.use(acceptPatch) // won't work because req.get('Allow') hasn't been auto-generated yet

[tunniclm]

@Risto-Stevcev Could you write something like this (EDIT: you can't):

let allowPatch = function(req, res, next) {
  if (router.methods.patch)
    res.set('Allow-Patch', 'application/json')
  next()
}
router.use(allowPatch)

I'm not sure whether methods is part of the public API, but it looks to be accessible.

It feels like a better API would allow you to both define middleware (for very general or very dynamic situations) but also specify something directly on the route, like:

router.patch('/some-resource', function(req, res, next) {
...
}, 'application/json');

router.patch('/some-resource', function(req, res, next) {
...
}, ['application/json', 'application/example']);

router.patch('/some-resource', function(req, res, next) {
...
}, function(...) {
...
});

I should mention, I haven't thought through the feasibility of those ideas. Jut thinking in "nice api" mode righ tnow.

[Risto-Stevcev]

router.methods doesn't seem to be accessible for me (using express 4.13.3), but I'm also weary of hacking using non-public methods because the code is for a web app company.

[tunniclm]

@Risto-Stevcev Apologies, my mistake then. I didn't have a chance to go in and check it for real, only by inspection.

[tunniclm]

@Risto-Stevcev I realised my mistake: methods is defined on a route not the Router. So the problem you'll have with middleware (either app or router middleware) is that it doesn't know yet which route the request is going to resolve to.

[tunniclm]

OK, this seems to work:

var express = require('express');
var app = express();

var router = express.Router();
router.patch('*', function(req, res, next) {
  res.set('Allow-Patch', 'application/json');
  next();
});

router.get('/a', function(req, res, next) { res.write('{}'); res.end(); });
router.patch('/a', function(req, res, next) { res.write('{}'); res.end(); });
router.patch('/b', function(req, res, next) { res.write('{}'); res.end(); });

app.use('/', router);

app.listen(3000);

Header not set for: GET /a
Header set for: PATCH /a and PATCH /b

[Risto-Stevcev]

@tunniclm That doesn't work either, because it affects the OPTIONS request and falsely advertises that PATCH is available for URIs that don't support it. For example, HTTP OPTIONS: /books/ has only GET, HEAD, and POST methods for a RESTful API, but it will wrongly advertise that PATCH is available for a collection of resources. It would work if the route only served resources that support PATCH such as /books/:id, /authors/:id, etc...

Something needs to change with how express does this, because there needs to be support from the library end for this.

[tunniclm]

@Risto-Stevcev Darn, yes, you're right.

So, you totally can already do something like the api I mentioned before. But that wouldn't be automatic like you wanted.

function allow_patch(type) {
  return function(req, res, next) {
    res.set('Allow-Patch', type);
    next();
  }
}

router.get('/a', function(req, res, next) { res.write('{}'); res.end(); });
router.patch('/a', allow_patch('application/json'), function(req, res, next) { res.write('{}'); res.end(); });
router.patch('/b', allow_patch('application/json'), function(req, res, next) { res.write('{}'); res.end(); });

Maybe with #2828 you could do something like:

router.with(function(req, res, next) {
  if (/PATCH/.test(req.get('Allow')))
    res.set('Allow-Patch', 'application/json')
  next()
}

...assuming these with() handlers run late enough.

[frankxw]

@tunniclm
My reference implementation for that feature request basically prepends the .with() middleware to a route's list of middleware when the route gets created. In essence it should do exactly the same thing as your above example (the whole reason for it is to avoid having to prepend allow_patch(...) to each route manually). The one caveat is that it affects every route in the router, but you explicitly test for 'Allow' so I think your second example would work just fine.

[Risto-Stevcev]

@demalus
If the proposed .with() middleware is prepended to the route's list of middleware, wouldn't that mean that the Allow headers aren't populated yet? That's essentially the problem I'm having with this -- The auto-generated OPTIONS middleware populates Allow after any middleware I add:

let acceptPatch = function(req, res, next) {
  if (/PATCH/.test(req.get('Allow')))
    res.set('Accept-Patch', 'application/json')
  next()
}
router.use(acceptPatch) // won't work because req.get('Allow') hasn't been auto-generated yet

[tunniclm]

@Risto-Stevcev It sounds like with() will work the same as pre-pending a route handler function, so if you can do it in a route handler, you should be able to do it.

[Risto-Stevcev]

Hopefully it would

[frankxw]

I just looked up how it sends the options - looks like it wraps the done callback and will send the options response if nothing else responded. Instead of testing any method against req.get('Allow'), you could do something similar to @tunniclm, and put it on a specific method, like router.patch() (since the header you want to add is specifically for patch):

function allow_patch(type) {
  return function(req, res, next) {
    if(req.method === 'OPTIONS') {
      res.set('Allow-Patch', type);
    }
    next();
  }
}
router.patch('*', allow_patch('application/json'));

[Risto-Stevcev]

@demalus
It's actually supposed to be part of OPTIONS. If the Allow header sent from that request includes PATCH, then it will include the Accept-Patch header as well.