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

hash long cache keys #24

Closed
wants to merge 2 commits into from
Closed

Conversation

Adrian2112
Copy link

Some cache stores have a size limit for the keys. E.g ActiveSupport::Cache::FileStore
So in case there are a lot of parameters and the cache key is too long we hash it

@@ -112,7 +112,10 @@ def cache_clear

# Generate the request cache key.
def cache_key(*arguments)
"#{name.parameterize.gsub("-", "/")}/#{arguments.join('/')}".downcase.delete(' ')
arguments_string = arguments.join('/')
arguments_key = arguments_string.length > 150 ? Digest::SHA2.hexdigest(arguments_string) : arguments_string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you constantize this "150" somewhere?

@mhgbrown
Copy link
Owner

mhgbrown commented Jul 9, 2015

Also would appreciate if you could create a test for this

@Daniel-ltw
Copy link
Collaborator

@Adrian2112 could you bump the version for this change?

travis.yml have been updated and we should re-run your test.

@Adrian2112
Copy link
Author

I just rebased agains master. Does that work?

@Daniel-ltw
Copy link
Collaborator

Could this be optional?

Setup a configuration variable for it.
Only hash if the configuration is set to true and could probably default to true on ActiveSupport::Cache::FileStore?

Just wondering the use case and would everyone want to have hashed keys.

@jlurena
Copy link
Collaborator

jlurena commented Oct 15, 2024

Closing for #72

@jlurena jlurena closed this Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants