Added cache control utilities

[delvedor]

Caching is very important for enabling a performant web. This pr adds a small utility to help users configure cache control headers.

Ther is a generic API for configure the header and two specialized apis, one for no caching and one for stale content. Is there something else we should have?

Useful reading:

• MDN docs
• HTTP cache 101

cc @fastify/core

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[jsumners]

🤔 I think maybe fastify-caching should be deprecated. The server side cache functionality it provides could be a new module, and fastify-etag already fills the other half. Then we could create a module like fastify-cache-control that is specifically for dealing with these sorts of headers. Thoughts?

[delvedor]

@jsumners good point, I forgot about fastify-caching 😅.
fastify-caching is doing a lot more than what I would like to accomplish, which is to offer a few APIs to make it easy configure cache control headers.

We could make a new plugin just for them, but I'm not sure it would be worth it. fastify-sensible feels like a good place for it, as the main goal of this plugin is to offer user friendly APIs for common tasks without need to reach the "low level" API of Fastify. Furthermore, it already houses the Vary header manipulation and some request header utilities.

[zekth]

Why not making fastify-caching and fastify-etag part of fastify-sensible ?

[mcollina]

This module is mainly a toolbelt module, it does not alter the workings of Fastify. Both fastify-etag and fastify-caching do.

[mcollina]

🤔 I think maybe fastify-caching should be deprecated. The server side cache functionality it provides could be a new module, and fastify-etag already fills the other half. Then we could create a module like fastify-cache-control that is specifically for dealing with these sorts of headers. Thoughts?

I would rename fastify-caching into fastify-caching-store or something similar. I think the functionality in there is useful.

I'm happy to have these in here or in a new fastify-caching of fastify-cache-control module. fastify-sensible could then depend to that module.

[mcollina]

(I'm also ok keep them here, they are just decorators)

[mcollina]

lgtm

[mcollina]

The only concern in keeping those accessors here is that this plugin is not built to be reused by other plugins, because it adds a error handler.

[delvedor]

because it adds a error handler.

We could cut a major and disable the error handler by default, or advise better that if you use it in a nested plugin, it should be disabled.

[delvedor]

I've updated the helpers and added a few utilities for common use cases:

• .preventCache() // tells to browsers and CDN to not cache anything
• .revalidate() // tells to the browser that the content must be revalidated asap
• .staticCache(time) // tells to the browser that the content is static and immutable, CDNs can cache it.

There are a .stale() and .maxAge helpers as well to help build a stale strategy.

[Eomm]

I agree to disable the error handler by default too

[jsumners]

The more I think about this, the more I dislike having the caching stuff in this module. I agree that the functionality in fastify-caching is useful, but I think we made the wrong decision to copy Hapi's caching feature set. We should have created fastify-cache-utils to manage the HTTP spec and fastify-caching to provide the server side cache interface.

As for this PR, I can easily see someone wanting to use the HTTP spec cache control features added by it but not wanting things like httpErrors and assert functions. For someone that just wants to manage the cache headers, these things are baggage.

So I think we should really create some new module to handle the HTTP cache control spec. I already mentioned a possible name of fastify-cache-utils, but maybe fastify-http-cache-control would be better?

[mcollina]

I likely prefer the approach of a separate module as well.

[delvedor]

Oks, I'll work on a separate module in the upcoming days :)

[delvedor]

I've been thinking about this and I don't think having a separate module is worth it.
It requires a lot of overhead and it will be yet another repo to maintain when the only thing it will be doing is to add six convenience methods for writing header values.

I think this repository is still the best place to have them, as we already have the vary control here as well. If a user doesn't want to use httpErrors and assert, they don't have to. This module is adding decorators (aside from the error handler which will be disabled), and decorators don't need to be used.
If we want to follow the "not wanting things like" logic, then every decorator should have its own plugin, where do you draw the line?

[mcollina]

lgtm

[jsumners]

It requires a lot of overhead and it will be yet another repo to maintain when the only thing it will be doing is to add six convenience methods for writing header values.

I agree. But that's why I suggested it be a module for the whole cache control spec.

I think this repository is still the best place to have them, as we already have the vary control here as well.

It should be moved to the module that is specifically for the cache control spec.

If we want to follow the "not wanting things like" logic, then every decorator should have its own plugin, where do you draw the line?

This is a bit of a straw man. There was a clear line proposed: a module is specific to a domain instead of a grab bag of things.

[delvedor]

A module for http caching should include not only the header helpers we are shipping here,but at least etag handling as well (and probably request headers caching as well).
We could build it, no problem with that.
But I still think that this plugin should ship this helpers as well. It can be handy if you need many utilities and you can always remove it if you need something more advanced.

[jsumners]

I am not blocking this.

[mcollina]

then land?

[jsumners]

then land?

I am leaving that decision up to the team.

[delvedor]

I'll merge and release this as soon as I have time.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mcollina]

@delvedor could you fix this PR?

[mcollina]

LGTM

[hsynlms]

It would be better to set pragma and expires headers to give compatibility support for HTTP/1.0? @delvedor

this
  .header('Cache-Control', 'no-store, max-age=0, private')
  .header('pragma', 'no-cache')
  .header('expires', 0)

[hsynlms]

see: https://owasp.org/www-community/OWASP_Application_Security_FAQ#how-do-i-ensure-that-sensitive-pages-are-not-cached-on-the-users-browser

[mcollina]

please open a new issue or propose a PR, this landed.

[hsynlms]

@mcollina I just wanted to get your opinions in the first place.