Implement a cache to fetch an image from the server only when the content has changed

asciidoctor / asciidoctor-kroki

Asciidoctor.js extension to convert diagrams to images using Kroki!

https://kroki.io/

MIT License

146 stars 47 forks source link

Implement a cache to fetch an image from the server only when the content has changed #90

Open ggrossetie opened 4 years ago

ggrossetie commented 4 years ago

The filename of the image can be defined by the user. In this case, we cannot tell if we need to fetch the image again because the content has changed or if we don't because the content is the same.

Previously, we were using the md5sum of the diagram content as filename. We might need to write a metadata file next to the images to compare the md5sum.

ggrossetie commented 4 years ago

We should leverage XDG https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html and create a single JSON file that contains a map of filename with the associated md5sum of the source diagram.

//cc @danyill

ggrossetie commented 4 years ago

Maybe we should revert https://github.com/Mogztter/asciidoctor-kroki/commit/00a8a69d32bb19939e151126802e04397ad6fd60 and release a new version. There's already a lot of changes! Do you mind @anb0s?

danyill commented 4 years ago

How about post-processing svg data to add the metadata in there rather than having a manifest?

The same thing could be done with png (see the spec). That would be at some level "nicer" than publishing a manifest.

Do we support other asciidoctor-diagram formats? (pdf and txt)?

It would probably add somewhat to processing time but might be a "cleaner" although more intricate solution (and perhaps harder to support for the browser).

Thoughts?

ggrossetie commented 4 years ago

Do we support other asciidoctor-diagram formats? (pdf and txt)?

Yes we support txt, pdf and jpeg but I think we can safely ignore pdf in this context.

It would probably add somewhat to processing time but might be a "cleaner" although more intricate solution (and perhaps harder to support for the browser).

Indeed, it means that we need libraries to understand png, svg and jpeg formats to add the additional metadata. In a browser, we do not fetch the images so that won't be an issue until we do.

The same thing could be done with png (see the spec). That would be at some level "nicer" than publishing a manifest.

It might be one of those cases where "le mieux est l'ennemi du bien." (not sure how to translate that, probably best/perfect is the enemy of the good). In other words, it sounds fancy and nice but I think we should keep it simple. I'm reluctant to alter the content of the images but it might be something we could implement in the Kroki server...?

danyill commented 4 years ago

Thanks for your thoughts @Mogztter I just wanted to add this idea into the mix. Yes we have the same expression "the perfect is the enemy of the good".

ggrossetie commented 4 years ago

Maybe we should revert 00a8a69 and release a new version.

I will move forward and do that. It will give us some time to think it through.

Thanks for your thoughts @Mogztter I just wanted to add this idea into the mix. Yes we have the same expression "the perfect is the enemy of the good".

Thanks, it's much appreciated and beneficial to explore all the possible solutions :+1:

ggrossetie commented 4 years ago

We could implement different strategies (configurable via attributes):

Host dependent cache (i.e. https://kroki.io/plantuml/svg/abcd1234 is not equivalent to https://localhost:8000/plantuml/svg/abcd1234)
Ignore/disable server cache (i.e. 304 Not Modified) using If-None-Match and/or If-Modified-Since
Ignore/disable local cache

For reference, currently the cache is host dependent, ignores server cache and local cache cannot be disabled.