Merge pull request #540 from pedro-stanaka/hotfix/default-params-remo…

…te-object-method RemoteObjectMethod generation: support for methods with default parameters
Ocramius · Jan 7, 2020 · fea82fb · fea82fb
2 parents b7bac96 + fba20dd
commit fea82fb
Show file tree

Hide file tree

Showing 9 changed files with 332 additions and 31 deletions.
diff --git a/composer.json b/composer.json
@@ -55,7 +55,8 @@
         "psr-4": {
             "ProxyManagerBench\\": "tests/ProxyManagerBench",
             "ProxyManagerTest\\": "tests/ProxyManagerTest",
-            "ProxyManagerTestAsset\\": "tests/ProxyManagerTestAsset"
+            "ProxyManagerTestAsset\\": "tests/ProxyManagerTestAsset",
+            "Zend\\Server\\": "tests/Stubbed/Zend/Server"
         }
     },
     "extra": {

diff --git a/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/RemoteObjectMethod.php b/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/RemoteObjectMethod.php
@@ -9,29 +9,75 @@
 use ReflectionClass;
 use Zend\Code\Generator\PropertyGenerator;
 use Zend\Code\Reflection\MethodReflection;
+use function count;
+use function strtr;
 use function var_export;
 
 /**
  * Method decorator for remote objects
  */
 class RemoteObjectMethod extends MethodGenerator
 {
+    private const TEMPLATE
+        = <<<'PHP'
+$defaultValues = #DEFAULT_VALUES#;
+$declaredParameterCount = #PARAMETER_COUNT#;
+
+$args = \func_get_args() + $defaultValues;
+
+#PROXIED_RETURN#
+PHP;
+
     /** @return self|static */
     public static function generateMethod(
         MethodReflection $originalMethod,
         PropertyGenerator $adapterProperty,
         ReflectionClass $originalClass
     ) : self {
         /** @var self $method */
-        $method = static::fromReflectionWithoutBodyAndDocBlock($originalMethod);
+        $method        = static::fromReflectionWithoutBodyAndDocBlock($originalMethod);
+        $proxiedReturn = '$return = $this->' . $adapterProperty->getName()
+            . '->call(' . var_export($originalClass->getName(), true)
+            . ', ' . var_export($originalMethod->getName(), true) . ', $args);' . "\n\n"
+            . ProxiedMethodReturnExpression::generate('$return', $originalMethod);
+
+        $defaultValues          = self::getDefaultValuesForMethod($originalMethod);
+        $declaredParameterCount = count($originalMethod->getParameters());
 
         $method->setBody(
-            '$return = $this->' . $adapterProperty->getName()
-            . '->call(' . var_export($originalClass->getName(), true)
-            . ', ' . var_export($originalMethod->getName(), true) . ', \func_get_args());' . "\n\n"
-            . ProxiedMethodReturnExpression::generate('$return', $originalMethod)
+            strtr(
+                self::TEMPLATE,
+                [
+                    '#PROXIED_RETURN#' => $proxiedReturn,
+                    '#DEFAULT_VALUES#' => var_export($defaultValues, true),
+                    '#PARAMETER_COUNT#' => var_export($declaredParameterCount, true),
+                ]
+            )
         );
 
         return $method;
     }
+
+    /**
+     * @return array
+     */
+    private static function getDefaultValuesForMethod(MethodReflection $originalMethod) : array
+    {
+        $defaultValues = [];
+        foreach ($originalMethod->getParameters() as $parameter) {
+            if ($parameter->isOptional() && $parameter->isDefaultValueAvailable()) {
+                /** @psalm-var int|float|bool|array|string|null */
+                $defaultValues[] = $parameter->getDefaultValue();
+                continue;
+            }
+
+            if ($parameter->isVariadic()) {
+                continue;
+            }
+
+            $defaultValues[] = null;
+        }
+
+        return $defaultValues;
+    }
 }
diff --git a/tests/ProxyManagerTest/Functional/RemoteObjectFunctionalTest.php b/tests/ProxyManagerTest/Functional/RemoteObjectFunctionalTest.php
@@ -15,6 +15,8 @@
 use ProxyManagerTestAsset\RemoteProxy\BazServiceInterface;
 use ProxyManagerTestAsset\RemoteProxy\Foo;
 use ProxyManagerTestAsset\RemoteProxy\FooServiceInterface;
+use ProxyManagerTestAsset\RemoteProxy\RemoteServiceWithDefaultsAndVariadicArguments;
+use ProxyManagerTestAsset\RemoteProxy\RemoteServiceWithDefaultsInterface;
 use ProxyManagerTestAsset\RemoteProxy\VariadicArgumentsServiceInterface;
 use ProxyManagerTestAsset\VoidCounter;
 use ReflectionClass;
@@ -34,15 +36,15 @@ final class RemoteObjectFunctionalTest extends TestCase
 {
     /**
      * @param mixed   $expectedValue
-     * @param mixed[] $params
+     * @param mixed[] $parametersExpectedByClient
      */
-    protected function getXmlRpcAdapter($expectedValue, string $method, array $params) : XmlRpcAdapter
+    protected function getXmlRpcAdapter($expectedValue, string $method, array $parametersExpectedByClient) : XmlRpcAdapter
     {
-        $client = $this->getMockBuilder(Client::class)->setMethods(['call'])->getMock();
+        $client = $this->getMockBuilder(Client::class)->getMock();
 
         $client
             ->method('call')
-            ->with(self::stringEndsWith($method), $params)
+            ->with(self::stringEndsWith($method), $parametersExpectedByClient)
             ->willReturn($expectedValue);
 
         return new XmlRpcAdapter(
@@ -57,7 +59,7 @@ protected function getXmlRpcAdapter($expectedValue, string $method, array $param
      */
     protected function getJsonRpcAdapter($expectedValue, string $method, array $params) : JsonRpcAdapter
     {
-        $client = $this->getMockBuilder(Client::class)->setMethods(['call'])->getMock();
+        $client = $this->getMockBuilder(Client::class)->getMock();
 
         $client
             ->method('call')
@@ -72,44 +74,56 @@ protected function getJsonRpcAdapter($expectedValue, string $method, array $para
 
     /**
      * @param string|object $instanceOrClassName
-     * @param mixed[]       $params
+     * @param array|mixed[] $passedParams
+     * @param mixed[]       $callParametersExpectedByAdapter
      * @param mixed         $expectedValue
      *
      * @dataProvider getProxyMethods
      *
      * @psalm-template OriginalClass of object
      * @psalm-param class-string<OriginalClass>|OriginalClass $instanceOrClassName
      */
-    public function testXmlRpcMethodCalls($instanceOrClassName, string $method, array $params, $expectedValue) : void
-    {
-        $proxy = (new RemoteObjectFactory($this->getXmlRpcAdapter($expectedValue, $method, $params)))
+    public function testXmlRpcMethodCalls(
+        $instanceOrClassName,
+        string $method,
+        array $passedParams,
+        array $callParametersExpectedByAdapter,
+        $expectedValue
+    ) : void {
+        $proxy = (new RemoteObjectFactory($this->getXmlRpcAdapter($expectedValue, $method, $callParametersExpectedByAdapter)))
             ->createProxy($instanceOrClassName);
 
         $callback = [$proxy, $method];
 
         self::assertIsCallable($callback);
-        self::assertSame($expectedValue, $callback(...$params));
+        self::assertSame($expectedValue, $callback(...$passedParams));
     }
 
     /**
      * @param string|object $instanceOrClassName
-     * @param mixed[]       $params
+     * @param array|mixed[] $passedParams
+     * @param mixed[]       $parametersForProxy
      * @param mixed         $expectedValue
      *
      * @dataProvider getProxyMethods
      *
      * @psalm-template OriginalClass of object
      * @psalm-param class-string<OriginalClass>|OriginalClass $instanceOrClassName
      */
-    public function testJsonRpcMethodCalls($instanceOrClassName, string $method, array $params, $expectedValue) : void
-    {
-        $proxy = (new RemoteObjectFactory($this->getJsonRpcAdapter($expectedValue, $method, $params)))
+    public function testJsonRpcMethodCalls(
+        $instanceOrClassName,
+        string $method,
+        array $passedParams,
+        array $parametersForProxy,
+        $expectedValue
+    ) : void {
+        $proxy = (new RemoteObjectFactory($this->getJsonRpcAdapter($expectedValue, $method, $parametersForProxy)))
             ->createProxy($instanceOrClassName);
 
         $callback = [$proxy, $method];
 
         self::assertIsCallable($callback);
-        self::assertSame($expectedValue, $callback(...$params));
+        self::assertSame($expectedValue, $callback(...$passedParams));
     }
 
     /**
@@ -143,38 +157,122 @@ public function getProxyMethods() : array
                 FooServiceInterface::class,
                 'foo',
                 [],
+                [],
                 'bar remote',
             ],
             [
                 Foo::class,
                 'foo',
                 [],
+                [],
                 'bar remote',
             ],
             [
                 new Foo(),
                 'foo',
                 [],
+                [],
                 'bar remote',
             ],
             [
                 BazServiceInterface::class,
                 'baz',
                 ['baz'],
+                ['baz'],
                 'baz remote',
             ],
             [
                 new ClassWithSelfHint(),
                 'selfHintMethod',
                 [$selfHintParam],
+                [$selfHintParam],
                 $selfHintParam,
             ],
             [
                 VariadicArgumentsServiceInterface::class,
                 'method',
                 ['aaa', 1, 2, 3, 4, 5],
+                ['aaa', 1, 2, 3, 4, 5],
                 true,
             ],
+            [
+                RemoteServiceWithDefaultsInterface::class,
+                'optionalNonNullable',
+                ['aaa'],
+                ['aaa', 'Optional parameter to be kept during calls'],
+                200,
+            ],
+            [
+                RemoteServiceWithDefaultsInterface::class,
+                'optionalNullable',
+                ['aaa'],
+                ['aaa', null],
+                200,
+            ],
+            'when passing only the required parameters' => [
+                RemoteServiceWithDefaultsInterface::class,
+                'manyRequiredWithManyOptional',
+                ['aaa', 100],
+                [
+                    'aaa',
+                    100,
+                    'Optional parameter to be kept during calls',
+                    100,
+                    'Yet another optional parameter to be kept during calls',
+                ],
+                200,
+            ],
+            'when passing required params and one optional params' => [
+                RemoteServiceWithDefaultsInterface::class,
+                'manyRequiredWithManyOptional',
+                ['aaa', 100, 'passed'],
+                [
+                    'aaa',
+                    100,
+                    'passed',
+                    100,
+                    'Yet another optional parameter to be kept during calls',
+                ],
+                200,
+            ],
+            'when passing required params and some optional params' => [
+                RemoteServiceWithDefaultsInterface::class,
+                'manyRequiredWithManyOptional',
+                ['aaa', 100, 'passed', 90],
+                [
+                    'aaa',
+                    100,
+                    'passed',
+                    90,
+                    'Yet another optional parameter to be kept during calls',
+                ],
+                200,
+            ],
+            'when passing only required for method with optional and variadic params' => [
+                RemoteServiceWithDefaultsAndVariadicArguments::class,
+                'optionalWithVariadic',
+                ['aaa'],
+                [
+                    'aaa',
+                    'Optional param to be kept on proxy call',
+                ],
+                200,
+            ],
+            'when passing required, optional and variadic params' => [
+                RemoteServiceWithDefaultsAndVariadicArguments::class,
+                'optionalWithVariadic',
+                ['aaa', 'Optional param to be kept on proxy call', 10, 20, 30, 50, 90],
+                [
+                    'aaa',
+                    'Optional param to be kept on proxy call',
+                    10,
+                    20,
+                    30,
+                    50,
+                    90,
+                ],
+                200,
+            ],
         ];
     }
 

diff --git a/...s/ProxyManagerTest/ProxyGenerator/RemoteObject/MethodGenerator/RemoteObjectMethodTest.php b/...s/ProxyManagerTest/ProxyGenerator/RemoteObject/MethodGenerator/RemoteObjectMethodTest.php
@@ -4,7 +4,6 @@
 
 namespace ProxyManagerTest\ProxyGenerator\RemoteObject\MethodGenerator;
 
-use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use ProxyManager\ProxyGenerator\RemoteObject\MethodGenerator\RemoteObjectMethod;
 use ProxyManagerTestAsset\BaseClass;
@@ -41,9 +40,17 @@ public function testBodyStructureWithParameters() : void
         self::assertSame('publicByReferenceParameterMethod', $method->getName());
         self::assertCount(2, $method->getParameters());
         self::assertSame(
-            '$return = $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
-            . '\'publicByReferenceParameterMethod\', \func_get_args());'
-            . "\n\nreturn \$return;",
+            '$defaultValues = array (
+  0 => NULL,
+  1 => NULL,
+);
+$declaredParameterCount = 2;
+
+$args = \func_get_args() + $defaultValues;
+
+$return = $this->adapter->call(\'Zend\\\\Code\\\\Generator\\\\PropertyGenerator\', \'publicByReferenceParameterMethod\', $args);
+
+return $return;',
             $method->getBody()
         );
     }
@@ -67,9 +74,16 @@ public function testBodyStructureWithArrayParameter() : void
         self::assertSame('publicArrayHintedMethod', $method->getName());
         self::assertCount(1, $method->getParameters());
         self::assertSame(
-            '$return = $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
-            . '\'publicArrayHintedMethod\', \func_get_args());'
-            . "\n\nreturn \$return;",
+            "\$defaultValues = array (
+  0 => NULL,
+);
+\$declaredParameterCount = 1;
+
+\$args = \\func_get_args() + \$defaultValues;
+
+\$return = \$this->adapter->call('Zend\\\\Code\\\\Generator\\\\PropertyGenerator', 'publicArrayHintedMethod', \$args);
+
+return \$return;",
             $method->getBody()
         );
     }
@@ -93,9 +107,15 @@ public function testBodyStructureWithoutParameters() : void
         self::assertSame('publicMethod', $method->getName());
         self::assertCount(0, $method->getParameters());
         self::assertSame(
-            '$return = $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
-            . '\'publicMethod\', \func_get_args());'
-            . "\n\nreturn \$return;",
+            "\$defaultValues = array (
+);
+\$declaredParameterCount = 0;
+
+\$args = \\func_get_args() + \$defaultValues;
+
+\$return = \$this->adapter->call('Zend\\\\Code\\\\Generator\\\\PropertyGenerator', 'publicMethod', \$args);
+
+return \$return;",
             $method->getBody()
         );
     }