Fix __call called before setting/getting the property directly (#642)

Author: oprypkhantc

src/Utils/PropertyAccessor.php

@@ -22,19 +22,14 @@ class PropertyAccessor
      */
     public static function findGetter(string $class, string $propertyName): string|null
     {
-        $name = ucfirst($propertyName);
-
         foreach (['get', 'is'] as $prefix) {
-            $methodName = $prefix . $name;
+            $methodName = self::propertyToMethodName($prefix, $propertyName);
+
             if (self::isPublicMethod($class, $methodName)) {
                 return $methodName;
             }
         }
 
-        if (method_exists($class, '__call')) {
-            return 'get' . $name;
-        }
-
         return null;
     }
 
@@ -43,10 +38,9 @@ public static function findGetter(string $class, string $propertyName): string|n
      */
     public static function findSetter(string $class, string $propertyName): string|null
     {
-        $name = ucfirst($propertyName);
+        $methodName = self::propertyToMethodName('set', $propertyName);
 
-        $methodName = 'set' . $name;
-        if (self::isPublicMethod($class, $methodName) || method_exists($class, '__call')) {
+        if (self::isPublicMethod($class, $methodName)) {
             return $methodName;
         }
 
@@ -66,6 +60,12 @@ public static function getValue(object $object, string $propertyName, mixed ...$
             return $object->$propertyName;
         }
 
+        if (method_exists($class, '__call')) {
+            $method = self::propertyToMethodName('get', $propertyName);
+
+            return $object->$method(...$args);
+        }
+
         throw AccessPropertyException::createForUnreadableProperty($class, $propertyName);
     }
 
@@ -84,6 +84,13 @@ public static function setValue(object $instance, string $propertyName, mixed $v
             return;
         }
 
+        if (method_exists($class, '__call')) {
+            $method = self::propertyToMethodName('set', $propertyName);
+
+            $instance->$method($value);
+            return;
+        }
+
         throw AccessPropertyException::createForUnwritableProperty($class, $propertyName);
     }
 
@@ -117,4 +124,9 @@ private static function isValidGetter(string $class, string $methodName): bool
 
         return true;
     }
+
+    private static function propertyToMethodName(string $prefix, string $propertyName): string
+    {
+        return $prefix . ucfirst($propertyName);
+    }
 }

tests/Fixtures/Types/GetterSetterType.php

@@ -0,0 +1,46 @@
+<?php
+
+namespace TheCodingMachine\GraphQLite\Fixtures\Types;
+
+use TheCodingMachine\GraphQLite\Annotations\Field;
+
+class GetterSetterType
+{
+    public function __construct(
+        #[Field]
+        public string $one = '',
+        #[Field]
+        public string $two = '',
+        #[Field]
+        public bool $three = false,
+        #[Field]
+        public string $four = '',
+    )
+    {
+    }
+
+    public function getTwo(string $arg = ''): string
+    {
+        return $arg;
+    }
+
+    public function setTwo(string $value): void
+    {
+        $this->two = $value . ' set';
+    }
+
+    public function isThree(string $arg = ''): bool
+    {
+        return $arg === 'foo';
+    }
+
+    private function getFour(string $arg = ''): string
+    {
+        throw new \RuntimeException('Should not be called');
+    }
+
+    private function setFour(string $value, string $arg): void
+    {
+        throw new \RuntimeException('Should not be called');
+    }
+}

tests/Fixtures/Types/MagicGetterSetterType.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace TheCodingMachine\GraphQLite\Fixtures\Types;
+
+class MagicGetterSetterType extends GetterSetterType
+{
+    private string $magic;
+
+    public function __get(string $name)
+    {
+        return $this->magic;
+    }
+
+    public function __call(string $name, array $arguments)
+    {
+        $this->magic = 'magic';
+
+        return 'magic';
+    }
+}

tests/Utils/PropertyAccessorTest.php

