apicache icon indicating copy to clipboard operation
apicache copied to clipboard

`app.use(cache('5 minutes'))` will cache all routes, regardless of methods like `GET` and `POST`

Open xinbenlv opened this issue 6 years ago • 6 comments

It turns out if we do app.use(cache('5 minutes')), it will cache all routes, regardless of methods like GET and POST. However, for methods like PUT and POST, in many cases this implied update on the server side and shall not be cached. Should we add documentation or functionality to make supporting such case more easily?

xinbenlv avatar Jul 18 '19 23:07 xinbenlv

@killdash9 @svozza Thoughts on this?

I agree that probably only GET should be cached, and am happy to put that check in, but for the following:

  • What am I missing? Should PUT/POST/PATCH/DELETE responses ever be cached?
  • Technically this would be a breaking change in terms of behavior, but I cringe at making a major version bump for something this trivial, especially if this is more of a "fixing expected behavior" patch fix. Thoughts on that as well?

kwhitley avatar Aug 15 '19 18:08 kwhitley

I don't think that PUT,PATCH,DELETE should ever be cached. POST is a gray area because the SOAP protocol uses POST for everything, although that technology is on the way out.

It could be built to ignore POST (and PUT,PATCH,DELETE) in the app.use(cache(...)) pattern, and if anyone needs post caching they could add it on the route explicitly. This seems like a good default behavior. It does sound like a "fixing expected behavior" fix to me.

killdash9 avatar Aug 15 '19 19:08 killdash9

+1 to @killdash9

xinbenlv avatar Aug 16 '19 08:08 xinbenlv

Something I would say is that I've used GraphQL APIs where the request was submitted as a POST request but I'm not sure how common that is.

svozza avatar Aug 16 '19 13:08 svozza

In my case I like that the POST is also being cached. So I can precache a route that never does a calculation or access database directly. So in a sense it's a route that is fed the data that it should respond with. And how the data is fetched and processed is done somewhere else, hidden from the api accessed on the forntend

Here is an example of my experience with this:

I have some trigger that posts to /api/:someParam The frontend has no way to reach this trigger or its functionality

// doCache: only on status=200
// this api is accessed from the frontend
app.use('/api/:someParam', doCache, (req: Request, res: Response, next: NextFunction) => {
  const { method, body } = req;

  // Reply with body on POST to cache the response
  // On the next GET request the response will be the cached response from the POST
  if (method === 'POST') {
    res.json(body);
  } else {
    res.status(404).json(null);
  }
});

elvismercado avatar Aug 29 '19 10:08 elvismercado

Not sure it's really an issue, just apply the cache middleware to the method functions (ie. app.post) and not the route ones (app.use) you dont have to explicitly handle 404s etc then express will do it.

curtdept avatar Dec 07 '19 18:12 curtdept