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

Added support for cache keys that are not hashed via config option READABLE_CACHE_KEYS #674

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

rdiaztushman
Copy link
Contributor

fixes #672

@telephone
Copy link
Contributor

A few things:

  1. You forgot to add the "READABLE_CACHE_KEYS" var to hive, with a default of FALSE
  2. There's no need for an !empty() check, a simple $this->hive['READABLE_CACHE_KEYS']?$key.'.var':$this->hash($key).'.var' works.
  3. "READABLE_CACHE_KEYS" is a bit too long for my taste. I'd prefer "HASH_KEYS" (If used, you'd reverse the hive default to TRUE, and change the ternary operator to suit).

@rdiaztushman
Copy link
Contributor Author

I was deliberately trying to 1) minimize the number of changes and 2) preserve current behavior. That's why I didn't add the key to the hive (which means I needed to use empty). There is no change in F3's behavior unless you add READABLE_CACHE_KEYS to your config with a value of true or 1.

@rdiaztushman
Copy link
Contributor Author

Changes keys in the cache from this:
f3_hashed_key

to this:
f3_readable_key

which makes them much easier to manage in our admin.

@telephone
Copy link
Contributor

I was deliberately trying to 1) minimize the number of changes and 2) preserve current behavior. That's why I didn't add the key to the hive (which means I needed to use empty). There is no change in F3's behavior unless you add READABLE_CACHE_KEYS to your config with a value of true or 1.

Leaving an undocumented variable check in an open source framework isn't the way. When someone browses the source they'd be met with a non-existent key and probably left scratching their heads (as I was when I viewed your commit).

By adding the key to hive, the behaviour stays identical and leaves behind a nicer trail to follow.

I agree with this toggle to switch hash on/off, as I've implemented it in userland code a few times before via Cache::, and it'd be handy for those not well versed with the source.

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.

None yet

2 participants