Skip to content

Fix autoloader issue using Prefix mechanism#9541

Merged
wilsonge merged 5 commits intojoomla:stagingfrom
v-escobar:feature/autoloader
May 7, 2016
Merged

Fix autoloader issue using Prefix mechanism#9541
wilsonge merged 5 commits intojoomla:stagingfrom
v-escobar:feature/autoloader

Conversation

@v-escobar
Copy link
Copy Markdown

@v-escobar v-escobar commented Mar 23, 2016

Pull Request for Issue #9523

Summary of Changes

Before this pull request, if the class that Joomla was trying to load only contains a capital word except the prefix, the autoloader method duplicates that word, so it forces to have a folder on the root of the path where the prefix is pointing to. Now it tries to search on the root the path a PHP file called the same as the capital word on lowercase, if it does not find it, it works as before, making this patch backwards compatible.

Testing Instructions

In order to test this patch, you need to use a component that has a class on the root of the folder and register a prefix on that folder, for example, com_test and the class would be called TestClass and it will be implemented on class.php. In order to test backwards compatibility, you can put that class on a folder call class inside of com_test folder.

…f the parts variable only contains one element, we tries with that one element, but in order to be B/C, I have added a few extra statements that works as before this PR.
@brianteeman
Copy link
Copy Markdown
Contributor

It would really help people to test this if you could provide a sample component thy can test with

@brianteeman
Copy link
Copy Markdown
Contributor

Can you take a look at fixing the code style error please

FILE: /home/travis/build/joomla/joomla-cms/libraries/loader.php

FOUND 1 ERROR(S) AFFECTING 1 LINE(S)

605 | ERROR | Expected "if (...)\n"; found "if(...)"

@v-escobar
Copy link
Copy Markdown
Author

@brianteeman Thanks for point that out. I have solved that issue

@v-escobar
Copy link
Copy Markdown
Author

I have created this component using Component Creator.

http://www.filedropper.com/comtest-100_2

You just need to go to the front-end of your Joomla installation and trying to execute a controller task called test (/index.php?option=com_test&task=classes.test). If you see a message, it means the patch is working as intended. Try other component to detect the patch is backwards compatible.

Thank you

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Mar 23, 2016

@vistiyos Please merge https://github.com/vistiyos/joomla-cms/pull/1 as it adds unit test coverage for the new behavior and validates the current behavior does not break.

With that PR applied, testing also becomes much easier. Without this patch, JFactory has to be explicitly loaded in the libraries bootstrap because it doesn't meet the folder/file structure required right now. With this patch applied, it is autoloaded.

Unit test loading behavior, don't explicitly import JFactory now
@v-escobar
Copy link
Copy Markdown
Author

@mbabker Merged. Thanks for those tests.

@jsanchezgr
Copy link
Copy Markdown

I have tested this item ✅ successfully

@v-escobar
Copy link
Copy Markdown
Author

Any news about this? The patch passes unit tests, so I don't know if it can be merged.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented May 7, 2016

I have tested this item ✅ successfully on 07dd724

Thanks.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9541.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented May 7, 2016

RTC. Thanks


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9541.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 7, 2016
@brianteeman brianteeman added this to the Joomla 3.6.0 milestone May 7, 2016
@wilsonge wilsonge merged commit de65813 into joomla:staging May 7, 2016
@brianteeman brianteeman removed the RTC This Pull Request is Ready To Commit label May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants