Motivation
- Compiled interceptors respect variable amount of arguments (variadic).
- Before plugin must have possibility to change a number of arguments passed to all other plugins and original methods in a chain without losing arguments.
- Compiled interceptors respect OOP approach. It must be possible to extend public methods with additional arguments in children classes without any drawbacks.
- There is no such kind of issue in M2 OOTB.
Possible scenarios
- Before plugin changes amount of result arguments, which is not equal to all arguments listed in an original method. For example original method has 5 arguments, but before plugin returns only 2, when 3 other are optional and could be omitted. There would be an issue with undefined offset when it comes to the
list
construction.
- Before plugin returns more arguments compared to a list of arguments of an original method. Then all extra arguments would be lost as
list
construction expects certain number of arguments. All other further before / around / after plugins and original method would not receive correct list of arguments replaced by the first before plugin.
- More arguments are passed to the original methods compared to the method's declaration. Then before / around / after plugins as well as original method would receive only list of arguments declared for the original method. All extra arguments would be lost.
- Children class may extend any public method with extra arguments at the end of a declaration, then plugins, written for a parent class, would ignore all extra arguments declared in children class. Because of that original method may receive less amount of arguments compared to those, which reached interceptor.
Possible drawbacks
- With current approach (without requested changes) it is possible to design plugin with less strict definition compared to the original subject method. For example one can design code without respect to default values of original method. Compiled interceptors would pass all arguments to other plugins and original method without any error. So, basically it is possible to write plugin, which wouldn't work in clean M2, but it would work with compiled interceptors. But definition of the plugin wouldn't respect OOP principles. Plugin must respect number of arguments, their types and default values. After this PR merged such plugins would fail with error
Too few arguments to function <class::method>, <number_of_args> passed in <path_to_class_file>
, but same issue would happen with that class if we disable compiled interceptors and return back to native M2 interceptors.
Examples
Here we'd like to show example for plugin file with 2 plugins applied to \Magento\ImportExport\Model\Import\Entity\AbstractEntity
class such as addRowError()
and isNeedToLogInHistory()
.
Despite namespace is different plugins are still applied to \Magento\ImportExport\Model\Import\Entity\AbstractEntity
.
Plugin class generated with current (non-patched) version of compiled interceptors
<?php
namespace Vaimo\InventoryImport\Model\Import\Source\Satellite;
use Magento\Framework\Config\ScopeInterface;
use Magento\Framework\ObjectManagerInterface;
/**
* Interceptor class for @see \Vaimo\InventoryImport\Model\Import\Source\Satellite
*/
class Interceptor extends \Vaimo\InventoryImport\Model\Import\Source\Satellite
{
/**
* @var ScopeInterface
*/
private $____scope = null;
/**
* @var ObjectManagerInterface
*/
private $____om = null;
/**
* @var \Vaimo\DynamicScheduledImport\Plugin\Model\Import\Entity
*/
private $____plugin_vaimo_dynamic_import_enable_logging = null;
/**
* {@inheritdoc}
*/
public function __construct(\Magento\Framework\ObjectManagerInterface $____om, \Magento\Framework\Config\ScopeInterface $____scope, \Magento\InventoryImportExport\Model\Import\Serializer\Json $jsonHelper, \Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface $errorAggregator, \Magento\ImportExport\Model\ResourceModel\Helper $resourceHelper, \Magento\ImportExport\Helper\Data $dataHelper, \Magento\ImportExport\Model\ResourceModel\Import\Data $importData, \Magento\InventoryImportExport\Model\Import\Validator\ValidatorInterface $validator, array $commands = [])
{
$this->____om = $____om;
$this->____scope = $____scope;
parent::__construct($jsonHelper, $errorAggregator, $resourceHelper, $dataHelper, $importData, $validator, $commands);
}
/**
* {@inheritdoc}
*/
public function addRowError($errorCode, $errorRowNum, $colName = null, $errorMessage = null, $errorLevel = 'critical', $errorDescription = null)
{
$beforeResult = $this->____plugin_vaimo_dynamic_import_enable_logging()->beforeAddRowError($this, $errorCode, $errorRowNum, $colName, $errorMessage, $errorLevel, $errorDescription);
if ($beforeResult !== null) list($errorCode, $errorRowNum, $colName, $errorMessage, $errorLevel, $errorDescription) = (array)$beforeResult;
return parent::addRowError($errorCode, $errorRowNum, $colName, $errorMessage, $errorLevel, $errorDescription);
}
/**
* {@inheritdoc}
*/
public function isNeedToLogInHistory()
{
$result = parent::isNeedToLogInHistory();
return (($tmp = $this->____plugin_vaimo_dynamic_import_enable_logging()->afterIsNeedToLogInHistory($this, $result)) !== null) ? $tmp : $result;
}
/**
* plugin "vaimo_dynamic_import_enable_logging"
* @return \Vaimo\DynamicScheduledImport\Plugin\Model\Import\Entity
*/
private function ____plugin_vaimo_dynamic_import_enable_logging()
{
if ($this->____plugin_vaimo_dynamic_import_enable_logging === null) {
$this->____plugin_vaimo_dynamic_import_enable_logging = $this->____om->get(\Vaimo\DynamicScheduledImport\Plugin\Model\Import\Entity::class);
}
return $this->____plugin_vaimo_dynamic_import_enable_logging;
}
}
Plugin class generated with patched version of compiled interceptors (with changes proposed by this pull-request)
<?php
namespace Vaimo\InventoryImport\Model\Import\Source\Satellite;
use Magento\Framework\Config\ScopeInterface;
use Magento\Framework\ObjectManagerInterface;
/**
* Interceptor class for @see \Vaimo\InventoryImport\Model\Import\Source\Satellite
*/
class Interceptor extends \Vaimo\InventoryImport\Model\Import\Source\Satellite
{
/**
* @var ScopeInterface
*/
private $____scope = null;
/**
* @var ObjectManagerInterface
*/
private $____om = null;
/**
* @var \Vaimo\DynamicScheduledImport\Plugin\Model\Import\Entity
*/
private $____plugin_vaimo_dynamic_import_enable_logging = null;
/**
* {@inheritdoc}
*/
public function __construct(\Magento\Framework\ObjectManagerInterface $____om, \Magento\Framework\Config\ScopeInterface $____scope, \Magento\InventoryImportExport\Model\Import\Serializer\Json $jsonHelper, \Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface $errorAggregator, \Magento\ImportExport\Model\ResourceModel\Helper $resourceHelper, \Magento\ImportExport\Helper\Data $dataHelper, \Magento\ImportExport\Model\ResourceModel\Import\Data $importData, \Magento\InventoryImportExport\Model\Import\Validator\ValidatorInterface $validator, array $commands = [])
{
$this->____om = $____om;
$this->____scope = $____scope;
parent::__construct($jsonHelper, $errorAggregator, $resourceHelper, $dataHelper, $importData, $validator, $commands);
}
/**
* {@inheritdoc}
*/
public function addRowError($errorCode, $errorRowNum, $colName = null, $errorMessage = null, $errorLevel = 'critical', $errorDescription = null)
{
$arguments = func_get_args();
$beforeResult = $this->____plugin_vaimo_dynamic_import_enable_logging()->beforeAddRowError($this, ...array_values($arguments));
if ($beforeResult !== null) $arguments = (array)$beforeResult;
return parent::addRowError(...array_values($arguments));
}
/**
* {@inheritdoc}
*/
public function isNeedToLogInHistory()
{
$arguments = func_get_args();
$result = parent::isNeedToLogInHistory(...array_values($arguments));
return (($tmp = $this->____plugin_vaimo_dynamic_import_enable_logging()->afterIsNeedToLogInHistory($this, $result, ...array_values($arguments))) !== null) ? $tmp : $result;
}
/**
* plugin "vaimo_dynamic_import_enable_logging"
* @return \Vaimo\DynamicScheduledImport\Plugin\Model\Import\Entity
*/
private function ____plugin_vaimo_dynamic_import_enable_logging()
{
if ($this->____plugin_vaimo_dynamic_import_enable_logging === null) {
$this->____plugin_vaimo_dynamic_import_enable_logging = $this->____om->get(\Vaimo\DynamicScheduledImport\Plugin\Model\Import\Entity::class);
}
return $this->____plugin_vaimo_dynamic_import_enable_logging;
}
}
Plugin class generated by clean M2
<?php
namespace Vaimo\InventoryImport\Model\Import\Source\Satellite;
/**
* Interceptor class for @see \Vaimo\InventoryImport\Model\Import\Source\Satellite
*/
class Interceptor extends \Vaimo\InventoryImport\Model\Import\Source\Satellite implements \Magento\Framework\Interception\InterceptorInterface
{
use \Magento\Framework\Interception\Interceptor;
public function __construct(\Magento\InventoryImportExport\Model\Import\Serializer\Json $jsonHelper, \Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface $errorAggregator, \Magento\ImportExport\Model\ResourceModel\Helper $resourceHelper, \Magento\ImportExport\Helper\Data $dataHelper, \Magento\ImportExport\Model\ResourceModel\Import\Data $importData, \Magento\InventoryImportExport\Model\Import\Validator\ValidatorInterface $validator, array $commands = [])
{
$this->___init();
parent::__construct($jsonHelper, $errorAggregator, $resourceHelper, $dataHelper, $importData, $validator, $commands);
}
/**
* {@inheritdoc}
*/
public function addRowError($errorCode, $errorRowNum, $colName = null, $errorMessage = null, $errorLevel = 'critical', $errorDescription = null)
{
$pluginInfo = $this->pluginList->getNext($this->subjectType, 'addRowError');
return $pluginInfo ? $this->___callPlugins('addRowError', func_get_args(), $pluginInfo) : parent::addRowError($errorCode, $errorRowNum, $colName, $errorMessage, $errorLevel, $errorDescription);
}
/**
* {@inheritdoc}
*/
public function isNeedToLogInHistory()
{
$pluginInfo = $this->pluginList->getNext($this->subjectType, 'isNeedToLogInHistory');
return $pluginInfo ? $this->___callPlugins('isNeedToLogInHistory', func_get_args(), $pluginInfo) : parent::isNeedToLogInHistory(...func_get_args());
}
}