Middleware.static: etag_of_fname should return Lwt.t ?!

[pkel]

Hi and thanks for your efforts on this project. Opium is really useful to get a web app running quickly.

I tried to enable caching for static files served with the static and static_unix middlewares. I have two use cases:

1. static: check last modification of file in Irmin/Git store, use commit hash as etag.
2. static_unix: derive etag from Lwt_unix.stat st_mtime or similar.

Unfortunately, the etag_of_fname argument is of type string -> string option. I'd prefer string -> string option Lwt.t. Are you interested in a PR changing the type? I could also extend the static_unix middleware such that it defaults to mtime-based etags.

[tmattio]

Hi @pkel!

Are you interested in a PR changing the type?

Sure, that makes sense, I'd be happy to merge such a PR

I could also extend the static_unix middleware such that it defaults to mtime-based etags.

Could you clarify what you have in mind?

I'm not sure what the spec has to say about it, but I've only seen Etag generated from content hashes.

[pkel]

Could you clarify what you have in mind?

I'd go for "a hash of the last modification timestamp" mentioned here:
https://en.wikipedia.org/wiki/HTTP_ETag#ETag_generation

I think this should be enough for static files served from a UNIX file system.

[tmattio]

I'd go for "a hash of the last modification timestamp" mentioned here

We could provide this as an option, something like ?etag_strategy:[> `Content_hash | `Modification_timestamp ] ? I'd be fine with having the timestamp by default, but I can imagine use cases where users would like to use a different generation strategy

[pkel]

What about keeping the optional ?etag_of_fname:(string -> string option Lwt.t) and doing the timestamp thing if ~etag_of_fname is not given? This way we would not break existing use-cases, e.g. someone tracking the etags in a hash-table.