Improve identity driver class resolution logic#110
Open
TDannhauer wants to merge 2 commits into
Open
Conversation
Enhance identity driver class loading by checking multiple class names and requiring a file if necessary.
This test suite verifies the functionality of the Horde_Core_Factory_Identity class, ensuring correct resolution of identity drivers and handling of various scenarios, including driver creation and error throwing for unknown drivers.
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.
Summary
Fix application-specific identity driver resolution in
Horde_Core_Factory_Identityso drivers whose class names use an uppercase application prefix (notably IMP) are found reliably, including when the identity class has not yet been autoloaded.Problem
When code requests an application identity via
Horde_Core_Factory_Identity::create(null, $driver), the factory builds the class name as:For the
impdriver this yieldsImp_Prefs_Identity, while the actual class defined in IMP isIMP_Prefs_Identity.That mismatch surfaces as:
When it shows up
The failure is easiest to reproduce on a "cold" request where
IMP_Prefs_Identityhas not been loaded yet:IMP_IdentityIMP_Factory_Identity→Horde_Core_Factory_Identity::create(null, 'imp')without a prior full IMP page loadOnce
IMP_Prefs_Identityis loaded, PHP's case-insensitive class aliases can mask the bug, which makes it intermittent and environment-dependent.Root cause
impImp_Prefs_IdentityIMP_Prefs_Identityclass_exists('Imp_Prefs_Identity')returnsfalseuntil the real class file is loaded. Composer's classmap indexesIMP_Prefs_Identity, notImp_Prefs_Identity, so autoload does not help for the wrong name.Solution
Extend the default branch of
create()to:ucfirst($driver)andstrtoupper($driver)before failing.require_once{fileroot}/lib/Prefs/Identity.phpfrom the registry and re-check both candidates withclass_exists(..., false).This keeps existing behaviour for normal apps (
kronolith→Kronolith_Prefs_Identity) and fixes acronym-style apps (imp→IMP_Prefs_Identity) without hard-coding IMP.Changes
File:
lib/Horde/Core/Factory/Identity.phpBefore (default branch):
After (default branch):
{Ucfirst}_Prefs_Identity,{UPPER}_Prefs_Identity{registry fileroot}/lib/Prefs/Identity.phpif present"{driver} identity driver does not exist."The special-case handling for
driver === 'horde'(Bug #9936 /Horde_Prefs_HordeIdentity) is unchanged.Test plan
Horde_Core_Factory_Identity::create(null, 'imp')returnsIMP_Prefs_Identityon a fresh request (no prior IMP bootstrap).$injector->getInstance('IMP_Identity')succeeds when IMP factories are bound.create(null, 'kronolith'),create()(defaultHorde_Core_Prefs_Identity),create(null, 'horde').create(null, 'nonexistent_app').IMP_Identity/ "identity driver does not exist" errors in Horde log.Suggested unit test (optional)
Backward compatibility
Horde_Core_Factory_Identity::create().{Ucfirst}_Prefs_Identitynaming are unaffected.Related
IMP_Factory_Identityinhorde/impcallscreate(null, 'imp').strtoupper($driver).