creatuity/magento2-interceptors

Productrepository interceptor bug

Closed this issue · 3 comments

Quazz commented

Installed this on Magento 2.4.0

Encountered what seems to be a bug in the productRepository interceptor.

/**
     * {@inheritdoc}
     */
    public function save(\Magento\Catalog\Api\Data\ProductInterface $product, $saveOptions = false)
    {
        switch ($this->____scope->getCurrentScope()) {
        	case 'global':
        	case 'frontend':
        	case 'adminhtml':
        	case 'graphql':
        	case 'crontab':
        	case 'primary':
        		$beforeResult = $this->____plugin_configurableProductSaveOptions()->beforeSave($this, $product, $saveOptions);
        		if ($beforeResult !== null) list($product, $saveOptions) = (array)$beforeResult;
        		
        		$result = parent::save($product, $saveOptions);
        		
        		return (($tmp = $this->____plugin_configurableProductSaveOptions()->afterSave($this, $result, $product, $saveOptions)) !== null) ? $tmp : $result;
        	default:
        		$beforeResult = $this->____plugin_product_authorization()->beforeSave($this, $product, $saveOptions);
        		if ($beforeResult !== null) list($product, $saveOptions) = (array)$beforeResult;
        		
        		$beforeResult = $this->____plugin_configurableProductSaveOptions()->beforeSave($this, $product, $saveOptions);
        		if ($beforeResult !== null) list($product, $saveOptions) = (array)$beforeResult;
        		
        		$result = parent::save($product, $saveOptions);
        		
        		return (($tmp = $this->____plugin_configurableProductSaveOptions()->afterSave($this, $result, $product, $saveOptions)) !== null) ? $tmp : $result;
        }
    }

When using productrepository to save a product, it will throw an undefined offset notice. That's because the configurableProductSaveOptions plugin only returns 1 item in its array, while the interceptor expects 2 at lines such as

if ($beforeResult !== null) list($product, $saveOptions) = (array)$beforeResult;

Or is this technically a bug in the magento 2 code itself? (plugin should return the same amount of parameters it gets fed)?

Quazz commented

Changing $result[] = $product; to $result = [ $product, $saveOptions ]; in the configurableproduct productrepository plugin fixes the issue. Therefore I will actually state this to be a magento core bug instead.

fsw commented

Hi @Quazz,

Thanks for working this out. Indeed this seems like a bug in a plugin that should return array of all arguments.

However this module tries to be backward compatible so if default interceptor generator (without module installed) handles such a case maybe we also should. Does this work fine without module installed?

This might be an interesting case when you add a parameter to a method in a class and it is still working with its plugins from other modules.

If you will report this as a core bug, please link it here as I am interested in the outcome.

Quazz commented

Yes, saving using productrepository works fine without this module.

That said, I'm not sure $saveOptions works or not in the original implementation as it seems like the parameter is actually completely unused in the Model file, but it is of course required by its Interface.

It is however, correctly implemented in a Magento 2 core plugin: https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Catalog/Plugin/ProductAuthorization.php#L44

So I think it may be some backwards compatible stuff that's left over from older versions.

That said, issue posted: magento/magento2#31870