Skip to content

Customize cache key generation #30

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

Closed
sagikazarmark opened this issue Mar 21, 2017 · 4 comments · Fixed by #35 or #36
Closed

Customize cache key generation #30

sagikazarmark opened this issue Mar 21, 2017 · 4 comments · Fixed by #35 or #36
Assignees

Comments

@sagikazarmark
Copy link
Member

Q A
Bug? no
New Feature? yes
Version 1.2

Actual Behavior

The cache key is currently generated based on the following details:

  • hash algo
  • request method
  • URI
  • request body

Expected Behavior

One should be able to customize this behaviour.

Example: Separate cache entry per authorization header.

Possible Solutions

Introduce a CacheKeyGenerator interface which would receive the request as a parameter.
It should be an optional parameter of the plugin, falling back to the current implementation.

@dbu
Copy link
Contributor

dbu commented Mar 21, 2017

👍
I think the key generator should also have the possibility to say a request is not cacheable. Maybe by throwing a specific exception, as that is nicer than returning null to indicate we don't want the request to be cached.

@Nyholm
Copy link
Member

Nyholm commented Mar 21, 2017

👍

I think the key generator should also have the possibility to say a request is not cacheable.

Maybe... or we put that functionality in another class.

@dbu
Copy link
Contributor

dbu commented Mar 21, 2017 via email

@sagikazarmark
Copy link
Member Author

I agree with @Nyholm, it's a separate responsibility. The CacheKeyGenerator can throw an exception though which can result in cacheability to be impossible, but that decision needs to be made elsewhere.

Actually I forsee that we are going to move the entire caching stuff to the message package and just wrap it in the plugin. Not sure if it makes sense, but we are putting tons of effort into it, would be nice to see if the caching logic can be used elsewhere or not.

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.

3 participants