[#5189] - Apply on-the-fly class aliases, even for Composer packages
- Closed
- 2 Dec 2014
- Medium
- Build: staging
- # 5189
- Diff
- nikosdion:fix/5179-class-aliasing-in-autoloader
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Fixes gh-5179
Technical description
The libraries/classmap.php file registers class aliases with JLoader. This works fine when a developer creates an object using the old class name (e.g. JRegistry) instead of the new class (e.g. Joomla\Registry\Registry). However, this causes an issue when PHP doesn't try to autoload a class, e.g. when the old class name is in the right hand side of instanceof. Example:
$foo = new \Joomla\Registry\Registry();
var_dump($foo instanceof \Joomla\Registry\Registry); // returns true
var_dump($foo instanceof JRegistry); // returns false
However, if you have forced the autoloader to run you have no problem. Example:
$foo = new \Joomla\Registry\Registry();
class_exists('JRegistry', true); // Dummy line just to force the autoloader to run
var_dump($foo instanceof \Joomla\Registry\Registry); // returns true
var_dump($foo instanceof JRegistry); // returns true now!!!
We can't use static class_alias() calls because PHP needs to load the original class, causing memory and performance issues. Instead, we need a way to create the aliases on-the-fly, when either then old class is loaded (via the dynamic alias set in JLoader) or through the new namespaced class name (through Composer).
This patch uses a fork of the \Composer\Autoload\ClassLoader class which calls JLoader::applyAliasFor($class) when loading a class. This allows us to register class aliases on the fly, when the new, namespaced class is being loaded. Moreover, the same trick is applied to all of the JLoader autoloaders. This results in perfect backwards compatibility while we are proceeding with the namespacing of the core code.
Backwards compatibility
This patch actually fixes backwards compatibility :)
Performance impact
I could not measure performance impact, not because of lack or try or incompetence but because of variance. Therefore I conclude that the performance impact is statistically not important. In layman's terms: it's so small you can't even separate it from the variance in page load times added by external factors such as filesystem caches, code caches, web server thread management etc.
As for the memory impact, it is about 0.2Kb per aliased class. Even with hundreds of classes the memory impact is miniscule compared to the core code memory footprint.
jissues-bot
- | Labels |
Added:
?
|
||
ClassLoaderJoomla is a fork of Composer's ClassLoader. Also, the style errors in loader.php are in other methods, not what I touched. Sorry, I honestly don't have the time to go through all of the style issues in 3PD code within the timeframe to implement this fix for a major backwards compatibility issue :(
sorry @nikosdion
Just add the file to the ignore list in https://github.com/joomla/joomla-cms/blob/staging/build/phpcs/Joomla/ruleset.xml
I am not sure in which rule I should add the exceptions
At the top where we have the exclude-patterns, that tells the sniffer to always ignore files matching those paths.
@mbabker Thanks, I believe I added the files in the correct place
@chrisdavenport Thanks for the heads up! I have fixed the file headers to indicate the code's origin.
| Category | ⇒ | Libraries |
@test before patch 3.40alpha and akeeba backup doesnt work
after patch it does
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5189.
| Status | Pending | ⇒ | Ready to Commit |
two successful tests + travis is happy now --> RTC
Thanks
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5189.
@nikosdion I experienced a single "glitch" that I think is related to this PR. Unhappily I've been totally unable to duplicate it. In case you want to dig deeper, this is what happened:
- I was willing to understand the nature of issue #5218
- I logged into the back-end
- I gave a quick look at com_patchtester just to check that no other patch was enabled beside this one, which I keep enabled for Akeeba Backup (the sole extension installed)
- I clicked on Components -> Contacts
- The "Contacts" page opened but I also received the following "Warning" message:
Warning: Cannot redeclare class JStringNormalise in D:\UniServerZ\vhosts\smztest\libraries\loader.php on line 513
As you can see the environment is Windows and the WAMP stack is Uniform Server, with PHP 5.3.29
As I said, I tried several times, even logging-out and back in, resetting my browser, and also shutting down and restarting Apache, but I've been unable to get that message again!
If there anything I can do, please let me know...
Ahhh!!! I found another page that triggers this "Warning", always:
Install Akeeba Backup 4.1.0.rc3, go to the "Control Panel" and then click on "Options": same "Warning"!
Here it is:
I don't think this a problem in Akeeba Backup, as the first time I got it in "Contacts"....
I Think we get it in every "Option" page:
Maybe my memory is failing me and the first time I was in "Contacts -> Options"...
... and reverting this PR the warning disappears.
Sergio, the PR is NOT written for Joomla! 3.3. It must be installed on Joomla! 3.4 alpha.=
@nikosdion Do you really think I've tested the last 10+ PRs on 3.3? 
@infograf768 Should I refresh 3.4.0-dev with the latest staging? Are you on Windows? PHP?
Mac here, not Windows
OK: give me some minutes to refresh with the latest staging...
i just got the same error message on the first page i visited after applying the patch
this was with the alpa release
now testing with current staging
confirmed that i got the error with staging as well
see video https://www.dropbox.com/s/v9q5ta7zpp9iqo3/ScreenFlow.mp4?dl=0
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5189.
removing the RTC setting for now
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5189.
| Status | Ready to Commit | ⇒ | Pending |
... while it disappeared here... 
I see what was going on and fixed it. The loadByAlias would execute class_alias(self::$classAliases[$class], $class); but the regular class (self::$classAliases[$class]) wasn't already loaded. So this line would first trigger the autoloader of the regular class which, in turn, it triggers the applyAliasFor which defines the class alias. So by the time this line would be executed by PHP both the original class and the alias would be loaded, hence the warning.
The fix was simple. I just call class_exists on the regular class first, then check if the alias is already set. By separating these two steps we can prevent this warning from displaying.
someone pleas explain me why it disappeared here! 
I will explain: 'cause I'm a moron: I forgot to apply the patch!!!
Will test again shortly
On 27 Nov 2014 11:57, "Sergio Manzi (smz)" notifications@github.com wrote:
@test https://github.com/test confirmed OK with latest commit
—
Reply to this email directly or view it on GitHub
#5189 (comment).
@infograf768 Can it be that you didn't have E_WARNING in your php.ini?
@test retested and all good now - setting back to RTC
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5189.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-12-02 19:13:48 |
Merged into staging. Thanks!
| Labels |
Removed:
?
|
||
joomla-cms-bot
- | Labels |
Added:
?
|
||
@nikosdion
can you have a look into Travis?
https://travis-ci.org/joomla/joomla-cms/jobs/41976583