[TASK] Avoid runtime reflection calls in ObjectAccess 31/59331/23
authorAlexander Schnitzler <git@alexanderschnitzler.de>
Fri, 1 Feb 2019 09:45:19 +0000 (10:45 +0100)
committerAndreas Wolf <andreas.wolf@typo3.org>
Tue, 5 Feb 2019 17:16:34 +0000 (18:16 +0100)
ObjectAccess had two kinds of runtime reflection calls:

1) To gather data about object properties and methods
2) To make non public properties accessible

The first one is tackled by using class schema instances
and by using the property accessor of symfony/property-access.

The latter is tackled by deprecating all method arguments
that trigger reflection to make properties accessible. In
the future, gettable/settable properties need to be either
public or have a getter (get*/has*/is*) or setter.

Releases: master
Resolves: #87332
Change-Id: I6ecef81de7aa4cc1244166d683874a1a87ac6bb7
Reviewed-on: https://review.typo3.org/59331
Reviewed-by: André Schließer <andy.schliesser@gmail.com>
Tested-by: André Schließer <andy.schliesser@gmail.com>
Reviewed-by: Andreas Wolf <andreas.wolf@typo3.org>
Tested-by: Andreas Wolf <andreas.wolf@typo3.org>
15 files changed:
composer.json
composer.lock
typo3/sysext/core/Documentation/Changelog/master/Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst [new file with mode: 0644]
typo3/sysext/extbase/Classes/Property/TypeConverter/ObjectConverter.php
typo3/sysext/extbase/Classes/Reflection/ClassSchema.php
typo3/sysext/extbase/Classes/Reflection/ObjectAccess.php
typo3/sysext/extbase/Classes/Validation/Validator/GenericObjectValidator.php
typo3/sysext/extbase/Tests/Unit/Mvc/View/JsonViewTest.php
typo3/sysext/extbase/Tests/Unit/Reflection/ObjectAccessTest.php
typo3/sysext/extbase/Tests/Unit/Validation/Validator/CollectionValidatorTest.php
typo3/sysext/extbase/Tests/Unit/Validation/Validator/GenericObjectValidatorTest.php
typo3/sysext/extbase/Tests/UnitDeprecated/Reflection/ObjectAccessTest.php [new file with mode: 0644]
typo3/sysext/extbase/composer.json
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedStaticMatcher.php
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php

index 086fc39..8dd193e 100644 (file)
@@ -56,6 +56,7 @@
                "symfony/finder": "^4.1",
                "symfony/polyfill-intl-icu": "^1.6",
                "symfony/polyfill-mbstring": "^1.2",
+               "symfony/property-access": "^4.2",
                "symfony/property-info": "^4.2",
                "symfony/routing": "^4.1",
                "symfony/yaml": "^4.1",
index b3c65e4..59cbe89 100644 (file)
@@ -4,7 +4,7 @@
         "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
         "This file is @generated automatically"
     ],
-    "content-hash": "665468d5ffefa8e2e72be0a338838765",
+    "content-hash": "ae4664b4ee7bf0aa9a614d355d67db78",
     "packages": [
         {
             "name": "cogpowered/finediff",
             "time": "2018-08-06T14:22:27+00:00"
         },
         {
+            "name": "symfony/property-access",
+            "version": "v4.2.2",
+            "source": {
+                "type": "git",
+                "url": "https://github.com/symfony/property-access.git",
+                "reference": "a21d40670000f61a1a4b90a607d54696aad914cd"
+            },
+            "dist": {
+                "type": "zip",
+                "url": "https://api.github.com/repos/symfony/property-access/zipball/a21d40670000f61a1a4b90a607d54696aad914cd",
+                "reference": "a21d40670000f61a1a4b90a607d54696aad914cd",
+                "shasum": ""
+            },
+            "require": {
+                "php": "^7.1.3",
+                "symfony/inflector": "~3.4|~4.0"
+            },
+            "require-dev": {
+                "symfony/cache": "~3.4|~4.0"
+            },
+            "suggest": {
+                "psr/cache-implementation": "To cache access methods."
+            },
+            "type": "library",
+            "extra": {
+                "branch-alias": {
+                    "dev-master": "4.2-dev"
+                }
+            },
+            "autoload": {
+                "psr-4": {
+                    "Symfony\\Component\\PropertyAccess\\": ""
+                },
+                "exclude-from-classmap": [
+                    "/Tests/"
+                ]
+            },
+            "notification-url": "https://packagist.org/downloads/",
+            "license": [
+                "MIT"
+            ],
+            "authors": [
+                {
+                    "name": "Fabien Potencier",
+                    "email": "fabien@symfony.com"
+                },
+                {
+                    "name": "Symfony Community",
+                    "homepage": "https://symfony.com/contributors"
+                }
+            ],
+            "description": "Symfony PropertyAccess Component",
+            "homepage": "https://symfony.com",
+            "keywords": [
+                "access",
+                "array",
+                "extraction",
+                "index",
+                "injection",
+                "object",
+                "property",
+                "property path",
+                "reflection"
+            ],
+            "time": "2019-01-05T16:37:49+00:00"
+        },
+        {
             "name": "symfony/property-info",
             "version": "v4.2.2",
             "source": {
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst
new file mode 100644 (file)
index 0000000..9b29faa
--- /dev/null
@@ -0,0 +1,40 @@
+.. include:: ../../Includes.txt
+
+====================================================================
+Deprecation: #87332 - Avoid runtime reflection calls in ObjectAccess
+====================================================================
+
+See :issue:`87332`
+
+Description
+===========
+
+1) Class :php:`\TYPO3\CMS\Extbase\Reflection\ObjectAccess` uses reflection to make non public properties gettable and settable. This behaviour is triggered by setting the argument :php:`$forceDirectAccess` of methods :php:`getProperty`, :php:`getPropertyInternal` or :php:`setProperty` to :php:`true`. Triggering this behaviour is deprecated and will be removed in TYPO3 11.0.
+
+2) Method :php:`\TYPO3\CMS\Extbase\Reflection\ObjectAccess::buildSetterMethodName` has been deprecated and will be removed in TYPO3 11.0.
+
+
+Impact
+======
+
+1) Accessing non public properties via the mentioned methods will no longer work in TYPO3 11.0.
+
+2) Calling :php:`\TYPO3\CMS\Extbase\Reflection\ObjectAccess::buildSetterMethodName` will no longer work in TYPO3 11.0.
+
+
+Affected Installations
+======================
+
+1) All installations that use the mentioned methods with argument :php:`$forceDirectAccess` set to :php:`true`.
+
+2) All installations that call :php:`\TYPO3\CMS\Extbase\Reflection\ObjectAccess::buildSetterMethodName`.
+
+
+Migration
+=========
+
+1) Make sure the affected property is accessible by either making it public or providing getters/hassers/issers or setters (:php:`getProperty()`, :php:`hasProperty()`, :php:`isProperty()`, :php:`setProperty()`).
+
+2) Build setter names manually: :php:`$setterMethodName = 'set' . ucfirst($propertyName);`
+
+.. index:: PHP-API, FullyScanned, ext:extbase
index 8f01d76..0d6e9d7 100644 (file)
@@ -16,7 +16,6 @@ namespace TYPO3\CMS\Extbase\Property\TypeConverter;
 
 use TYPO3\CMS\Extbase\Reflection\ClassSchema\Exception\NoSuchMethodException;
 use TYPO3\CMS\Extbase\Reflection\ClassSchema\Exception\NoSuchMethodParameterException;
