Skip to content
This repository has been archived by the owner on Apr 18, 2021. It is now read-only.

Allow users to specify that caddy-cache should ONLY cache paths from match_path #26

Open
rickles42 opened this issue Jul 8, 2018 · 5 comments

Comments

@rickles42
Copy link

Currently, it appears that even if I use the "advanced usage" approach and specify a particular path to be cached, other paths can automatically be cached as well.

In my case, I would like to instruct caddy-cache only to cache the /api path, and leave everything else out of the cache, regardless of what headers it may include. However, even if I specify match_path /api in the cache directive of my Caddyfile, I see other files from outside that path appearing in the cache.

I'm not sure if this behavior is by design or not. But the documentation seems to suggest that only the "basic usage" approach should result in this automatic behavior, and the presence of any "advanced usage" should restrict caching to just the path the user specified.

@nicolasazrak
Copy link
Owner

Maybe I should update the documentation, but the idea is to always respect the headers. The "basic usage" (if you specify a path in the config) is just a "hint".

For example: if you add /api, it means to be cached unless a header specifies otherwise. It doesn't override what headers indicate. If /public has a cache-control it will be cached even if it wasn't specified in the config. What it changes when you specify a path in the config is that if /api doesn't has any header, it will be cached.

I hope this clarifies how it works.

@rickles42
Copy link
Author

That makes sense, thanks for the clarification. Maybe the documentation could make this clearer if it were structured along the lines of:

When the cache plugin is enabled, any responses containing headers A, B, or C will be cached, except when <RFC> indicates they should not be. In addition, the presence of match_path instructs the plugin to cache matching responses even without A, B, or C.

@csytan
Copy link

csytan commented Jul 6, 2019

I ran into this issue as well. In combo with issue #28, it resulted in the unexpected behavior of serving up empty 304 responses 😮.

I think it's confusing because the label syntax in many other Caddy plugins is activated based on the path. For example: proxy /explicit_path localhost:8080 { ... }.

Confusion results because the label syntax in the cache plugin is opt-out and not based on the path. The naming of the directive match_path doesn't help, because on skimming the docs, it makes it appear to be opt-in and based on the path.

@nicolasazrak
Copy link
Owner

@csytan The idea was to be able to use it without configure anything and use cache headers. I'm sorry if that's not how it looks. I wanted to offer both ways to enable it: header based rules and config based rules (becase maybe you don't control headers or what a simpler way). What would it be the ideal caddyfile for you having that in mind? (#28 was already solved)

@csytan
Copy link

csytan commented Jul 10, 2019

@nicolasazrak I think a path based approach would be easier to understand. For example:

cache /my_website {
    except /my_website/do_not_cache_this_path
}

For backwards compatibility, if no is path specified, then cache every path:

cache {
    ...
}

# above is equivalent to:
cache / {
     ...
}

match_path would be even more confusing at first glance. Perhaps re-naming it to something like cache_path_without_header, force_cache_path, or force_cache would be more obvious. Also deprecating match_path in a future version.

P.S. Thank you for working on this project. Looking forward to trying the fix for #28 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants