Compatibility with Magento 2.4.1
Closed this issue · 9 comments
Hi,
Having tried out Magento 2.4.1-beta2, it appears that the module no longer generates interceptors for anything else besides "global" and "primary" scope, all other frontend, adminhtml etc receive the exact same content for interceptors configuration.
The root cause of this is because the gathering plugin information by scope was moved out of the PluginList class
magento/magento2@7e4a72e
magento/magento2@11c5a67
A new class called PluginListGenerator (https://github.com/magento/magento2/blob/2.4-develop/lib/internal/Magento/Framework/Interception/PluginListGenerator.php) was born, which has its own instance of ScopeConfig.
The current compiled-interceptors flow is based on creating instances of PluginList and providing it with a custom ScopeConfig implementation -> https://github.com/creatuity/magento2-interceptors/blob/master/Generator/AreasPluginList.php#L51
This worked very well before, as PluginList was responsible for gathering the data about interceptors in different scopes, but as of M 2.4.1, this functionality is outsourced to PluginListGenerator, which has its own understanding of scope.
In native Magento there is only one instance of PluginList and then the scopeConfig is updated with scopes to generate the specific scope. Compiled interceptors module creates several instances of these with custom scope values and aggregates them later.
Now because everything is declared private in PluginListGenerator, it is not a trivial task to fix it in a way that the generator is not completely overridden by custom implementation.
As a solution, one possibility is to patch the core PluginList and PluginListGenerator to make it possible to switch the scope in run-time for the generator:
+++ Interception/PluginListGenerator.php 2020-10-02 15:03:39.000000000 +0300
@@ -267,6 +267,14 @@
}
/**
+ * @param $scopeCode
+ */
+ public function setCurrentScope($scopeCode)
+ {
+ $this->scopeConfig->setCurrentScope($scopeCode);
+ }
+
+ /**
* Collect parent types configuration for requested type
*
* @param string $type
--- Interception/PluginList/PluginList-o.php 2020-09-17 17:31:14.000000000 +0300
+++ Interception/PluginList/PluginList.php 2020-10-02 15:05:55.000000000 +0300
@@ -231,6 +231,7 @@
$this->_loadedScopes[$scopeCode] = true;
}
} else {
+ $this->pluginListGenerator->setCurrentScope($scope);
[
$virtualTypes,
$this->_scopePriorityScheme,
However, taking possible future changes and modularity into account, this goes more into the "ugly hack" side of things.
EDIT: now there is a bit more decent PR available: #4
On each generation run, current scope is forced into the generator, so it will generate all scopes correctly. The hard bit here is that the PluginListGenerator has shared parts, but then again, the scope needs to change on each run, otherwise we could have multiple instances of PluginListGenerator as well (each PluginList would have its own).
I was wondering, perhaps you have some ideas how to solve this in a more decent way. I am also happy to create a PR in case I figure it out meanwhile.
P.S. This is a very good module that you have and I would love to see this as part of Magento at some point.
Hi @siimm ,
Thanks a lot for this detailed description and your contribution.
I will test this on 2.3 and 2.4 instances when I will have a chance.
Have you tried version that is submitted as a pull request magento/magento2#22826 ?
I have stopped following the PR but saw some changes there and I am not completely sure why they were introduced but maybe they are related to changes in 2.4.1 you are referring to.
Also, I am hoping this will be merged so this module is temporary in some sense and there is no problem with "ugly hacks" as long as they work. The way scope mechanism is working at interceptors generation time is tricky and making it work here was all in the 'hackish' area.
cheers.
Hi @fsw ,
This reminds me that it is not even worth checking on 2.3 and 2.4, as there core code differs and the current PR will probably break it.
Even on 2.4 the module worked as it is, but after trying 2.4.1-beta2 it started to misbehave and I started to investigate.
It probably makes sense to add some version constraint together with this PR, so it could not be installed <2.4.1 versions of Magento.
About the magento/magento2#22826 - I have tried to backport it to Magento 2.2.5 (back when we were using 2.2.5) but that took incredible amount of time (due to 2.2.5 and 2.3 differences) and we ended up just upgrading to 2.3 and including the current module. A bit sad that the PR is not merged due to people are arguing about minor formatting details etc there for over a year.
The current attached PR is a bit less hackish than the solution provided in the issue body (but still changing private properties of a class), but at the moment it is not produciton-tested yet, only in local dev env things "started to work" after these were introduced, but I will try to keep it up to date as soon as I find anything.
Hi @siimm ,
If the PR would break M 2.3 then I am closing it so it wont hang.
I am leaving this issue open till someone will find a solution to make it work on 1.4.1 without breaking previous versions.
If you will work it out please feel free to create a new one or maybe I will get to it one day and I will base it on your solution.
Thanks for your contribution.
Hello, what is the current status of this module - is it compatible / required for 2.4.1-p1 (the version we are running right now)...
thanks!
Hello, what is the current status of this module - is it compatible / required for 2.4.1-p1 (the version we are running right now)...
thanks!
The same question
hi @pmsteil @vadim-kozlovskiy-ewave
From my perspective (not being the maintainer of the module): The module works on 2.4.1 in case changes from #4 are applied.
P.S. Unfortunately I have not had the time to make the PR 2.3.x compatible, so this won't probably be part of a release any time soon. But I see it could be done fairly easily by incorporating a version check in the code.
out of the box, the latest release 1.2.1 is not M2.4.1 compatible
Looks good! class_exists is even simpler than comparing versions 👍
Sorry for delay here.
It seemed to work at first glance but I had issues with it in compiled production mode.
I marged this and other pending changes into master branch but to avoid breaking Magento 2.3 instances we will now manage two separate versions of the module:
use 1.3.* for Magento 2.4 (branch: master)
use 1.2.* for Magento 2.3 (branch: magento23)
this was also included in readme.
Master branch with 1.3 should now work with Magento 2.4