-use TYPO3\CMS\Extbase\Reflection\ClassSchema\MethodParameter;
 
 /**
  * This converter transforms arrays to simple objects (POPO) by setting properties.
@@ -122,9 +121,9 @@ class ObjectConverter extends AbstractTypeConverter
         $specificTargetType = $this->objectContainer->getImplementationClassName($targetType);
         $classSchema = $this->reflectionService->getClassSchema($specificTargetType);
 
-        if ($classSchema->hasMethod(\TYPO3\CMS\Extbase\Reflection\ObjectAccess::buildSetterMethodName($propertyName))) {
-            $methodParameters = $classSchema->getMethod(\TYPO3\CMS\Extbase\Reflection\ObjectAccess::buildSetterMethodName($propertyName))->getParameters();
-            /** @var MethodParameter $methodParameter */
+        $methodName = 'set' . ucfirst($propertyName);
+        if ($classSchema->hasMethod($methodName)) {
+            $methodParameters = $classSchema->getMethod($methodName)->getParameters() ?? [];
             $methodParameter = current($methodParameters);
             if ($methodParameter->getType() === null) {
                 throw new \TYPO3\CMS\Extbase\Property\Exception\InvalidTargetException('Setter for property "' . $propertyName . '" had no type hint or documentation in target object of type "' . $specificTargetType . '".', 1303379158);
index 66e3dd8..4aa4860 100644 (file)
@@ -560,7 +560,7 @@ class ClassSchema
     }
 
     /**
-     * @return array
+     * @return array|Method[]
      */
     public function getMethods(): array
     {
index aa15bab..dd5699d 100644 (file)
@@ -1,4 +1,6 @@
 <?php
+declare(strict_types = 1);
+
 namespace TYPO3\CMS\Extbase\Reflection;
 
 /*
@@ -14,6 +16,11 @@ namespace TYPO3\CMS\Extbase\Reflection;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Symfony\Component\PropertyAccess\PropertyAccess;
+use Symfony\Component\PropertyAccess\PropertyAccessor;
+use Symfony\Component\PropertyAccess\PropertyPath;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\StringUtility;
 use TYPO3\CMS\Extbase\Persistence\ObjectStorage;
 
 /**
@@ -27,11 +34,10 @@ use TYPO3\CMS\Extbase\Persistence\ObjectStorage;
  */
 class ObjectAccess
 {
-    const ACCESS_GET = 0;
-
-    const ACCESS_SET = 1;
-
-    const ACCESS_PUBLIC = 2;
+    /**
+     * @var PropertyAccessor
+     */
+    private static $propertyAccessor;
 
     /**
      * Get a property of a given object.
@@ -49,16 +55,18 @@ class ObjectAccess
      * @param bool $forceDirectAccess directly access property using reflection(!)
      *
      * @throws \InvalidArgumentException in case $subject was not an object or $propertyName was not a string
+     * @throws Exception\PropertyNotAccessibleException
      * @return mixed Value of the property
      */
-    public static function getProperty($subject, $propertyName, $forceDirectAccess = false)
+    public static function getProperty($subject, string $propertyName, bool $forceDirectAccess = false)
     {
         if (!is_object($subject) && !is_array($subject)) {
-            throw new \InvalidArgumentException('$subject must be an object or array, ' . gettype($subject) . ' given.', 1237301367);
-        }
-        if (!is_string($propertyName) && (!is_array($subject) && !$subject instanceof \ArrayAccess)) {
-            throw new \InvalidArgumentException('Given property name is not of type string.', 1231178303);
+            throw new \InvalidArgumentException(
+                '$subject must be an object or array, ' . gettype($subject) . ' given.',
+                1237301367
+            );
         }
+
         return self::getPropertyInternal($subject, $propertyName, $forceDirectAccess);
     }
 
@@ -77,52 +85,38 @@ class ObjectAccess
      * @return mixed Value of the property
      * @internal
      */
-    public static function getPropertyInternal($subject, $propertyName, $forceDirectAccess = false)
+    public static function getPropertyInternal($subject, string $propertyName, bool $forceDirectAccess = false)
     {
-        // type check and conversion of iterator to numerically indexed array
-        if ($subject === null || is_scalar($subject)) {
-            return null;
+        if ($forceDirectAccess === true) {
+            trigger_error('Argument $forceDirectAccess will be removed in TYPO3 11.0', E_USER_DEPRECATED);
         }
+
         if (!$forceDirectAccess && ($subject instanceof \SplObjectStorage || $subject instanceof ObjectStorage)) {
             $subject = iterator_to_array(clone $subject, false);
         }
 
-        // value get based on data type of $subject (possibly converted above)
-        if (($subject instanceof \ArrayAccess && $subject->offsetExists($propertyName)) || is_array($subject)) {
-            // isset() is safe; array_key_exists would only be needed to determine
-            // if the value is NULL - and that's already what we return as fallback.
-            if (isset($subject[$propertyName])) {
-                return $subject[$propertyName];
-            }
-        } elseif (is_object($subject)) {
-            if ($forceDirectAccess) {
-                if (property_exists($subject, $propertyName)) {
-                    $propertyReflection = new \ReflectionProperty($subject, $propertyName);
-                    if ($propertyReflection->isPublic()) {
-                        return $propertyReflection->getValue($subject);
-                    }
-                    $propertyReflection->setAccessible(true);
-                    return $propertyReflection->getValue($subject);
-                }
-                throw new Exception\PropertyNotAccessibleException('The property "' . $propertyName . '" on the subject does not exist.', 1302855001);
-            }
-            $upperCasePropertyName = ucfirst($propertyName);
-            $getterMethodName = 'get' . $upperCasePropertyName;
-            if (is_callable([$subject, $getterMethodName])) {
-                return $subject->{$getterMethodName}();
-            }
-            $getterMethodName = 'is' . $upperCasePropertyName;
-            if (is_callable([$subject, $getterMethodName])) {
-                return $subject->{$getterMethodName}();
-            }
-            $getterMethodName = 'has' . $upperCasePropertyName;
-            if (is_callable([$subject, $getterMethodName])) {
-                return $subject->{$getterMethodName}();
-            }
-            if (property_exists($subject, $propertyName)) {
-                return $subject->{$propertyName};
+        $propertyPath = new PropertyPath($propertyName);
+
+        if ($subject instanceof \ArrayAccess) {
+            $accessor = self::createAccessor();
+
+            // Check if $subject is an instance of \ArrayAccess and therefore maybe has actual accessible properties.
+            if ($accessor->isReadable($subject, $propertyPath)) {
+                return $accessor->getValue($subject, $propertyPath);
             }
-            throw new Exception\PropertyNotAccessibleException('The property "' . $propertyName . '" on the subject does not exist.', 1476109666);
+
+            // Use array style property path for instances of \ArrayAccess
+            // https://symfony.com/doc/current/components/property_access.html#reading-from-arrays
+
+            $propertyPath = self::convertToArrayPropertyPath($propertyPath);
+        }
+
+        if (is_object($subject)) {
+            return self::getObjectPropertyValue($subject, $propertyPath, $forceDirectAccess);
+        }
+
+        if (is_array($subject)) {
+            return self::getArrayIndexValue($subject, self::convertToArrayPropertyPath($propertyPath));
         }
 
         return null;
@@ -141,11 +135,10 @@ class ObjectAccess
      *
      * @return mixed Value of the property
      */
-    public static function getPropertyPath($subject, $propertyPath)
+    public static function getPropertyPath($subject, string $propertyPath)
     {
-        $propertyPathSegments = explode('.', $propertyPath);
         try {
-            foreach ($propertyPathSegments as $pathSegment) {
+            foreach (new PropertyPath($propertyPath) as $pathSegment) {
                 $subject = self::getPropertyInternal($subject, $pathSegment);
             }
         } catch (Exception\PropertyNotAccessibleException $error) {
@@ -173,8 +166,12 @@ class ObjectAccess
      * @throws \InvalidArgumentException in case $object was not an object or $propertyName was not a string
      * @return bool TRUE if the property could be set, FALSE otherwise
      */
-    public static function setProperty(&$subject, $propertyName, $propertyValue, $forceDirectAccess = false)
+    public static function setProperty(&$subject, string $propertyName, $propertyValue, bool $forceDirectAccess = false): bool
     {
+        if ($forceDirectAccess === true) {
+            trigger_error('Argument $forceDirectAccess will be removed in TYPO3 11.0', E_USER_DEPRECATED);
+        }
+
         if (is_array($subject) || ($subject instanceof \ArrayAccess && !$forceDirectAccess)) {
             $subject[$propertyName] = $propertyValue;
             return true;
@@ -182,10 +179,13 @@ class ObjectAccess
         if (!is_object($subject)) {
             throw new \InvalidArgumentException('subject must be an object or array, ' . gettype($subject) . ' given.', 1237301368);
         }
-        if (!is_string($propertyName)) {
-            throw new \InvalidArgumentException('Given property name is not of type string.', 1231178878);
+
+        $accessor = self::createAccessor();
+        if ($accessor->isWritable($subject, $propertyName)) {
+            $accessor->setValue($subject, $propertyName, $propertyValue);
+            return true;
         }
-        $result = true;
+
         if ($forceDirectAccess) {
             if (property_exists($subject, $propertyName)) {
                 $propertyReflection = new \ReflectionProperty($subject, $propertyName);
@@ -194,22 +194,11 @@ class ObjectAccess
             } else {
                 $subject->{$propertyName} = $propertyValue;
             }
-            return $result;
-        }
-        $setterMethodName = self::buildSetterMethodName($propertyName);
-        if (is_callable([$subject, $setterMethodName])) {
-            $subject->{$setterMethodName}($propertyValue);
-        } elseif (property_exists($subject, $propertyName)) {
-            $reflection = new \ReflectionProperty($subject, $propertyName);
-            if ($reflection->isPublic()) {
-                $subject->{$propertyName} = $propertyValue;
-            } else {
-                $result = false;
-            }
-        } else {
-            $result = false;
+
+            return true;
         }
-        return $result;
+
+        return false;
     }
 
     /**
@@ -221,51 +210,55 @@ class ObjectAccess
      *
      * @param object $object Object to receive property names for
      *
-     * @throws \InvalidArgumentException
      * @return array Array of all gettable property names
+     * @throws Exception\UnknownClassException
      */
-    public static function getGettablePropertyNames($object)
+    public static function getGettablePropertyNames(object $object): array
     {
-        if (!is_object($object)) {
-            throw new \InvalidArgumentException('$object must be an object, ' . gettype($object) . ' given.', 1237301369);
-        }
         if ($object instanceof \stdClass) {
             $properties = array_keys((array)$object);
             sort($properties);
             return $properties;
         }
 
-        $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;
-                    }
+        $classSchema = GeneralUtility::makeInstance(ReflectionService::class)
+            ->getClassSchema($object);
+
+        $accessor = self::createAccessor();
+        $propertyNames = array_keys($classSchema->getProperties());
+        $accessiblePropertyNames = array_filter($propertyNames, function ($propertyName) use ($accessor, $object) {
+            return $accessor->isReadable($object, $propertyName);
+        });
+
+        foreach ($classSchema->getMethods() as $methodName => $methodDefinition) {
+            if (!$methodDefinition->isPublic()) {
+                continue;
+            }
+
+            foreach ($methodDefinition->getParameters() as $methodParam) {
+                if (!$methodParam->isOptional()) {
+                    continue 2;
                 }
             }
-            $methodName = $method->getName();
-            if (strpos($methodName, 'is') === 0) {
-                $declaredPropertyNames[] = lcfirst(substr($methodName, 2));
+
+            if (StringUtility::beginsWith($methodName, 'get')) {
+                $accessiblePropertyNames[] = lcfirst(substr($methodName, 3));
+                continue;
             }
-            if (strpos($methodName, 'get') === 0) {
-                $declaredPropertyNames[] = lcfirst(substr($methodName, 3));
+
+            if (StringUtility::beginsWith($methodName, 'has')) {
+                $accessiblePropertyNames[] = lcfirst(substr($methodName, 3));
+                continue;
             }
-            if (strpos($methodName, 'has') === 0) {
-                $declaredPropertyNames[] = lcfirst(substr($methodName, 3));
+
+            if (StringUtility::beginsWith($methodName, 'is')) {
+                $accessiblePropertyNames[] = lcfirst(substr($methodName, 2));
             }
         }
-        $propertyNames = array_unique($declaredPropertyNames);
-        sort($propertyNames);
 
-        return $propertyNames;
+        $accessiblePropertyNames = array_unique($accessiblePropertyNames);
+        sort($accessiblePropertyNames);
+        return $accessiblePropertyNames;
     }
 
     /**
@@ -280,22 +273,29 @@ class ObjectAccess
      * @throws \InvalidArgumentException
      * @return array Array of all settable property names
      */
-    public static function getSettablePropertyNames($object)
+    public static function getSettablePropertyNames(object $object): array
     {
-        if (!is_object($object)) {
-            throw new \InvalidArgumentException('$object must be an object, ' . gettype($object) . ' given.', 1264022994);
-        }
-        if ($object instanceof \stdClass) {
-            $declaredPropertyNames = array_keys((array)$object);
+        $accessor = self::createAccessor();
+
+        if ($object instanceof \stdClass || $object instanceof \ArrayAccess) {
+            $propertyNames = array_keys((array)$object);
         } else {
-            $declaredPropertyNames = array_keys(get_class_vars(get_class($object)));
-        }
-        foreach (get_class_methods($object) as $methodName) {
-            if (strpos($methodName, 'set') === 0 && is_callable([$object, $methodName])) {
-                $declaredPropertyNames[] = lcfirst(substr($methodName, 3));
+            $classSchema = GeneralUtility::makeInstance(ReflectionService::class)->getClassSchema($object);
+
+            $propertyNames = array_filter(array_keys($classSchema->getProperties()), function ($methodName) use ($accessor, $object) {
+                return $accessor->isWritable($object, $methodName);
+            });
+
+            $setters = array_filter(array_keys($classSchema->getMethods()), function ($methodName) use ($object) {
+                return StringUtility::beginsWith($methodName, 'set') && is_callable([$object, $methodName]);
+            });
+
+            foreach ($setters as $setter) {
+                $propertyNames[] = lcfirst(substr($setter, 3));
             }
         }
-        $propertyNames = array_unique($declaredPropertyNames);
+
+        $propertyNames = array_unique($propertyNames);
         sort($propertyNames);
         return $propertyNames;
     }
@@ -303,24 +303,21 @@ class ObjectAccess
     /**
      * Tells if the value of the specified property can be set by this Object Accessor.
      *
-     * @param object $object Object containting the property
+     * @param object $object Object containing the property
      * @param string $propertyName Name of the property to check
      *
      * @throws \InvalidArgumentException
      * @return bool
      */
-    public static function isPropertySettable($object, $propertyName)
+    public static function isPropertySettable(object $object, $propertyName): bool
     {
-        if (!is_object($object)) {
-            throw new \InvalidArgumentException('$object must be an object, ' . gettype($object) . ' given.', 1259828920);
-        }
         if ($object instanceof \stdClass && array_key_exists($propertyName, get_object_vars($object))) {
             return true;
         }
         if (array_key_exists($propertyName, get_class_vars(get_class($object)))) {
             return true;
         }
-        return is_callable([$object, self::buildSetterMethodName($propertyName)]);
+        return is_callable([$object, 'set' . ucfirst($propertyName)]);
     }
 
     /**
@@ -332,31 +329,17 @@ class ObjectAccess
      * @throws \InvalidArgumentException
      * @return bool
      */
-    public static function isPropertyGettable($object, $propertyName)
+    public static function isPropertyGettable($object, $propertyName): bool
     {
-        if (!is_object($object)) {
-            throw new \InvalidArgumentException('$object must be an object, ' . gettype($object) . ' given.', 1259828921);
-        }
-        if ($object instanceof \ArrayAccess && isset($object[$propertyName])) {
-            return true;
-        }
-        if ($object instanceof \stdClass && isset($object->$propertyName)) {
-            return true;
+        if (($object instanceof \ArrayAccess) && !$object->offsetExists($propertyName)) {
+            return false;
         }
-        if (is_callable([$object, 'get' . ucfirst($propertyName)])) {
-            return true;
-        }
-        if (is_callable([$object, 'has' . ucfirst($propertyName)])) {
-            return true;
-        }
-        if (is_callable([$object, 'is' . ucfirst($propertyName)])) {
-            return true;
-        }
-        if (property_exists($object, $propertyName)) {
-            $propertyReflection = new \ReflectionProperty($object, $propertyName);
-            return $propertyReflection->isPublic();
+
+        if (is_array($object) || $object instanceof \ArrayAccess) {
+            $propertyName = self::wrap($propertyName);
         }
-        return false;
+
+        return self::createAccessor()->isReadable($object, $propertyName);
     }
 
     /**
@@ -369,11 +352,8 @@ class ObjectAccess
      * @return array Associative array of all properties.
      * @todo What to do with ArrayAccess
      */
-    public static function getGettableProperties($object)
+    public static function getGettableProperties(object $object): array
     {
-        if (!is_object($object)) {
-            throw new \InvalidArgumentException('$object must be an object, ' . gettype($object) . ' given.', 1237301370);
-        }
         $properties = [];
         foreach (self::getGettablePropertyNames($object) as $propertyName) {
             $properties[$propertyName] = self::getPropertyInternal($object, $propertyName);
@@ -388,9 +368,88 @@ class ObjectAccess
      * @param string $propertyName Name of the property
      *
      * @return string Name of the setter method name
+     * @deprecated
      */
-    public static function buildSetterMethodName($propertyName)
+    public static function buildSetterMethodName($propertyName): string
     {
+        trigger_error(__METHOD__ . ' will be removed in TYPO3 11.0', E_USER_DEPRECATED);
+
         return 'set' . ucfirst($propertyName);
     }
+
+    /**
+     * @return PropertyAccessor
+     */
+    private static function createAccessor(): PropertyAccessor
+    {
+        if (static::$propertyAccessor === null) {
+            static::$propertyAccessor = PropertyAccess::createPropertyAccessorBuilder()
+                ->getPropertyAccessor();
+        }
+
+        return static::$propertyAccessor;
+    }
+
+    /**
+     * @param object $subject
+     * @param PropertyPath $propertyPath
+     * @param bool $forceDirectAccess
+     * @return mixed
+     * @throws Exception\PropertyNotAccessibleException
+     * @throws \ReflectionException
+     */
+    private static function getObjectPropertyValue(object $subject, PropertyPath $propertyPath, bool $forceDirectAccess)
+    {
+        $accessor = self::createAccessor();
+
+        if ($accessor->isReadable($subject, $propertyPath)) {
+            return $accessor->getValue($subject, $propertyPath);
+        }
+
+        $propertyName = (string)$propertyPath;
+
+        if (!$forceDirectAccess) {
+            throw new Exception\PropertyNotAccessibleException('The property "' . $propertyName . '" on the subject does not exist.', 1476109666);
+        }
+
+        if (!property_exists($subject, $propertyName)) {
+            throw new Exception\PropertyNotAccessibleException('The property "' . $propertyName . '" on the subject does not exist.', 1302855001);
+        }
+
+        $propertyReflection = new \ReflectionProperty($subject, $propertyName);
+        $propertyReflection->setAccessible(true);
+        return $propertyReflection->getValue($subject);
+    }
+
+    /**
+     * @param array $subject
+     * @param PropertyPath $propertyPath
+     * @return mixed
+     */
+    private static function getArrayIndexValue(array $subject, PropertyPath $propertyPath)
+    {
+        return self::createAccessor()->getValue($subject, $propertyPath);
+    }
+
+    /**
+     * @param PropertyPath $propertyPath
+     * @return PropertyPath
+     */
+    private static function convertToArrayPropertyPath(PropertyPath $propertyPath): PropertyPath
+    {
+        $segments = array_map(function ($segment) {
+            return static::wrap($segment);
+        }, $propertyPath->getElements());
+
+        return new PropertyPath(implode('.', $segments));
+    }
+
+    /**
+     * @param string $segment
+     * @return string
+     */
+    private static function wrap(string $segment): string
+    {
+        return '[' . $segment . ']';
+    }
 }
index def5f15..2448c49 100644 (file)
@@ -67,7 +67,15 @@ class GenericObjectValidator extends AbstractValidator implements ObjectValidato
         if (ObjectAccess::isPropertyGettable($object, $propertyName)) {
             return ObjectAccess::getProperty($object, $propertyName);
         }
-        return ObjectAccess::getProperty($object, $propertyName, true);
+        throw new \RuntimeException(
+            sprintf(
+                'Could not get value of property "%s::%s", make sure the property is either public or has a getter get%3$s(), a hasser has%3$s() or an isser is%3$s().',
+                get_class($object),
+                $propertyName,
+                ucfirst($propertyName)
+            ),
+            1546632293
+        );
     }
 
     /**
index 90b30f3..fdd03e3 100644 (file)
@@ -27,6 +27,8 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
  */
 class JsonViewTest extends UnitTestCase
 {
+    protected $resetSingletonInstances = true;
+
     /**
      * @var \TYPO3\CMS\Extbase\Mvc\View\JsonView
      */
index ddcb864..3dab3be 100644 (file)
@@ -25,6 +25,11 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 class ObjectAccessTest extends UnitTestCase
 {
     /**
+     * @var bool Reset singletons created by subject
+     */
+    protected $resetSingletonInstances = true;
+
+    /**
      * @var DummyClassWithGettersAndSetters
      */
     protected $dummyObject;
@@ -61,35 +66,6 @@ class ObjectAccessTest extends UnitTestCase
     /**
      * @test
      */
-    public function getPropertyReturnsExpectedValueForUnexposedPropertyIfForceDirectAccessIsTrue()
-    {
-        $property = ObjectAccess::getProperty($this->dummyObject, 'unexposedProperty', true);
-        $this->assertEquals($property, 'unexposed', 'A property of a given object was not returned correctly.');
-    }
-
-    /**
-     * @test
-     */
-    public function getPropertyReturnsExpectedValueForUnknownPropertyIfForceDirectAccessIsTrue()
-    {
-        $this->dummyObject->unknownProperty = 'unknown';
-        $property = ObjectAccess::getProperty($this->dummyObject, 'unknownProperty', true);
-        $this->assertEquals($property, 'unknown', 'A property of a given object was not returned correctly.');
-    }
-
-    /**
-     * @test
-     */
-    public function getPropertyThrowsPropertyNotAccessibleExceptionForNotExistingPropertyIfForceDirectAccessIsTrue()
-    {
-        $this->expectException(PropertyNotAccessibleException::class);
-        $this->expectExceptionCode(1302855001);
-        ObjectAccess::getProperty($this->dummyObject, 'notExistingProperty', true);
-    }
-
-    /**
-     * @test
-     */
     public function getPropertyThrowsExceptionIfPropertyDoesNotExist()
     {
         $this->expectException(PropertyNotAccessibleException::class);
@@ -118,26 +94,6 @@ class ObjectAccessTest extends UnitTestCase
     /**
      * @test
      */
-    public function getPropertyThrowsExceptionIfThePropertyNameIsNotAString()
-    {
-        $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionCode(1231178303);
-        ObjectAccess::getProperty($this->dummyObject, new \ArrayObject());
-    }
-
-    /**
-     * @test
-     */
-    public function setPropertyThrowsExceptionIfThePropertyNameIsNotAString()
-    {
-        $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionCode(1231178878);
-        ObjectAccess::setProperty($this->dummyObject, new \ArrayObject(), 42);
-    }
-
-    /**
-     * @test
-     */
     public function setPropertyReturnsFalseIfPropertyIsNotAccessible()
     {
         $this->assertFalse(ObjectAccess::setProperty($this->dummyObject, 'protectedProperty', 42));
@@ -146,24 +102,6 @@ class ObjectAccessTest extends UnitTestCase
     /**
      * @test
      */
-    public function setPropertySetsValueIfPropertyIsNotAccessibleWhenForceDirectAccessIsTrue()
-    {
-        $this->assertTrue(ObjectAccess::setProperty($this->dummyObject, 'unexposedProperty', 'was set anyway', true));
-        $this->assertAttributeEquals('was set anyway', 'unexposedProperty', $this->dummyObject);
-    }
-
-    /**
-     * @test
-     */
-    public function setPropertySetsValueIfPropertyDoesNotExistWhenForceDirectAccessIsTrue()
-    {
-        $this->assertTrue(ObjectAccess::setProperty($this->dummyObject, 'unknownProperty', 'was set anyway', true));
-        $this->assertAttributeEquals('was set anyway', 'unknownProperty', $this->dummyObject);
-    }
-
-    /**
-     * @test
-     */
     public function setPropertyCallsASetterMethodToSetThePropertyValueIfOneIsAvailable()
     {
         ObjectAccess::setProperty($this->dummyObject, 'property', 4242);
index fd3da88..9c1a38c 100644 (file)
@@ -166,7 +166,6 @@ class CollectionValidatorTest extends UnitTestCase
             'someProperty',
             ['someNotEmptyValue']
         );
-        \TYPO3\CMS\Extbase\Reflection\ObjectAccess::setProperty($lazyObjectStorage, 'isInitialized', false, true);
         // only in this test case we want to mock the isValid method
         $validator = $this->getValidator(['elementType' => $elementType], ['isValid']);
         $validator->expects($this->never())->method('isValid');
index 6b39c92..c76e450 100644 (file)
@@ -58,6 +58,16 @@ class GenericObjectValidatorTest extends UnitTestCase
         $objectWithPrivateProperties = new class() {
             protected $foo = 'foovalue';
             protected $bar = 'barvalue';
+
+            public function getFoo()
+            {
+                return $this->foo;
+            }
+
+            public function getBar()
+            {
+                return $this->bar;
+            }
         };
 
         return [
diff --git a/typo3/sysext/extbase/Tests/UnitDeprecated/Reflection/ObjectAccessTest.php b/typo3/sysext/extbase/Tests/UnitDeprecated/Reflection/ObjectAccessTest.php
new file mode 100644 (file)
index 0000000..948257d
--- /dev/null
@@ -0,0 +1,95 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Extbase\Tests\UnitDeprecated\Reflection;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+use TYPO3\CMS\Extbase\Reflection\Exception\PropertyNotAccessibleException;
+use TYPO3\CMS\Extbase\Reflection\ObjectAccess;
+use TYPO3\CMS\Extbase\Tests\Unit\Reflection\Fixture\DummyClassWithGettersAndSetters;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+/**
+ * Test case
+ */
+class ObjectAccessTest extends UnitTestCase
+{
+    /**
+     * @var bool Reset singletons created by subject
+     */
+    protected $resetSingletonInstances = true;
+
+    /**
+     * @var DummyClassWithGettersAndSetters
+     */
+    protected $dummyObject;
+
+    /**
+     * Set up
+     */
+    protected function setUp()
+    {
+        $this->dummyObject = new DummyClassWithGettersAndSetters();
+        $this->dummyObject->setProperty('string1');
+        $this->dummyObject->setAnotherProperty(42);
+        $this->dummyObject->shouldNotBePickedUp = true;
+    }
+
+    /**
+     * @test
+     */
+    public function getPropertyReturnsExpectedValueForUnexposedPropertyIfForceDirectAccessIsTrue()
+    {
+        $property = ObjectAccess::getProperty($this->dummyObject, 'unexposedProperty', true);
+        $this->assertEquals($property, 'unexposed', 'A property of a given object was not returned correctly.');
+    }
+
+    /**
+     * @test
+     */
+    public function getPropertyReturnsExpectedValueForUnknownPropertyIfForceDirectAccessIsTrue()
+    {
+        $this->dummyObject->unknownProperty = 'unknown';
+        $property = ObjectAccess::getProperty($this->dummyObject, 'unknownProperty', true);
+        $this->assertEquals($property, 'unknown', 'A property of a given object was not returned correctly.');
+    }
+
+    /**
+     * @test
+     */
+    public function getPropertyThrowsPropertyNotAccessibleExceptionForNotExistingPropertyIfForceDirectAccessIsTrue()
+    {
+        $this->expectException(PropertyNotAccessibleException::class);
+        $this->expectExceptionCode(1302855001);
+        ObjectAccess::getProperty($this->dummyObject, 'notExistingProperty', true);
+    }
+
+    /**
+     * @test
+     */
+    public function setPropertySetsValueIfPropertyIsNotAccessibleWhenForceDirectAccessIsTrue()
+    {
+        $this->assertTrue(ObjectAccess::setProperty($this->dummyObject, 'unexposedProperty', 'was set anyway', true));
+        $this->assertAttributeEquals('was set anyway', 'unexposedProperty', $this->dummyObject);
+    }
+
+    /**
+     * @test
+     */
+    public function setPropertySetsValueIfPropertyDoesNotExistWhenForceDirectAccessIsTrue()
+    {
+        $this->assertTrue(ObjectAccess::setProperty($this->dummyObject, 'unknownProperty', 'was set anyway', true));
+        $this->assertAttributeEquals('was set anyway', 'unknownProperty', $this->dummyObject);
+    }
+}
index c00971e..c8a7842 100644 (file)
@@ -14,6 +14,7 @@
        },
        "require": {
                "phpdocumentor/reflection-docblock": "^4.3",
+               "symfony/property-access": "^4.2",
                "symfony/property-info": "^4.2",
                "typo3/cms-core": "10.0.*@dev",
                "webmozart/assert": "^1.0"
index 7953365..950dffc 100644 (file)
@@ -47,4 +47,22 @@ return [
             'Deprecation-85801-GeneralUtilityexplodeUrl2Array-2ndMethodArgument.rst',
         ],
     ],
+    'TYPO3\CMS\Extbase\Reflection\ObjectAccess::getProperty' => [
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst',
+        ],
+    ],
+    'TYPO3\CMS\Extbase\Reflection\ObjectAccess::getPropertyInternal' => [
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst',
+        ],
+    ],
+    'TYPO3\CMS\Extbase\Reflection\ObjectAccess::setProperty' => [
+        'maximumNumberOfArguments' => 3,
+        'restFiles' => [
+            'Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst',
+        ],
+    ],
 ];
index fb54759..5c98003 100644 (file)
@@ -854,4 +854,11 @@ return [
             'Deprecation-87550-UseControllerClassesWhenRegisteringPluginsmodules.rst',
         ],
     ],
+    'TYPO3\CMS\Extbase\Reflection\ObjectAccess::buildSetterMethodName' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst',
+        ],
+    ],
 ];