ZF-4697: Bug in PluginLoader breaks two Zend_View UnitTests

Issue Type:
Bug
Created:
2008-10-23T19:27:16.000+0000
Last Updated:
2010-04-09T06:51:13.000+0000
Status:
Resolved
Fix version(s):
  • 1.10.2 (24/Feb/10)
Reporter:
Jon Whitcraft (sidhighwind)
Assignee:
Matthew Weier O'Phinney (matthew)
Tags:
  • Zend_Loader
Related issues:
Attachments:

Description

The two view tests that it breaks are

ViewTests.php -> testGetHelperPath() and testGetFilterPath()

The problem comes in with this code in the PluginLoader load() Method


if (class_exists($className, false)) {
    $found = true;
    break;
}

With the break it takes it out of the loop and never creates the $loadFile variable which sets the path to the class.

If you comment out this if statement everything is fine..

Comments

Posted by Jon Whitcraft (sidhighwind) on 2008-10-23T19:27:42.000+0000

Attached is the patch to fix this. If you want me to fix it then I will.

Posted by Alexander Veremyev (alexander) on 2008-11-06T08:23:33.000+0000

BTW It produces one unit tests error and one unit tests failure now:

{panel} 8) testGetHelperPath(Zend_ViewTest) strpos(): Empty delimiter {panel}

{panel} 12) testGetFilterPath(Zend_ViewTest) Failed asserting that two strings are equal. expected string </home/cawa/ZendFramework/svn/framework/trunk/tests/Zend/View/_stubs/FilterDir1/Foo.php> difference <??????????????????????????????????????????????????????????????????????????????????????> got string <> {panel}

Posted by Matthew Weier O'Phinney (matthew) on 2008-11-22T09:13:41.000+0000

This appears to be resolved in current trunk.

Posted by Jon Whitcraft (sidhighwind) on 2008-12-03T14:51:36.000+0000

OK I found how this bug is being produced.

If you call a helper by doing $mhc = new My_Helpers_Custom() before calling it from the helperBroker it produces the error.

I'm going to setup a test page and submit it for testing.

Posted by Jon Whitcraft (sidhighwind) on 2009-10-13T16:42:29.000+0000

Update to remove the Fix Version since this is not fixed.

Posted by Matthew Weier O'Phinney (matthew) on 2009-10-14T05:03:59.000+0000

All unit tests for Zend_View are running fine currently. What OS, PHP version, and ZF version are you seeing the issue on?

Posted by Jon Whitcraft (sidhighwind) on 2009-10-14T14:07:08.000+0000

Matthew,

This issue was submitted almost a year ago and I don't think it's valid any more. I forget what version of PHP I ran this on but it was for the 1.7.0 PR release.

I think this can be closed an a Non-Issue.

Posted by Michael Rehbein (tech13) on 2010-02-12T10:38:25.000+0000

Found a test case that shows the bug. Also created a patch to correct it. Will attach patch next.

Posted by Michael Rehbein (tech13) on 2010-02-12T10:40:33.000+0000

Patch: - adds a unit test for this issue - uses reflection to get which path the class was loaded from if it is already loaded

Posted by Matthew Weier O'Phinney (matthew) on 2010-02-23T09:04:00.000+0000

I'm reluctant to use this patch, as Reflection introduces substantial overhead -- and with something used as widely as the PluginLoader, this will have a lot of impact. In looking at your test case and patch, I discovered that we already have the code you describe: in getClassPath(). Since this is a method only used on-demand (not in every request), it makes sense to simply leverage this. Once I looked at that, I realized that I found the flaw: it was in how the $loadFile was detected and cached. I simply removed the logic to cache that information, and subsequently all tests, including your new one, pass.

Posted by Matthew Weier O'Phinney (matthew) on 2010-02-23T09:06:10.000+0000

Patch applied (with revisions) to trunk and 1.10 release branch.

Posted by Hendri Smit (hendri.smit) on 2010-02-25T06:48:44.000+0000

The changes made to the PluginLoader make my application crash Apache. I can't really point my finger on it but if add a simple isset() there is no problem :S


...
        if ($this->_useStaticRegistry) {
            self::$_staticLoadedPlugins[$this->_useStaticRegistry][$name]     = $className;
            isset($className);
        } else {
            $this->_loadedPlugins[$name]     = $className;
        }
...

Any thoughts?

Posted by Matthew Weier O'Phinney (matthew) on 2010-02-25T07:37:52.000+0000

@Hendri Actually, no thoughts. The lines you quote were never changed. I actually simply modified the load() method not to store the class filename; instead, it's only retrieved on-demand when you use getClassFile().

Can you provide a reproduce case that demonstrates the behavior? The ZF site itself is using 1.10.2, and we're not experiencing any crashes at this time.

Posted by Hendri Smit (hendri.smit) on 2010-04-09T06:51:13.000+0000

@Matthew There was probably something wrong with my Apache installation