plugin sort order dependencies

[airbone42]

Hi,

I'm currently playing around with the plugin interceptors. Currently the only way to handle the order of plugins is by the sortOrder attribute. Unfortunately that's very lmiited as it only accepts a hardcoded integer value. Especially if you have dependencies between different modules it's very risky to use these, as if some other module changes it's sort order your extension might break.

Would Magento be interested in a solution where you could define a before or after attribute (similar to the layout blocks) to define if a plugin should be run before or after any other specific plugin? For me that would be a great solution to also handle bigger dependency chains and it's less likely to break if when the amount of plugins is growing. What drawbacks can you see in this solution?

[antonkril]

What should happen if module that your plugin depends on (goes "after") is not installed?

[airbone42]

An idea would be to write that in the error.log. I would not throw an exception, so it could be an optional dependency. If it's a hard dependency that module itself should already depend on the other module.

[antonkril]

And if you want to go both after Plugin A and Plugin B?

[airbone42]

That's more a question about the syntax. Separate it by some separator or let's do it as sub-elements, not an attribute? I first wanted to ask about the concept of that idea, if there's any reason you didn't think about it? If you think it's a good idea it's definitely time to get one step further and think about a concrete implementation (including syntax and exceptions)

[CodeMonkey90]

Great idea! I just read the documentation and immediately thought about all the possible problems. I especially fear that everyone will try to make sure his/her plugin is loaded as the first plugin and all plugins will end up having sortOrder="1". Just like in the Magento 1 backend, where everyone tries to make his plugin appear at the top of the configuration page.

BTW: I would just silently ignore plugins which are not installed. Spamming the error.log file is unnecessary, since it makes a lot of sense to make sure your plugin works nicely with some other community plugins which it doesn't depend on.

[antonkril]

We had these two approaches used in Magento 1. The problem with before/after approach is that sort order of your plugin depends on presence of other plugin in system. But if that plugin is not present, your sort order becomes undefined. So we choose approach with sort order. This way its sort order is always defined. But if you see problems with this, we can discuss syntax for before/after.

[CodeMonkey90]

Okay, if none of the specified modules exist, then the sort order is undefined, of course. But I don't think that's a problem, since in this case the module developer obviously doesn't care about the sort order.

Problems with the current approach

Two examples to illustrate problems with the current approach that would be solved by the above proposal:

Scenario 1

Imagine a Magento installation with a lot of 3rd party plugins (I maintain one of those). Plugin A and B both have a sort order of 1.¹ Now I need to write a plugin that runs between those two. Entirely impossible!

If you'd implement airbone42's proposal, OTOH, both A and B wouldn't specify any ordering (because their developers obviously don't care about the sort order) and my plugin could specify that it needs to run between those two.

Of course, there could be cases where the ordering requirements of two plugins conflict. But in that case, the two modules are obviously incompatible and cannot be used together anyway.

Scenario 2

I have installed two third-party plugins. The developers of those modules didn't know or care about each others module, but during testing I discover that one of them must run before the other. I can now simply create a third module with a dummy plugin and specify that it must be run between those two: Problem solved.

My Proposal (formal description)

Let plugin developers specify that their plugin needs to run before or after certain other plugins. Build a directed acyclic dependency graph from that information (if the dependency graph has cycles, die with an appropriate error message). Then sort the plugins topologically and execute them in that order.

---

¹ Which is what most 3rd-party developers are probably going to specify, since they cannot possibly test their module's interaction with any other 3rd-party module. So, in practice, the sort order is neither well-defined nor sensible anyway, even if you use integers.

P.S.: Sorry for the multiple edits

[airbone42]

Landkeks already explained the problems with the current approach. Especially for supporting the wider community where modules are developed by different companies, which do not know of each other or just won't cooperate the current approach will lead to problems.

I would like to add a possible syntax example. To not mix attributes and elements too much I would move the current sortOrder attribute in a new child-element and set it's value as the attribute of that element. Additionally we could define multiple before and after elements.

<type name="Magento\Backend\App\AbstractAction">
    <plugin name="adminAuthentication" type="Magento\Backend\App\Action\Plugin\Authentication">
        <sortOrder value="100">
            <before value="plugin_name_a" />
            <before value="plugin_name_b" />
            <after value="plugin_name_c" />
        </sortOrder>
    </plugin>
</type>

If this leads to conflicts (e.g. circular dependencies or the sortOrder of a before plugin is higher than the current specified) we should ignore the before and after values and log this as a possible error.

[CodeMonkey90]

@airbone42
I wonder why you're suggesting we should keep the sortOrder attribute/element at all. I think mixing two different ways of defining the sort order is confusing and a sorting algorithm which takes both into account sounds hard to implement. I'd drop the sortOrder completely and order based on the before and after elements. Those state the intent of the developer more clearly and allow efficient topological sorting.

