community-features
community-features copied to clipboard
Eliminate the need for inheritance for action controllers
By design, all Magento action controllers should implement \Magento\Framework\App\ActionInterface
. But some crucial request processing behavior (event dispatching, authorisation, etc) resides in classes like \Magento\Framework\App\Action\Action
and \Magento\Backend\App\AbstractAction
. So if an action controller implements the ActionInterface
but does not inherit from one of these "layer supertype" classes, it will lose that crucial behavior.
So, Magento module developers do not have a way to create an action controller without using inheritance. To avoid inheritance (see why), the request processing behavior should be extracted from "layer supertypes". Proposed solution is to move the behavior to action controller plugins.
Action controller should contain only its custom behavior.
AC:
- module developer does not have to extend from any class to create a fully functional action controller, implementing
\Magento\Framework\App\ActionInterface
is enough - controllers "supertypes" (
\Magento\Backend\App\AbstractAction
,\Magento\Framework\App\Action\Action
,\Magento\Framework\App\Action\AbstractAction
) are deprecated - magento supports both controller implementations (inheritance based and non-inheritance based)
- [optional] controllers are migrated
Original Report: https://github.com/magento/magento2/issues/9582 by @antonkril
Basically this feature request was already implemented
- ✔ implemented in https://github.com/magento/magento2/pull/26778 (thanks @vinai and @lbajsarowicz),
- ✔ documentation updated in https://github.com/magento/devdocs/pull/6901 (thanks @lbajsarowicz)
- ✔ new controllers should not use inheritance - enforced by static test in https://github.com/magento/magento2/pull/29339 (thanks @swnsma).
The only thing that still not done
- ❌controllers still weren't migrated to not use inheritance
Good job! 👍
Migrating controllers would be an unnecessary backward compatibility break. Why not keep them the way they are, and only enforce new ones to use composition?
@Vinai I would agree with you that migration of all controllers might be not necessary, but I'm strongly believe we should have good examples as much as possible in order to show to extension developers how to use controllers without inheritance and that it's best practice.
Also for some controllers it might be really needed, for instance for controller that handles ESI requests from varnish.
Was using "controller_action_predispatch" to forward requests, by doing something like this
$request->initForward()
->setRouteName('routename')
->setModuleName('modulename')
->setControllerName('index')
->setActionName('index')
->setDispatched(false);
This does not work any more since the event dispatching was moved to a plugin.
Before, the original matched actions execute()
was never run, because it checked if request was dispatched before running it. Now execute()
is always run.
Just wanted to report, in case it wasn't intentional to break it. And if some one else have the same problem, solved it by making a Router to handle forwarding instead.
Have you test your case again in latest code branch (2.4-develop github). New approach no-longer used plugin for event dispatch
controller_action_predispatch