fix(cache): delegate legacy Hashtable storage to modern adapter#5
Closed
TDannhauer wants to merge 1 commit into
Closed
fix(cache): delegate legacy Hashtable storage to modern adapter#5TDannhauer wants to merge 1 commit into
TDannhauer wants to merge 1 commit into
Conversation
Horde_Cache_Storage_Hashtable still called get() with an array of keys, which breaks with Horde\HashTable\HashTable drivers that require a string. Route modern backends through Horde\Cache\HashtableStorage so ActiveSync and other cache users work with Memcache again.
Member
|
Mind #4 and related horde/Core#131 |
Member
|
Superseded by 3.0.0RC2 a4ebb16 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(cache): delegate legacy Hashtable storage to modern adapter
Summary
TypeErrorin ActiveSync and other code paths that use the legacyHorde_CacheAPI with a modernHorde\HashTable\HashTablebackend (e.g. Memcache).Horde_Core_Factory_Cacheinjects a PSR-4Horde\HashTable\HashTableinstance,Horde_Cache_Storage_Hashtablenow delegates toHorde\Cache\HashtableStorageinstead of calling the old array-basedget()API.Horde_HashTablebackends are unchanged and still use the existing code path.Problem
After the FRAMEWORK_6_0 HashTable modernization,
Horde\HashTable\Driver\Memcache::get()accepts only astringkey. The legacy cache storage adapter still called:That surfaced during ActiveSync requests via:
Horde_Perms_Sql->exists()→Horde_Cache->get()→Horde_Cache_Storage_Hashtable->get()→Memcache::get(array)Horde_Core_Factory_Cachealready documents routing modern HashTable instances throughHashtableStorage, but that was never implemented in the storage class.Solution
Detect
Horde\HashTable\HashTablein_initOb()and wrap it withHorde\Cache\HashtableStorage. Delegateget(),set(), andexpire()to that adapter, which correctly usesgetMultiple()and the modern TTL API.Test plan
Hashtablewith Memcache backend ($conf['cache']['driver'] = 'Hashtable',$conf['hashtable']['driver'] = 'Memcache').rpc.phpActiveSync endpoint).TypeError: Memcache::get(): Argument #1 ($key) must be of type string, array givenin Horde logs.Horde_HashTablebackends still work if used directly.