<type name="Magento\Backend\App\AbstractAction">
    <plugin name="adminAuthentication" type="Magento\Backend\App\Action\Plugin\Authentication">
        <before value="plugin_name_a" />
        <before value="plugin_name_b" />
        <after value="plugin_name_c" />
    </plugin>
</type>

looks better to me.

[airbone42]

Good point, I thought more about backwards compatibility, but that's non-sense in a system which is in that early stage and when changing the syntax. Thank god it's friday :D

I would anyway encapsulare the before and after elements in some kind of sortOrder-parent (without a value) to be more flexible when other child types will be added. But that's more a personal preference and has no real technical reason.

[airbone42]

So as an alternative: It's possible to have plugins of plugins, so everyone can also execute something before, after or around another plugin. Just looks weird to have method names like beforeAfterFilter.

The question is: Is this the best practice? Shouldn't we remove in this case the sortOrder attribute at all to force one way of solve these conflicts? The hardcoded integer sortOrder solution is quite instable if you work with 3rd party extensions you have no influence to.

[CodeMonkey90]

@antonkril Have you decided whether you'll implement this proposal yet? I think this is an important change that should rather be made sooner than later (esp. since some people have already started porting their modules to Magento 2).

[vpelipenko]

Internal ticket: MAGETWO-32336

[alankent]

As we get closer to M2 end of dev beta we want to stabilize things. Until then it is a question of priorities. What is not clear is how often plugins will be on same method, and how often the ordering in such cases matters. This would then influence priority of such work over other work. Its not a question of "is this a good idea", but rather "is this more important than what it would replace on the backlog". (If the community does it, then that overhead is lower - it is more "is it the right solution").

So "what are our plans here"? There is not a single answer yet, but personally my question is how big an issue is this. How often is it going to be used? I would like to understand if the before/after is really going to fix the problem. Does it matter for plugin 1 to be before or after plugin 2? (Is it a problem in 1.X with event observers?) If not, it may be we just make sort order optional and discourage its use by extension developers. Leave it for a system integrator / developer to resolve conflicts in the rare case it matters.

I personally think before/after makes more sense as it talks about specific combinations, implying the developers talked together and worked out between them which was important to come first or last. I would love to hear some examples of why it is important as I think that would help verify the technical solution proposed. (E.g. if its for two extension developers to work out the ordering between them, great. If that would not happen, then it seems conceptually nicer but not that important in practice?)

The negative I can see is if extension developer 1 says "I should be first" and extension 2 says the same thing, then how to resolve that difference?

[CodeMonkey90]

Personally, I've had to solve rewrite conflicts in Magento 1 quite a few times. Since M2’s plugins are supposed to make rewrites unnecessary in many cases, there will most likely be multiple plugins on the same methods.

Does it matter for plugin 1 to be before or after plugin 2?

In some cases, it does (multiple plugins which affect the price calculation / one module sets a product attribute that another module depends on / …).

If not, it may be we just make sort order optional and discourage its use by extension developers. Leave it for a system integrator / developer to resolve conflicts in the rare case it matters.

I think you should do at least that before M2 is released (deprecate the sort order and provide some way to order plugins manually if a conflict arises). I suspect that in many cases the shop owner/developer will have to solve those conflicts manually anyway. One question, though: How are developers supposed to fix the ordering if no mechanism like the one outlined above is introduced?

The negative I can see is if extension developer 1 says "I should be first" and extension 2 says the same thing, then how to resolve that difference?

If that happens, the extensions are obviously not compatible and should not be used together. As I suggested above, M2 should treat that like an unresolved dependency and die with an appropriate error message.

[wsagen]

Actual rewrite conflicts can be eliminated in Magento 1, currently, in most cases using events etc but this doesn't rule out logic conflicts.

These are extremely frequent especially in areas of common functionality e.g. product prices, product attributes, quote object and order objects. Offering a before or after field is great however it requires the extension developer to have some view of what is already running on a system.

The ideal scenario is to be able to manually configure the order of execution via the admin panel. Either sort order or before/after field would work in this case. In the same way as you can currently view extension conflicts with the widely available Extension Conflict tool, you could view dependency conflicts and resolve.

As an extension developer, I would actually prefer to go last than first ;)

[joshuaswarren]

Now that a few M2 extensions have been released, I'm curious if anyone has seen a problem with this "in the wild", and if anyone has thought from an SI/developer point of view what you plan to do when you come across a site with a sort order conflict/issue like this? Obviously we could edit the sort order in the extension's XML file, but that means at every future extension update we have to re-apply our change which is less than ideal.

[piotrekkaminski]

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.