Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugins/Interceptors Don't Work with Early Stage Single Instance Objects in Developer Mode #2674

Closed
astorm opened this issue Dec 10, 2015 · 7 comments
Assignees
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@astorm
Copy link

astorm commented Dec 10, 2015

If I enable developer mode by adding the following to my .htacess file

SetEnv MAGE_MODE developer

and then add a plugin configuration

#File: app/code/Pulsestorm/Testbed/etc/di.xml
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:App/etc/routes.xsd">
    <type name="Magento\Framework\Logger\Monolog">
        <plugin name="namespace_vendor_magento_framework_logger_monolog" type="Pulsestorm\Testbed\Plugin\MagentoFrameworkLoggerMonolog" />
    </type>
</config>

Magento doesn't seem to see the plugin until I do a full compile

php bin/magento setup:di:compile

Expected Behavior: When Magento developer mode is enabled, all plugins are active immediately. From an end-client-programmer-user perspective the plugin system should behave the same in all Magento modes (developer, production, default, etc.) irrespective of the implementation details

Actual Behavior: Magento doesn't recognize certain plugins until the setup:di:compile command is run

Notes

I believe this problem is limited to what I'd call early stage single-instance-object instantiation. If the object manager creates an instance/singleton object (i.e. via get) before the framework loads the di configuration, that instance/singleton is cached in the object manager. Since the object manager, by definition, caches instance/singleton objects, this means these early stage objects never have a chance to be intercepted in developer mode.

That is, in

#File: vendor/magento/framework/Interception/ObjectManager/Config/Developer.php
public function getInstanceType($instanceName)
{
    $type = parent::getInstanceType($instanceName);
    if ($this->interceptionConfig && $this->interceptionConfig->hasPlugins($instanceName)) {
        return $type . '\\Interceptor';
    }
    return $type;
}

The $this->interceptionConfig is false for a time. In a quick test on the 2.0 system, that means the following classes are "un-intercept-able" by the developer mode object manager (without a compile)

Magento\Framework\App\ObjectManager\ConfigLoader
Magento\Framework\App\Cache\Type\Config
Magento\Framework\App\Cache\Type\FrontendPool
Magento\Framework\App\Cache\Frontend\Pool
Magento\Framework\App\Cache\Frontend\Factory
Magento\Framework\Filesystem
Magento\Framework\Filesystem\Directory\ReadFactory
Magento\Framework\Filesystem\Directory\WriteFactory
Magento\Framework\App\ResourceConnection\Proxy
Magento\Framework\ObjectManager\Config\Reader\DomFactory
Magento\Framework\App\ObjectManager\ConfigCache
Magento\Framework\Cache\Frontend\Adapter\Zend
Magento\Framework\Cache\Frontend\Decorator\TagScope
Magento\Framework\Cache\Frontend\Decorator\Logger
Magento\Framework\Cache\InvalidateLogger
Magento\Framework\App\Request\Http
Magento\Framework\Stdlib\Cookie\PhpCookieReader
Magento\Framework\Stdlib\StringUtils
Magento\Framework\App\Route\ConfigInterface\Proxy
Magento\Backend\App\Request\PathInfoProcessor\Proxy
Magento\Framework\Logger\Monolog
Magento\Framework\Logger\Handler\System
Magento\Framework\Filesystem\Driver\File
Magento\Framework\Logger\Handler\Exception
Magento\Framework\Logger\Handler\Debug
Magento\Framework\Cache\Frontend\Adapter\Zend
Magento\Framework\Cache\Frontend\Decorator\TagScope
Magento\Framework\Cache\Frontend\Decorator\Logger
Magento\Framework\App\Cache\Type\AccessProxy
Magento\Framework\App\Cache\State
Magento\Framework\App\DeploymentConfig\Writer
Magento\Framework\App\DeploymentConfig\Reader
Magento\Framework\Config\File\ConfigFilePool
Magento\Framework\Config\Scope
Magento\Framework\App\AreaList\Proxy
Magento\Framework\Interception\Config\Config
Magento\Framework\ObjectManager\Config\Reader\Dom\Proxy
Magento\Framework\Config\Scope

Not sure what the right fix here is. My naive approach to something like this would be to

  1. Limit the pre di configuration loading use of the object manager
  2. Formally introduce the concept of stages into the object manager (pre-di.xml, pre area-di.xml, etc.)
  3. Add tooling that warns developers if they're trying to plugin to one of these classes.

That said, as a third-part I wouldn't presume to come up with a solution myself as this is the sort of thing that will have impact system wide across multiple teams.

Right now if developers are trying to plugin to one of the above classes while working in developer mode they're going to spin their wheels for a bit before realize them need to run setup:di:compile even though developer mode is supposed, and sometimes does, make that unnecessary. That's the problem I'd be interested in seeing solved.

@astorm astorm changed the title Early Stage Plugins/Interceptors Don't Work in Developer Mode Plugins/Interceptors Don't Work with Early Stage Single Instance Objects in Developer Mode Dec 10, 2015
@ghost
Copy link

ghost commented Dec 16, 2015

@astorm I'm facing similar issues but I dont have a problem with plugins but with <preference ..> for Magento\Framework\Logger\Handler\Base, but I strongly belive this is related

@astorm
Copy link
Author

astorm commented Dec 16, 2015

@xhallix Yes, it's the same problem. Basically, the developer mode object manager needs to instantiate some classes before the di.xml files are fully loaded. That includes class preferences. Compile your di stuff with setup:di:compile and you should be set. (also, if possible, considering not using a preference. If there's a public method available a Plugin is going to be more supportable/sustainable long term)

@antonkril antonkril added bug report and removed TECH labels Dec 17, 2015
@ishakhsuvarov
Copy link
Contributor

Thank you for reporting.
We have created an internal ticket MAGETWO-47009 to investigate and fix the issue.

@ishakhsuvarov ishakhsuvarov added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development PS labels Dec 17, 2015
@dsikkema
Copy link

dsikkema commented Jan 6, 2016

@astorm how did you get that list of which classes won't be interceptable?

@astorm
Copy link
Author

astorm commented Jan 6, 2016

@ColdGreenTea I dropped the following debugging code into my system

#File: vendor/magento/framework/Interception/ObjectManager/Config/Developer.php0
public function getInstanceType($instanceName)
{
    $type = parent::getInstanceType($instanceName);
    //start debugging code
    if(!$this->interceptionConfig)
    {
        file_put_contents('/tmp/test.log',"$type\n",FILE_APPEND);
    }
    //end debugging code
    if ($this->interceptionConfig && $this->interceptionConfig->hasPlugins($instanceName)) {
        return $type . '\\Interceptor';
    }
    return $type;
}

In plain english, this code basically says

Hey, if there's no interception config object loaded yet, log the requests class to a file.

Then, I ran through some user flows in the browser on both the frontend and backend, then sorted and uniqed the log file.

dsikkema-magento pushed a commit that referenced this issue Jan 8, 2016
…y Stage Single Instance Objects in Developer Mode
dsikkema-magento pushed a commit that referenced this issue Jan 8, 2016
…y Stage Single Instance Objects in Developer Mode
okorshenko pushed a commit that referenced this issue Jan 20, 2016
…y Stage Single Instance Objects in Developer Mode

 - 2.0.1 release
@sshrewz
Copy link

sshrewz commented Mar 17, 2016

This issue has been resolved and merged to develop. Feel free to let us know if you encounter any other issues.

@sshrewz sshrewz closed this as completed Mar 17, 2016
magento-engcom-team pushed a commit that referenced this issue Jun 9, 2018
…amespace-agnostic

[borg] MAGETWO-92468: Make PR testsuite namespace-agnostic
@Laiba-Gillani
Copy link

encountering same issue when trying to overload sortRatesByPrice() core function of \Magento\Shipping\Model\Rate\Result Class.
In script its returning prefered method but on frontend its returning core method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

7 participants