[TASK] Streamline ClassSchema property api 01/61101/7
authorAlexander Schnitzler <git@alexanderschnitzler.de>
Thu, 20 Jun 2019 14:32:10 +0000 (16:32 +0200)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Fri, 5 Jul 2019 12:48:55 +0000 (14:48 +0200)
This patch tackles three different issues with the current
ClassSchema property api.

1) Due to the history and the refactoring of the ReflectionService,
   the ClassSchema class cached data redundantly. Information like
   the type of a property has been stored multiple times in different
   cache keys. With this patch, the redundancy has been removed.

2) The information about properties had been roughly grouped by the
   ones gathered by php reflection and the ones gathered from doc
   blocks and/or annotations. This kind of grouping had also been
   exposed to the public by different methods in the Property class.

   Said grouping has been removed and the api does no longer allow
   for requesting annotation data. The purpose of the api is to tell
   the status quo of the information about a property, regardless of
   how said information had been gathered in the first place.

   Instead of exposing the annotation data itself via method
   `getAnnotationValue('lazy')`, there is a specific method for each
   relevant piece of information like `isLazy()`.

3) A lot of information about properties are boolean values which
   had been stored as separate keys in the cache. All boolean keys
   are now represented by a BitSet, which saves quite a lot of
   bytes during serialization of ClassSchema instances.

Releases: master
Resolves: #88612
Change-Id: I838c7b996275341d4ee65e6b6c6a4f7de1b92e28
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61101
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/extbase/Classes/Object/Container/Container.php
typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
typo3/sysext/extbase/Classes/Reflection/ClassSchema.php
typo3/sysext/extbase/Classes/Reflection/ClassSchema/Exception/NoSuchPropertyException.php
typo3/sysext/extbase/Classes/Reflection/ClassSchema/Property.php
typo3/sysext/extbase/Classes/Reflection/ClassSchema/PropertyCharacteristics.php [new file with mode: 0644]
typo3/sysext/extbase/Tests/Unit/Property/TypeConverter/PersistentObjectConverterTest.php
typo3/sysext/extbase/Tests/Unit/Reflection/ClassSchema/PropertyTest.php
typo3/sysext/extbase/Tests/Unit/Reflection/ClassSchemaTest.php

index ce6e95f..266cb6c 100644 (file)
@@ -200,8 +200,9 @@ class Container implements \TYPO3\CMS\Core\SingletonInterface
                 $instance->{$injectMethodName}($instanceToInject);
             }
         }
-        // todo: let getInjectProperties return Property objects
-        foreach ($classSchema->getInjectProperties() as $injectPropertyName => $classNameToInject) {
+        foreach ($classSchema->getInjectProperties() as $injectPropertyName => $injectProperty) {
+            $classNameToInject = $injectProperty->getType();
+
             $instanceToInject = $this->getInstanceInternal($classNameToInject);
             if ($classSchema->isSingleton() && !$instanceToInject instanceof \TYPO3\CMS\Core\SingletonInterface) {
                 $this->getLogger()->notice('The singleton "' . $classSchema->getClassName() . '" needs a prototype in "' . $injectPropertyName . '". This is often a bad code smell; often you rather want to inject a singleton.');
index 07a2597..a4d2d73 100644 (file)
@@ -420,7 +420,7 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
         $property = $this->reflectionService->getClassSchema($className)->getProperty($propertyName);
         foreach ($this->getRemovedChildObjects($parentObject, $propertyName) as $removedObject) {
             $this->detachObjectFromParentObject($removedObject, $parentObject, $propertyName);
-            if ($columnMap->getTypeOfRelation() === \TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap::RELATION_HAS_MANY && $property->getAnnotationValue('cascade') === 'remove') {
+            if ($columnMap->getTypeOfRelation() === \TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap::RELATION_HAS_MANY && $property->getCascadeValue() === 'remove') {
                 $this->removeEntity($removedObject);
             }
         }
@@ -1044,7 +1044,7 @@ class Backend implements \TYPO3\CMS\Extbase\Persistence\Generic\BackendInterface
                 continue;
             }
             $property = $classSchema->getProperty($propertyName);
-            if ($property->getAnnotationValue('cascade') === 'remove') {
+            if ($property->getCascadeValue() === 'remove') {
                 if ($columnMap->getTypeOfRelation() === \TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap::RELATION_HAS_MANY) {
                     foreach ($propertyValue as $containedObject) {
                         $this->removeEntity($containedObject);
index 0d92753..28bed8a 100644 (file)
@@ -351,7 +351,7 @@ class DataMapper
     public function fetchRelated(DomainObjectInterface $parentObject, $propertyName, $fieldValue = '', $enableLazyLoading = true)
     {
         $property = $this->reflectionService->getClassSchema(get_class($parentObject))->getProperty($propertyName);
-        if ($enableLazyLoading === true && $property->getAnnotationValue('lazy') === true) {
+        if ($enableLazyLoading === true && $property->isLazy()) {
             if ($property->getType() === Persistence\ObjectStorage::class) {
                 $result = $this->objectManager->get(\TYPO3\CMS\Extbase\Persistence\Generic\LazyObjectStorage::class, $parentObject, $propertyName, $fieldValue, $this);
             } else {
index d05bca7..2b3e724 100644 (file)
@@ -39,6 +39,7 @@ use TYPO3\CMS\Extbase\Reflection\ClassSchema\Exception\NoSuchMethodException;
 use TYPO3\CMS\Extbase\Reflection\ClassSchema\Exception\NoSuchPropertyException;
 use TYPO3\CMS\Extbase\Reflection\ClassSchema\Method;
 use TYPO3\CMS\Extbase\Reflection\ClassSchema\Property;
+use TYPO3\CMS\Extbase\Reflection\ClassSchema\PropertyCharacteristics;
 use TYPO3\CMS\Extbase\Reflection\DocBlock\Tags\Null_;
 use TYPO3\CMS\Extbase\Reflection\PropertyInfo\Extractor\PhpDocPropertyTypeExtractor;
 use TYPO3\CMS\Extbase\Utility\TypeHandlingUtility;
@@ -98,11 +99,6 @@ class ClassSchema
     /**
      * @var array
      */
-    private $injectProperties = [];
-
-    /**
-     * @var array
-     */
     private $injectMethods = [];
 
     /**
@@ -195,32 +191,26 @@ class ClassSchema
     {
         $annotationReader = new AnnotationReader();
 
+        $classHasInjectProperties = false;
         $defaultProperties = $reflectionClass->getDefaultProperties();
 
         foreach ($reflectionClass->getProperties() as $reflectionProperty) {
             $propertyName = $reflectionProperty->getName();
 
+            $propertyCharacteristicsBit = 0;
+            $propertyCharacteristicsBit += $reflectionProperty->isPrivate() ? PropertyCharacteristics::VISIBILITY_PRIVATE : 0;
+            $propertyCharacteristicsBit += $reflectionProperty->isProtected() ? PropertyCharacteristics::VISIBILITY_PROTECTED : 0;
+            $propertyCharacteristicsBit += $reflectionProperty->isPublic() ? PropertyCharacteristics::VISIBILITY_PUBLIC : 0;
+            $propertyCharacteristicsBit += $reflectionProperty->isStatic() ? PropertyCharacteristics::IS_STATIC : 0;
+
             $this->properties[$propertyName] = [
-                'default'      => $reflectionProperty->isDefault(),
-                'defaultValue' => $defaultProperties[$propertyName] ?? null,
-                'private'      => $reflectionProperty->isPrivate(),
-                'protected'    => $reflectionProperty->isProtected(),
-                'public'       => $reflectionProperty->isPublic(),
-                'static'       => $reflectionProperty->isStatic(),
-                'type'         => null, // Extbase
-                'elementType'  => null, // Extbase
-                'annotations'  => [],
-                'tags'         => [],
-                'validators'   => []
+                'c' => null, // cascade
+                'd' => $defaultProperties[$propertyName] ?? null, // defaultValue
+                'e' => null, // elementType
+                't' => null, // type
+                'v' => [] // validators
             ];
 
-            $this->properties[$propertyName]['annotations']['inject'] = false;
-            $this->properties[$propertyName]['annotations']['lazy'] = false;
-            $this->properties[$propertyName]['annotations']['transient'] = false;
-            $this->properties[$propertyName]['annotations']['type'] = null;
-            $this->properties[$propertyName]['annotations']['cascade'] = null;
-            $this->properties[$propertyName]['annotations']['dependency'] = null;
-
             $annotations = $annotationReader->getPropertyAnnotations($reflectionProperty);
 
             /** @var array|Validate[] $validateAnnotations */
@@ -232,7 +222,7 @@ class ClassSchema
                 foreach ($validateAnnotations as $validateAnnotation) {
                     $validatorObjectName = ValidatorClassNameResolver::resolve($validateAnnotation->validator);
 
-                    $this->properties[$propertyName]['validators'][] = [
+                    $this->properties[$propertyName]['v'][] = [
                         'name' => $validateAnnotation->validator,
                         'options' => $validateAnnotation->options,
                         'className' => $validatorObjectName,
@@ -241,16 +231,21 @@ class ClassSchema
             }
 
             if ($annotationReader->getPropertyAnnotation($reflectionProperty, Lazy::class) instanceof Lazy) {
-                $this->properties[$propertyName]['annotations']['lazy'] = true;
+                $propertyCharacteristicsBit += PropertyCharacteristics::ANNOTATED_LAZY;
             }
 
             if ($annotationReader->getPropertyAnnotation($reflectionProperty, Transient::class) instanceof Transient) {
-                $this->properties[$propertyName]['annotations']['transient'] = true;
+                $propertyCharacteristicsBit += PropertyCharacteristics::ANNOTATED_TRANSIENT;
             }
 
             $isInjectProperty = $propertyName !== 'settings'
                 && ($annotationReader->getPropertyAnnotation($reflectionProperty, Inject::class) instanceof Inject);
 
+            if ($isInjectProperty) {
+                $propertyCharacteristicsBit += PropertyCharacteristics::ANNOTATED_INJECT;
+                $classHasInjectProperties = true;
+            }
+
             /** @var Type[] $types */
             $types = (array)self::$propertyInfoExtractor->getTypes($this->className, $propertyName, ['reflectionProperty' => $reflectionProperty]);
             $typesCount = count($types);
@@ -259,19 +254,11 @@ class ClassSchema
                 && ($annotation = $annotationReader->getPropertyAnnotation($reflectionProperty, Cascade::class)) instanceof Cascade
             ) {
                 /** @var Cascade $annotation */
-                $this->properties[$propertyName]['annotations']['cascade'] = $annotation->value;
-            }
-
-            if ($isInjectProperty && ($type = $types[0]) instanceof Type) {
-                $this->properties[$propertyName]['annotations']['inject'] = true;
-                $this->properties[$propertyName]['annotations']['type'] = $type->getClassName();
-                $this->properties[$propertyName]['annotations']['dependency'] = $type->getClassName();
-
-                $this->injectProperties[] = $propertyName;
+                $this->properties[$propertyName]['c'] = $annotation->value;
             }
 
             if ($typesCount === 1) {
-                $this->properties[$propertyName]['type'] = $types[0]->getClassName() ?? $types[0]->getBuiltinType();
+                $this->properties[$propertyName]['t'] = $types[0]->getClassName() ?? $types[0]->getBuiltinType();
             } elseif ($typesCount === 2) {
                 [$type, $elementType] = $types;
                 $actualType = $type->getClassName() ?? $type->getBuiltinType();
@@ -281,13 +268,15 @@ class ClassSchema
                     && $elementType->getCollectionValueType() instanceof Type
                     && $elementType->getCollectionValueType()->getClassName() !== null
                 ) {
-                    $this->properties[$propertyName]['type'] = ltrim($actualType, '\\');
-                    $this->properties[$propertyName]['elementType'] = ltrim($elementType->getCollectionValueType()->getClassName(), '\\');
+                    $this->properties[$propertyName]['t'] = ltrim($actualType, '\\');
+                    $this->properties[$propertyName]['e'] = ltrim($elementType->getCollectionValueType()->getClassName(), '\\');
                 }
             }
+
+            $this->properties[$propertyName]['propertyCharacteristicsBit'] = $propertyCharacteristicsBit;
         }
 
-        if (count($this->injectProperties) > 0) {
+        if ($classHasInjectProperties) {
             $this->bitSet->set(self::BIT_CLASS_HAS_INJECT_PROPERTIES);
         }
     }
@@ -645,20 +634,18 @@ class ClassSchema
     }
 
     /**
-     * @return array
+     * @return array|Property[]
      */
     public function getInjectProperties(): array
     {
-        $injectProperties = [];
-        foreach ($this->injectProperties as $injectPropertyName) {
-            $injectProperties[$injectPropertyName] = $this->properties[$injectPropertyName]['annotations']['dependency'];
-        }
-
-        return $injectProperties;
+        return array_filter($this->buildPropertyObjects(), static function ($property) {
+            /** @var Property $property */
+            return $property->isInjectProperty();
+        });
     }
 
     /**
-     * @return array
+     * @return array|Property[]
      */
     private function buildPropertyObjects(): array
     {
index 537b578..b53f875 100644 (file)
@@ -3,6 +3,19 @@ declare(strict_types = 1);
 
 namespace TYPO3\CMS\Extbase\Reflection\ClassSchema\Exception;
 
+/*
+ * 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!
+ */
+
 /**
  * Class TYPO3\CMS\Extbase\Reflection\ClassSchema\Exception\NoSuchPropertyException
  */
index 7682734..0417bd1 100644 (file)
@@ -3,6 +3,19 @@ declare(strict_types = 1);
 
 namespace TYPO3\CMS\Extbase\Reflection\ClassSchema;
 
+/*
+ * 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!
+ */
+
 /**
  * Class TYPO3\CMS\Extbase\Reflection\ClassSchema\Property
  * @internal only to be used within Extbase, not part of TYPO3 Core API.
@@ -20,21 +33,26 @@ class Property
     private $definition;
 
     /**
+     * @var PropertyCharacteristics
+     */
+    private $characteristics;
+
+    /**
      * @param string $name
      * @param array $definition
      */
     public function __construct(string $name, array $definition)
     {
         $this->name = $name;
+        $this->characteristics = new PropertyCharacteristics($definition['propertyCharacteristicsBit']);
+        unset($definition['propertyCharacteristicsBit']);
 
         $defaults = [
-            'type' => null,
-            'elementType' => null,
-            'public' => false,
-            'protected' => false,
-            'private' => false,
-            'annotations' => [],
-            'validators' => [],
+            'c' => null, // cascade
+            'd' => null, // defaultValue
+            't' => null, // type
+            'e' => null, // elementType
+            'v' => [], // validators
         ];
 
         foreach ($defaults as $key => $defaultValue) {
@@ -63,7 +81,7 @@ class Property
      */
     public function getType(): ?string
     {
-        return $this->definition['type'];
+        return $this->definition['t'];
     }
 
     /**
@@ -78,7 +96,7 @@ class Property
      */
     public function getElementType(): ?string
     {
-        return $this->definition['elementType'];
+        return $this->definition['e'];
     }
 
     /**
@@ -86,7 +104,7 @@ class Property
      */
     public function isPublic(): bool
     {
-        return $this->definition['public'];
+        return $this->characteristics->get(PropertyCharacteristics::VISIBILITY_PUBLIC);
     }
 
     /**
@@ -94,7 +112,7 @@ class Property
      */
     public function isProtected(): bool
     {
-        return $this->definition['protected'];
+        return $this->characteristics->get(PropertyCharacteristics::VISIBILITY_PROTECTED);
     }
 
     /**
@@ -102,25 +120,31 @@ class Property
      */
     public function isPrivate(): bool
     {
-        return $this->definition['private'];
+        return $this->characteristics->get(PropertyCharacteristics::VISIBILITY_PRIVATE);
     }
 
     /**
-     * @param string $annotationKey
      * @return bool
      */
-    public function hasAnnotation(string $annotationKey): bool
+    public function isLazy(): bool
     {
-        return isset($this->definition['annotations'][$annotationKey]);
+        return $this->characteristics->get(PropertyCharacteristics::ANNOTATED_LAZY);
     }
 
     /**
-     * @param string $annotationKey
-     * @return mixed
+     * @return bool
      */
-    public function getAnnotationValue(string $annotationKey)
+    public function isTransient(): bool
     {
-        return $this->definition['annotations'][$annotationKey] ?? null;
+        return $this->characteristics->get(PropertyCharacteristics::ANNOTATED_TRANSIENT);
+    }
+
+    /**
+     * @return bool
+     */
+    public function isInjectProperty(): bool
+    {
+        return $this->characteristics->get(PropertyCharacteristics::ANNOTATED_INJECT);
     }
 
     /**
@@ -128,7 +152,7 @@ class Property
      */
     public function getValidators(): array
     {
-        return $this->definition['validators'];
+        return $this->definition['v'];
     }
 
     /**
@@ -136,6 +160,14 @@ class Property
      */
     public function getDefaultValue()
     {
-        return $this->definition['defaultValue'];
+        return $this->definition['d'];
+    }
+
+    /**
+     * @return string|null
+     */
+    public function getCascadeValue(): ?string
+    {
+        return $this->definition['c'];
     }
 }
diff --git a/typo3/sysext/extbase/Classes/Reflection/ClassSchema/PropertyCharacteristics.php b/typo3/sysext/extbase/Classes/Reflection/ClassSchema/PropertyCharacteristics.php
new file mode 100644 (file)
index 0000000..582f224
--- /dev/null
@@ -0,0 +1,33 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Extbase\Reflection\ClassSchema;
+
+/*
+ * 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\Core\Type\BitSet;
+
+/**
+ * @internal only to be used within Extbase, not part of TYPO3 Core API.
+ */
+final class PropertyCharacteristics extends BitSet
+{
+    public const VISIBILITY_PRIVATE = 1 << 0;
+    public const VISIBILITY_PROTECTED = 1 << 1;
+    public const VISIBILITY_PUBLIC = 1 << 2;
+    public const IS_STATIC = 1 << 3;
+    public const ANNOTATED_LAZY = 1 << 4;
+    public const ANNOTATED_TRANSIENT = 1 << 5;
+    public const ANNOTATED_INJECT = 1 << 6;
+}
index 5dba6b5..231825e 100644 (file)
@@ -157,8 +157,9 @@ class PersistentObjectConverterTest extends UnitTestCase
         $mockSchema->expects($this->any())->method('getProperty')->with('thePropertyName')->will($this->returnValue(new ClassSchema\Property(
             'thePropertyName',
             [
-                'type' => 'TheTypeOfSubObject',
-                'elementType' => null
+                'propertyCharacteristicsBit' => 0,
+                't' => 'TheTypeOfSubObject',
+                'e' => null,
             ]
         )));
         $configuration = $this->buildConfiguration([]);
index 8eb0cc3..42cc5cf 100644 (file)
@@ -37,7 +37,7 @@ class PropertyTest extends UnitTestCase
     public function classSchemaDetectsPropertiesWithLazyAnnotation(): void
     {
         $classSchema = new ClassSchema(DummyClassWithLazyDoctrineAnnotation::class);
-        static::assertTrue($classSchema->getProperty('propertyWithLazyAnnotation')->getAnnotationValue('lazy'));
+        static::assertTrue($classSchema->getProperty('propertyWithLazyAnnotation')->isLazy());
     }
 
     /**
@@ -71,8 +71,7 @@ class PropertyTest extends UnitTestCase
         $property = (new ClassSchema(DummyClassWithAllTypesOfProperties::class))
             ->getProperty('propertyWithInjectAnnotation');
 
-        static::assertTrue($property->hasAnnotation('inject'));
-        static::assertTrue($property->getAnnotationValue('inject'));
+        static::assertTrue($property->isInjectProperty());
     }
 
     /**
@@ -83,8 +82,7 @@ class PropertyTest extends UnitTestCase
         $property = (new ClassSchema(DummyClassWithAllTypesOfProperties::class))
             ->getProperty('propertyWithTransientAnnotation');
 
-        static::assertTrue($property->hasAnnotation('transient'));
-        static::assertTrue($property->getAnnotationValue('transient'));
+        static::assertTrue($property->isTransient());
     }
 
     /**
@@ -95,8 +93,7 @@ class PropertyTest extends UnitTestCase
         $property = (new ClassSchema(DummyClassWithAllTypesOfProperties::class))
             ->getProperty('propertyWithCascadeAnnotation');
 
-        static::assertTrue($property->hasAnnotation('cascade'));
-        static::assertSame('remove', $property->getAnnotationValue('cascade'));
+        static::assertSame('remove', $property->getCascadeValue());
     }
 
     /**
@@ -107,8 +104,7 @@ class PropertyTest extends UnitTestCase
         $property = (new ClassSchema(DummyClassWithAllTypesOfProperties::class))
             ->getProperty('propertyWithCascadeAnnotationWithoutVarAnnotation');
 
-        static::assertFalse($property->hasAnnotation('cascade'));
-        static::assertNull($property->getAnnotationValue('cascade'));
+        static::assertNull($property->getCascadeValue());
     }
 
     /**
index 1d767f6..1b971f2 100644 (file)
@@ -122,16 +122,16 @@ class ClassSchemaTest extends UnitTestCase
 
         $injectProperties = $classSchema->getInjectProperties();
         static::assertArrayHasKey('propertyWithFullQualifiedClassName', $injectProperties);
-        static::assertSame(Fixture\DummyClassWithInjectDoctrineAnnotation::class, $injectProperties['propertyWithFullQualifiedClassName']);
+        static::assertSame(Fixture\DummyClassWithInjectDoctrineAnnotation::class, $injectProperties['propertyWithFullQualifiedClassName']->getType());
 
         static::assertArrayHasKey('propertyWithRelativeClassName', $injectProperties);
-        static::assertSame(Fixture\DummyClassWithInjectDoctrineAnnotation::class, $injectProperties['propertyWithRelativeClassName']);
+        static::assertSame(Fixture\DummyClassWithInjectDoctrineAnnotation::class, $injectProperties['propertyWithRelativeClassName']->getType());
 
         static::assertArrayHasKey('propertyWithImportedClassName', $injectProperties);
-        static::assertSame(self::class, $injectProperties['propertyWithImportedClassName']);
+        static::assertSame(self::class, $injectProperties['propertyWithImportedClassName']->getType());
 
         static::assertArrayHasKey('propertyWithImportedAndAliasedClassName', $injectProperties);
-        static::assertSame(self::class, $injectProperties['propertyWithImportedAndAliasedClassName']);
+        static::assertSame(self::class, $injectProperties['propertyWithImportedAndAliasedClassName']->getType());
     }
 
     /**