[BUGFIX] Check identity map for existing objects 52/25252/11
authorNico de Haen <mail@ndh-websolutions.de>
Sun, 10 Nov 2013 23:35:36 +0000 (00:35 +0100)
committerSteffen Müller <typo3@t3node.com>
Tue, 18 Feb 2014 22:35:09 +0000 (23:35 +0100)
The DataMapper should check if an object is already in the identityMap
before calling fetchRelated, since otherwise the object will be retrieved
from the database again.

Resolves: #53514
Releases: 6.1,6.2
Change-Id: I24e262322f1f0ba3c346fa01c50fa9063866aef7
Reviewed-on: https://review.typo3.org/25252
Reviewed-by: Nico de Haen
Reviewed-by: Fabien Udriot
Tested-by: Wouter Wolters
Reviewed-by: Steffen Müller
Tested-by: Steffen Müller
typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php

index bce9ea0..7519b8d 100644 (file)
@@ -221,7 +221,11 @@ class DataMapper implements \TYPO3\CMS\Core\SingletonInterface {
                                        case 'SplObjectStorage':
                                        case 'Tx_Extbase_Persistence_ObjectStorage':
                                        case 'TYPO3\\CMS\\Extbase\\Persistence\\ObjectStorage':
-                                               $propertyValue = $this->mapResultToPropertyValue($object, $propertyName, $this->fetchRelated($object, $propertyName, $row[$columnName]));
+                                               $propertyValue = $this->mapResultToPropertyValue(
+                                                       $object,
+                                                       $propertyName,
+                                                       $this->fetchRelated($object, $propertyName, $row[$columnName])
+                                               );
                                                break;
                                        default:
                                                if ($propertyData['type'] === 'DateTime' || in_array('DateTime', class_parents($propertyData['type']))) {
@@ -229,8 +233,13 @@ class DataMapper implements \TYPO3\CMS\Core\SingletonInterface {
                                                } elseif (TypeHandlingUtility::isCoreType($propertyData['type'])) {
                                                        $propertyValue = $this->mapCoreType($propertyData['type'], $row[$columnName]);
                                                } else {
-                                                       $propertyValue = $this->mapResultToPropertyValue($object, $propertyName, $this->fetchRelated($object, $propertyName, $row[$columnName]));
+                                                       $propertyValue = $this->mapObjectToClassProperty(
+                                                               $object,
+                                                               $propertyName,
+                                                               $row[$columnName]
+                                                       );
                                                }
+
                                }
                        }
                        if ($propertyValue !== NULL) {
@@ -408,6 +417,50 @@ class DataMapper implements \TYPO3\CMS\Core\SingletonInterface {
        }
 
        /**
+        * Returns the mapped classProperty from the identiyMap or
+        * mapResultToPropertyValue()
+        *
+        * If the field value is empty and the column map has no parent key field name,
+        * the relation will be empty. If the identityMap has a registered object of
+        * the correct type and identity (fieldValue), this function returns that object.
+        * Otherwise, it proceeds with mapResultToPropertyValue().
+        *
+        * @param \TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface $parentObject
+        * @param string $propertyName
+        * @param mixed $fieldValue the raw field value
+        * @return mixed
+        * @see mapResultToPropertyValue()
+        */
+       protected function mapObjectToClassProperty(\TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface $parentObject, $propertyName, $fieldValue) {
+               if (empty($fieldValue) && !$this->propertyMapsByForeignKey($parentObject, $propertyName)) {
+                       $propertyValue = $this->getEmptyRelationValue($parentObject, $propertyName);
+               } else {
+                       $propertyMetaData = $this->reflectionService->getClassSchema(get_class($parentObject))->getProperty($propertyName);
+
+                       if ($this->persistenceSession->hasIdentifier($fieldValue, $propertyMetaData['type'])) {
+                               $propertyValue = $this->persistenceSession->getObjectByIdentifier($fieldValue, $propertyMetaData['type']);
+                       } else {
+                               $result = $this->fetchRelated($parentObject, $propertyName, $fieldValue);
+                               $propertyValue = $this->mapResultToPropertyValue($parentObject, $propertyName, $result);
+                       }
+               }
+
+               return $propertyValue;
+       }
+
+       /**
+        * Checks if the relation is based on a foreign key.
+        *
+        * @param \TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface $parentObject
+        * @param string $propertyName
+        * @return boolean TRUE if the property is mapped
+        */
+       protected function propertyMapsByForeignKey(\TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface $parentObject, $propertyName) {
+               $columnMap = $this->getDataMap(get_class($parentObject))->getColumnMap($propertyName);
+               return ($columnMap->getParentKeyFieldName() !== NULL);
+       }
+
+       /**
         * Returns the given result as property value of the specified property type.
         *
         * @param \TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface $parentObject
index 751d961..efb5a92 100644 (file)
@@ -140,6 +140,69 @@ class DataMapperTest extends \TYPO3\CMS\Extbase\Tests\Unit\BaseTestCase {
        }
 
        /**
+        * Test if fetchRelatedEager method returns NULL when $fieldValue = ''
+        * and relation type == RELATION_HAS_ONE without calling fetchRelated
+        *
+        * @test
+        */
+       public function mapObjectToClassPropertyReturnsNullForEmptyRelationHasOne() {
+               $columnMap = new \TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap('columnName', 'propertyName');
+               $columnMap->setTypeOfRelation(\TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap::RELATION_HAS_ONE);
+               $dataMap = $this->getMock('TYPO3\\CMS\\Extbase\\Persistence\\Generic\\Mapper\\DataMap', array('getColumnMap'), array(), '', FALSE);
+               $dataMap->expects($this->any())->method('getColumnMap')->will($this->returnValue($columnMap));
+               $dataMapper = $this->getAccessibleMock('TYPO3\\CMS\\Extbase\\Persistence\\Generic\\Mapper\\DataMapper', array('getDataMap', 'fetchRelated'));
+               $dataMapper->expects($this->any())->method('getDataMap')->will($this->returnValue($dataMap));
+               $dataMapper->expects($this->never())->method('fetchRelated');
+               $result = $dataMapper->_call('mapObjectToClassProperty', $this->getMock('TYPO3\\CMS\\Extbase\\DomainObject\\AbstractEntity'), 'SomeName', '');
+               $this->assertEquals(NULL, $result);
+       }
+
+       /**
+        * Test if mapObjectToClassProperty method returns objects
+        * that are already registered in the persistence session
+        * without query it from the persistence layer
+        *
+        * @test
+        */
+       public function mapObjectToClassPropertyReturnsExistingObjectWithoutCallingFetchRelated() {
+               $columnMap = new \TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap('columnName', 'propertyName');
+               $columnMap->setTypeOfRelation(\TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap::RELATION_HAS_ONE);
+               $dataMap = $this->getMock('TYPO3\\CMS\\Extbase\\Persistence\\Generic\\Mapper\\DataMap', array('getColumnMap'), array(), '', FALSE);
+
+               $className = 'Class1' . md5(uniqid(mt_rand(), TRUE));
+               $classNameWithNS = __NAMESPACE__ . '\\' . $className;
+               eval('namespace ' . __NAMESPACE__ . '; class ' . $className . ' extends \\TYPO3\\CMS\\Extbase\\DomainObject\\AbstractEntity { public $relationProperty; }');
+               $object = new $classNameWithNS();
+               $classSchema1 = $this->getAccessibleMock('TYPO3\CMS\Extbase\Reflection\ClassSchema', array('dummy'), array($classNameWithNS));
+               $classSchema1->_set('typeHandlingService', new \TYPO3\CMS\Extbase\Service\TypeHandlingService());
+
+               $className2 = 'Class2' . md5(uniqid(mt_rand(), TRUE));
+               $className2WithNS = __NAMESPACE__ . '\\' . $className2;
+               eval('namespace ' . __NAMESPACE__ . '; class ' . $className2 . ' extends \\TYPO3\\CMS\\Extbase\\DomainObject\\AbstractEntity { }');
+               $child = new $className2WithNS();
+               $classSchema2 = $this->getAccessibleMock('TYPO3\CMS\Extbase\Reflection\ClassSchema', array('dummy'), array($className2WithNS));
+               $classSchema2->_set('typeHandlingService', new \TYPO3\CMS\Extbase\Service\TypeHandlingService());
+
+               $classSchema1->addProperty('relationProperty', $className2WithNS);
+               $identifier = 1;
+
+               $session = new \TYPO3\CMS\Extbase\Persistence\Generic\Session();
+               $session->registerObject($child, $identifier);
+
+               $mockReflectionService = $this->getMock('\TYPO3\CMS\Extbase\Reflection\ReflectionService', array('getClassSchema'), array(), '', FALSE);
+               $mockReflectionService->expects($this->any())->method('getClassSchema')->will($this->returnValue($classSchema1));
+
+               $dataMap->expects($this->any())->method('getColumnMap')->will($this->returnValue($columnMap));
+               $dataMapper = $this->getAccessibleMock('TYPO3\\CMS\\Extbase\\Persistence\\Generic\\Mapper\\DataMapper', array('getDataMap', 'getNonEmptyRelationValue'));
+               $dataMapper->_set('reflectionService', $mockReflectionService);
+               $dataMapper->_set('persistenceSession', $session);
+               $dataMapper->expects($this->any())->method('getDataMap')->will($this->returnValue($dataMap));
+               $dataMapper->expects($this->never())->method('getNonEmptyRelationValue');
+               $result = $dataMapper->_call('mapObjectToClassProperty', $object, 'relationProperty', $identifier);
+               $this->assertEquals($child, $result);
+       }
+
+       /**
         * Data provider for date checks. Date will be stored based on UTC in
         * the database. That's why it's not possible to check for explicit date
         * strings but using the date('c') conversion instead, which considers the