Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the content URI caching feature in a Node environment #1507

Open
ggrossetie opened this issue Dec 18, 2021 · 4 comments · May be fixed by #1523
Open

Implement the content URI caching feature in a Node environment #1507

ggrossetie opened this issue Dec 18, 2021 · 4 comments · May be fixed by #1523

Comments

@ggrossetie
Copy link
Member

Asciidoctor provides a built-in cache for content URI (based on open-uri-cached gem).
Currently, this feature is not available in Asciidoctor.js because the open-uri-cached is not transpile from Ruby to JavaScript.

I think it could be great to implement this feature in a Node environment. This feature will use the cache-uri attribute to enable the cache (similar to what Asciidoctor Ruby is doing: https://docs.asciidoctor.org/asciidoc/latest/directives/include-uri/#caching-uri-content)

To implement this feature, we could use: https://github.com/isaacs/node-lru-cache in order to set expiration time depending on response headers, configure a default max size, etc...

@ggrossetie
Copy link
Member Author

Asciidoctor core (Ruby) relies on https://github.com/tigris/open-uri-cached which cache content on disk. I don't like this approach since it creates "temporary" files in an arbitrary location (by default /tmp/open-uri-#{Process.uid}).
In addition, the library does not automatically clean/remove/invalidate cache. As a result, a long running process could lead to a disk saturation.

I think we should define a default cache size and use an in-memory cache. It might be interesting to configure a global cache or tie a cache to a document.

@ggrossetie
Copy link
Member Author

Interestingly, the cache is actually used between executions since the uid is stable (for instance, when running the CLI multiple times). It's also not possible to disable the cache in a program once enabled:

Asciidoctor.convert("image::http://localhost:5000/cc-zero.svg[opts=inline]", { 
  safe: 'safe', 
  attributes: {'allow-uri-read' => '', 'cache-uri' => ''} 
})

Asciidoctor.convert("image::http://localhost:5000/cc-zero.svg[opts=inline]", { 
  safe: 'safe',
  attributes: {'allow-uri-read' => ''}
})

The second call will use the cache despite the fact that cache-uri is absent. The reason is that open-uri/cache was loaded (and cannot or at least is not unloaded)

@mojavelinux
Copy link
Member

I think the URI/URL caching layer in Asciidoctor Ruby could be improved by making it pluggable. We integrated open-uri/cache in the early days to provide a quick win, but it's clearly a simple implementation.

@ggrossetie
Copy link
Member Author

I think the URI/URL caching layer in Asciidoctor Ruby could be improved by making it pluggable

An abstract layer on content read (file/URI) would be my dream ✨

We integrated open-uri/cache in the early days to provide a quick win, but it's clearly a simple implementation.

The most straightforward approach is to provide a JavaScript compatible implementation of open-uri/cache.
Opal cannot unload a module (or I least I don't know how) once loaded so the behavior is consistent with Asciidoctor Ruby.

I think I will store the cache in memory with a default value of 16Mb and use a simple implementation.

ggrossetie added a commit to ggrossetie/asciidoctor.js that referenced this issue Dec 31, 2021
ggrossetie added a commit to ggrossetie/asciidoctor.js that referenced this issue Dec 31, 2021
ggrossetie added a commit to ggrossetie/asciidoctor.js that referenced this issue Dec 31, 2021
ggrossetie added a commit to ggrossetie/asciidoctor.js that referenced this issue Dec 31, 2021
ggrossetie added a commit to ggrossetie/asciidoctor.js that referenced this issue Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants