Skip to content

Conversation

@gelanivishal
Copy link
Contributor

@gelanivishal gelanivishal commented Jun 13, 2018

Original Pull Request

#15177

Proxy for class with variadic argument method

Pull request fixes proxy class generation, so class with method with variadic arguments can be also used in proxy class.

Description

In Magento/Framework/ObjectManager/Code/Generator/Proxy (lines 158, 159) I added check if parameter is variadic and if so its name is suffixed by '...', so then in method _getMethodBody() proxy method body is generated properly.

Fixed Issues (if relevant)

Extends fixes from 27065cf

Manual testing scenarios

  1. Create class, containing method, which as last argument takes variadic argument.
  2. Declare in di.xml proxy for this class.
  3. Let Magento generate it.
  4. Test if using class is possible and there are no errors. Moreover check manually content of generated proxy in /generated/code.

I was trying to extend unit test for modified class (Magento\Framework\ObjectManager\Test\Unit\Code\Generator), so it also would generate and test class with method with variadic argument, but with no success. Test broke on (mocked) method - generateResultFileName(). Actually, I only tried to edit fixtures files for test, not test itself:

Magento\Framework\ObjectManager\Code\Generator\Sample, adding method like:

/**
     * @param int $int
     * @param Sample[] ...$samples
     * @return int
     */
    public function takeVariadicObjectArguments(int $int, Sample ...$samples) : int
    {
        return (int) count($samples);
    }

And Magento\Framework\ObjectManager\Code\Generator\Sample_Proxy.txt, adding generated method like this:

/**
     * {@inheritdoc}
     */
    public function takeVariadicObjectArguments(int $int, \Magento\Framework\ObjectManager\Code\Generator\Sample ... $samples) : int
    {
        return $this->_getSubject()->takeVariadicObjectArguments($int, ... $samples);
    }

Maybe someone else would be able to extend unit test for this class. If we don't change anything in case of unit tests, test is green.

@magento-engcom-team
Copy link
Contributor

Hi @vgelani. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on Pull Request changes
  • @magento-engcom-team give me new test instance - deploy NEW test instance based on Pull Request changes
  • @magento-engcom-team give me {$VERSION} instance - deploy Vanilla Magento instance for Issue or Pull Request

For more details, please, review the Magento Contributor Assistant documentation

@magento-engcom-team
Copy link
Contributor

Hi @vgelani. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.2.6 release.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants