[BUGFIX] Use reflection information in getGettablePropertyNames 61/51361/3
authorNicole Cordes <typo3@cordes.co>
Wed, 12 Oct 2016 15:55:27 +0000 (17:55 +0200)
committerPhilipp Gampe <philipp.gampe@typo3.org>
Thu, 19 Jan 2017 13:58:25 +0000 (14:58 +0100)
In \TYPO3\CMS\Extbase\Reflection\ObjectAccess there is a method to get
all available property names of an object. Currently all get/is/has
methods are joined as they can be fetch from Extbase as well. But for
those methods it is necessary to respect their arguments as Extbase
calls those functions without any argument.
This can trigger PHP warnings.

The patch uses a class reflection to get public properties and inspect
the method arguments. Only those functions without arguments or only
optional arguments are considered as valid property name.

Resolves: #78270
Releases: master, 7.6
Change-Id: Ie286dca2a249b73d3dc58f7388dda593a678db3d
Reviewed-on: https://review.typo3.org/51361
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Philipp Gampe <philipp.gampe@typo3.org>
Tested-by: Philipp Gampe <philipp.gampe@typo3.org>
typo3/sysext/extbase/Classes/Reflection/ObjectAccess.php
typo3/sysext/extbase/Tests/Unit/Reflection/Fixture/DummyClassWithGettersAndSetters.php
typo3/sysext/extbase/Tests/Unit/Reflection/ObjectAccessTest.php

index 10fda6d..9f2daf2 100644 (file)
@@ -235,25 +235,41 @@ class ObjectAccess
             throw new \InvalidArgumentException('$object must be an object, ' . gettype($object) . ' given.', 1237301369);
         }
         if ($object instanceof \stdClass) {
-            $declaredPropertyNames = array_keys(get_object_vars($object));
-        } else {
-            $declaredPropertyNames = array_keys(get_class_vars(get_class($object)));
+            $properties = array_keys((array)$object);
+            sort($properties);
+            return $properties;
         }
-        foreach (get_class_methods($object) as $methodName) {
-            if (is_callable([$object, $methodName])) {
-                if (substr($methodName, 0, 2) === 'is') {
-                    $declaredPropertyNames[] = lcfirst(substr($methodName, 2));
-                }
-                if (substr($methodName, 0, 3) === 'get') {
-                    $declaredPropertyNames[] = lcfirst(substr($methodName, 3));
-                }
-                if (substr($methodName, 0, 3) === 'has') {
-                    $declaredPropertyNames[] = lcfirst(substr($methodName, 3));
+
+        $reflection = new \ReflectionClass($object);
+        $declaredPropertyNames = array_map(
+            function (\ReflectionProperty $property) {
+                return $property->getName();
+            },
+            $reflection->getProperties(\ReflectionProperty::IS_PUBLIC)
+        );
+        foreach ($reflection->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) {
+            $methodParameters = $method->getParameters();
+            if (!empty($methodParameters)) {
+                foreach ($methodParameters as $parameter) {
+                    if (!$parameter->isOptional()) {
+                        continue 2;
+                    }
                 }
             }
+            $methodName = $method->getName();
+            if (substr($methodName, 0, 2) === 'is') {
+                $declaredPropertyNames[] = lcfirst(substr($methodName, 2));
+            }
+            if (substr($methodName, 0, 3) === 'get') {
+                $declaredPropertyNames[] = lcfirst(substr($methodName, 3));
+            }
+            if (substr($methodName, 0, 3) === 'has') {
+                $declaredPropertyNames[] = lcfirst(substr($methodName, 3));
+            }
         }
         $propertyNames = array_unique($declaredPropertyNames);
         sort($propertyNames);
+
         return $propertyNames;
     }
 
index 5fbda16..1bc63fa 100644 (file)
@@ -145,4 +145,14 @@ class DummyClassWithGettersAndSetters
     {
         return $this->anotherBooleanProperty;
     }
+
+    /**
+     * @param int $value
+     *
+     * @return bool
+     */
+    public function hasSomeValue($value = 42)
+    {
+        return true;
+    }
 }
index cc2e952..8ec8f25 100644 (file)
@@ -282,13 +282,24 @@ class ObjectAccessTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
     public function getGettablePropertyNamesReturnsAllPropertiesWhichAreAvailable()
     {
         $gettablePropertyNames = \TYPO3\CMS\Extbase\Reflection\ObjectAccess::getGettablePropertyNames($this->dummyObject);
-        $expectedPropertyNames = ['anotherBooleanProperty', 'anotherProperty', 'booleanProperty', 'property', 'property2', 'publicProperty', 'publicProperty2'];
+        $expectedPropertyNames = ['anotherBooleanProperty', 'anotherProperty', 'booleanProperty', 'property', 'property2', 'publicProperty', 'publicProperty2', 'someValue'];
         $this->assertEquals($gettablePropertyNames, $expectedPropertyNames, 'getGettablePropertyNames returns not all gettable properties.');
     }
 
     /**
      * @test
      */
+    public function getGettablePropertyNamesRespectsMethodArguments()
+    {
+        $dateTimeZone = new \DateTimeZone('+2');
+        $gettablePropertyNames = \TYPO3\CMS\Extbase\Reflection\ObjectAccess::getGettablePropertyNames($dateTimeZone);
+        $expectedPropertyNames = ['location', 'name'];
+        $this->assertEquals($gettablePropertyNames, $expectedPropertyNames, 'getGettablePropertyNames does not respect method arguments.');
+    }
+
+    /**
+     * @test
+     */
     public function getSettablePropertyNamesReturnsAllPropertiesWhichAreAvailable()
     {
         $settablePropertyNames = \TYPO3\CMS\Extbase\Reflection\ObjectAccess::getSettablePropertyNames($this->dummyObject);
@@ -322,7 +333,8 @@ class ObjectAccessTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
             'property' => 'string1',
             'property2' => null,
             'publicProperty' => null,
-            'publicProperty2' => 42
+            'publicProperty2' => 42,
+            'someValue' => true,
         ];
         $this->assertEquals($allProperties, $expectedProperties, 'expectedProperties did not return the right values for the properties.');
     }