@@ -0,0 +1,95 @@
+<?php
+
+namespace TheCodingMachine\GraphQLite\Utils;
+
+use Exception;
+use PHPUnit\Framework\TestCase;
+use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Contact;
+use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Post;
+use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Preferences;
+use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Product;
+use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\TrickyProduct;
+use TheCodingMachine\GraphQLite\Fixtures\Types\GetterSetterType;
+use TheCodingMachine\GraphQLite\Fixtures\Types\MagicGetterSetterType;
+
+class PropertyAccessorTest extends TestCase
+{
+    /**
+     * @dataProvider findGetterProvider
+     */
+    public function testFindGetter(mixed $expected, string $class, string $propertyName): void
+    {
+        self::assertSame($expected, PropertyAccessor::findGetter($class, $propertyName));
+    }
+
+    public static function findGetterProvider(): iterable
+    {
+        yield 'regular property' => [null, MagicGetterSetterType::class, 'one'];
+        yield 'getter' => ['getTwo', MagicGetterSetterType::class, 'two'];
+        yield 'isser' => ['isThree', MagicGetterSetterType::class, 'three'];
+        yield 'private getter' => [null, MagicGetterSetterType::class, 'four'];
+        yield 'undefined property' => [null, MagicGetterSetterType::class, 'twenty'];
+    }
+
+    /**
+     * @dataProvider findSetterProvider
+     */
+    public function testFindSetter(mixed $expected, string $class, string $propertyName): void
+    {
+        self::assertSame($expected, PropertyAccessor::findSetter($class, $propertyName));
+    }
+
+    public static function findSetterProvider(): iterable
+    {
+        yield 'regular property' => [null, MagicGetterSetterType::class, 'one'];
+        yield 'setter' => ['setTwo', MagicGetterSetterType::class, 'two'];
+        yield 'private setter' => [null, MagicGetterSetterType::class, 'four'];
+        yield 'undefined property' => [null, MagicGetterSetterType::class, 'twenty'];
+    }
+
+    /**
+     * @dataProvider getValueProvider
+     */
+    public function testGetValue(mixed $expected, object $object, string $propertyName, array $args = []): void
+    {
+        if ($expected instanceof Exception) {
+            $this->expectExceptionObject($expected);
+        }
+
+        self::assertSame($expected, PropertyAccessor::getValue($object, $propertyName, ...$args));
+    }
+
+    public static function getValueProvider(): iterable
+    {
+        yield 'regular property' => ['result', new MagicGetterSetterType(one: 'result'), 'one'];
+        yield 'getter' => ['result', new MagicGetterSetterType(), 'two', ['result']];
+        yield 'isser #1' => [true, new MagicGetterSetterType(), 'three', ['foo']];
+        yield 'isser #2' => [false, new MagicGetterSetterType(), 'three', ['bar']];
+        yield 'private getter' => ['result', new MagicGetterSetterType(four: 'result'), 'four'];
+        yield 'magic getter' => ['magic', new MagicGetterSetterType(), 'twenty'];
+        yield 'undefined property' => [AccessPropertyException::createForUnreadableProperty(GetterSetterType::class, 'twenty'), new GetterSetterType(), 'twenty'];
+    }
+
+    /**
+     * @dataProvider setValueProvider
+     */
+    public function testSetValue(mixed $expected, object $object, string $propertyName, mixed $value): void
+    {
+        if ($expected instanceof Exception) {
+            $this->expectExceptionObject($expected);
+        }
+
+        PropertyAccessor::setValue($object, $propertyName, $value);
+
+        self::assertSame($expected, $object->{$propertyName});
+    }
+
+    public static function setValueProvider(): iterable
+    {
+        yield 'regular property' => ['result', new MagicGetterSetterType(one: 'result'), 'one', 'result'];
+        yield 'setter' => ['result set', new MagicGetterSetterType(), 'two', 'result'];
+        yield 'private setter' => ['result', new MagicGetterSetterType(four: 'result'), 'four', 'result'];
+        yield 'magic setter' => ['magic', new MagicGetterSetterType(), 'twenty', 'result'];
+        yield 'undefined property' => [AccessPropertyException::createForUnwritableProperty(GetterSetterType::class, 'twenty'), new GetterSetterType(), 'twenty', 'result'];
+    }
+}