From 09d4da5895e60dbcad6801288bbc3b0df7b9eb3a Mon Sep 17 00:00:00 2001
From: Matthias Vogel <typo3@kanti.de>
Date: Tue, 8 Aug 2017 19:20:38 +0200
Subject: [PATCH] [BUGFIX] Consider property clean if lazy loaded proxy is
untouched
Objects containing a LazyLoadingProxy will be marked dirty even if the
lazy proxy is untouched.
For more details see IsDirtyTest.
Resolves: #82065
Releases: master, 8.7
Change-Id: I579a275bb7d22af836be2497064ed09ea0203df6
Reviewed-on: https://review.typo3.org/53665
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Tomas Norre Mikkelsen <tomasnorre@gmail.com>
Reviewed-by: Andreas Wolf <andreas.wolf@typo3.org>
Reviewed-by: Claus Due <claus@phpmind.net>
Tested-by: Claus Due <claus@phpmind.net>
Tested-by: Benni Mack <benni@typo3.org>
---
.../DomainObject/AbstractDomainObject.php | 20 ++-
.../Persistence/Generic/LazyLoadingProxy.php | 9 ++
.../Configuration/TCA/Overrides/fe_users.php | 4 +-
.../TCA/tx_blogexample_domain_model_blog.php | 2 +-
.../Private/Language/locallang_db.xml | 2 +-
.../Functional/Persistence/Fixtures/blogs.xml | 2 +
.../Persistence/Fixtures/fe_users.xml | 13 +-
.../Functional/Persistence/IsDirtyTest.php | 151 ++++++++++++++++++
.../Functional/Persistence/OperatorTest.php | 2 +-
.../Persistence/QueryParserTest.php | 4 +-
10 files changed, 198 insertions(+), 11 deletions(-)
create mode 100644 typo3/sysext/extbase/Tests/Functional/Persistence/IsDirtyTest.php
diff --git a/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php b/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php
index b8b914c92dcc..fcc8b6bf3dc0 100644
--- a/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php
+++ b/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php
@@ -14,6 +14,8 @@ namespace TYPO3\CMS\Extbase\DomainObject;
* The TYPO3 project - inspiring people to share!
*/
+use TYPO3\CMS\Extbase\Persistence\Generic\LazyLoadingProxy;
+
/**
* A generic Domain Object.
*
@@ -271,8 +273,22 @@ abstract class AbstractDomainObject implements DomainObjectInterface, \TYPO3\CMS
// In case it is an object and it implements the ObjectMonitoringInterface, we call _isDirty() instead of a simple comparison of objects.
// We do this, because if the object itself contains a lazy loaded property, the comparison of the objects might fail even if the object didn't change
if (is_object($currentValue)) {
- if ($currentValue instanceof DomainObjectInterface) {
- $result = !is_object($previousValue) || get_class($previousValue) !== get_class($currentValue) || $currentValue->getUid() !== $previousValue->getUid();
+ $currentTypeString = null;
+ if ($currentValue instanceof LazyLoadingProxy) {
+ $currentTypeString = $currentValue->_getTypeAndUidString();
+ } elseif ($currentValue instanceof DomainObjectInterface) {
+ $currentTypeString = get_class($currentValue) . ':' . $currentValue->getUid();
+ }
+
+ if ($currentTypeString !== null) {
+ $previousTypeString = null;
+ if ($previousValue instanceof LazyLoadingProxy) {
+ $previousTypeString = $previousValue->_getTypeAndUidString();
+ } elseif ($previousValue instanceof DomainObjectInterface) {
+ $previousTypeString = get_class($previousValue) . ':' . $previousValue->getUid();
+ }
+
+ $result = $currentTypeString !== $previousTypeString;
} elseif ($currentValue instanceof \TYPO3\CMS\Extbase\Persistence\ObjectMonitoringInterface) {
$result = !is_object($previousValue) || $currentValue->_isDirty() || get_class($previousValue) !== get_class($currentValue);
} else {
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php b/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php
index b4001c1ea1e2..1eba811f8d6b 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php
@@ -90,6 +90,15 @@ class LazyLoadingProxy implements \Iterator, \TYPO3\CMS\Extbase\Persistence\Gene
return $this->parentObject->_getProperty($this->propertyName);
}
+ /**
+ * @return string
+ */
+ public function _getTypeAndUidString()
+ {
+ $type = $this->dataMapper->getType(get_class($this->parentObject), $this->propertyName);
+ return $type . ':' . $this->fieldValue;
+ }
+
/**
* Magic method call implementation.
*
diff --git a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/Overrides/fe_users.php b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/Overrides/fe_users.php
index 48a040345990..91ba2c3a3f3a 100644
--- a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/Overrides/fe_users.php
+++ b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/Overrides/fe_users.php
@@ -2,6 +2,6 @@
defined('TYPO3_MODE') or die();
if (is_array($GLOBALS['TCA']['fe_users']['columns']['tx_extbase_type'])) {
- $GLOBALS['TCA']['fe_users']['types']['Tx_BlogExample_Domain_Model_Administrator'] = $GLOBALS['TCA']['fe_users']['types']['0'];
- $GLOBALS['TCA']['fe_users']['columns']['tx_extbase_type']['config']['items'][] = ['LLL:EXT:blog_example/Resources/Private/Language/locallang_db.xml:fe_users.tx_extbase_type.Tx_BlogExample_Domain_Model_Administrator', 'Tx_BlogExample_Domain_Model_Administrator'];
+ $GLOBALS['TCA']['fe_users']['types']['ExtbaseTeam\BlogExample\Domain\Model\Administrator'] = $GLOBALS['TCA']['fe_users']['types']['0'];
+ $GLOBALS['TCA']['fe_users']['columns']['tx_extbase_type']['config']['items'][] = ['LLL:EXT:blog_example/Resources/Private/Language/locallang_db.xml:fe_users.tx_extbase_type.ExtbaseTeam\BlogExample\Domain\Model\Administrator', 'Tx_BlogExample_Domain_Model_Administrator'];
}
diff --git a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_blog.php b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_blog.php
index 716f43ad7bcb..89907f1abe32 100644
--- a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_blog.php
+++ b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_blog.php
@@ -161,7 +161,7 @@ return [
'type' => 'select',
'renderType' => 'selectSingle',
'foreign_table' => 'fe_users',
- 'foreign_table_where' => "AND fe_users.tx_extbase_type='Tx_BlogExample_Domain_Model_Administrator'",
+ 'foreign_table_where' => "AND fe_users.tx_extbase_type='ExtbaseTeam\BlogExample\Domain\Model\Administrator'",
'items' => [
['--none--', 0],
],
diff --git a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Resources/Private/Language/locallang_db.xml b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Resources/Private/Language/locallang_db.xml
index 50a05731ee4e..3d743576ea55 100644
--- a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Resources/Private/Language/locallang_db.xml
+++ b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Resources/Private/Language/locallang_db.xml
@@ -41,7 +41,7 @@
<label index="tx_blogexample_domain_model_tag.name">Name</label>
<label index="tx_blogexample_domain_model_tag.posts">Related posts</label>
<label index="tx_blogexample_domain_model_dateexample">Date example</label>
- <label index="fe_users.tx_extbase_type.Tx_BlogExample_Domain_Model_Administrator">Blog Admin (BlogExample)</label>
+ <label index="fe_users.tx_extbase_type.ExtbaseTeam\BlogExample\Domain\Model\Administrator">Blog Admin (BlogExample)</label>
</languageKey>
</data>
</T3locallang>
diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.xml b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.xml
index bf51895ffff8..bdf9e6128336 100644
--- a/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.xml
+++ b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.xml
@@ -19,6 +19,7 @@
<l18n_diffsource></l18n_diffsource>
<deleted>0</deleted>
<posts>1</posts>
+ <administrator>4</administrator>
</tx_blogexample_domain_model_blog>
<tx_blogexample_domain_model_blog>
<uid>3</uid>
@@ -29,6 +30,7 @@
<l18n_diffsource></l18n_diffsource>
<deleted>0</deleted>
<posts>1</posts>
+ <administrator>3</administrator>
</tx_blogexample_domain_model_blog>
<tx_blogexample_domain_model_blog>
<uid>4</uid>
diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/fe_users.xml b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/fe_users.xml
index 4b067413003c..e45808e08c53 100644
--- a/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/fe_users.xml
+++ b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/fe_users.xml
@@ -19,10 +19,19 @@
<fe_users>
<uid>3</uid>
<pid>0</pid>
- <username>administrator</username>
+ <username>administratora</username>
<deleted>0</deleted>
<usergroup>1,2</usergroup>
<disable>0</disable>
- <tx_extbase_type>Tx_BlogExample_Domain_Model_Administrator</tx_extbase_type>
+ <tx_extbase_type>ExtbaseTeam\BlogExample\Domain\Model\Administrator</tx_extbase_type>
+ </fe_users>
+ <fe_users>
+ <uid>4</uid>
+ <pid>0</pid>
+ <username>administratorb</username>
+ <deleted>0</deleted>
+ <usergroup>1,2</usergroup>
+ <disable>0</disable>
+ <tx_extbase_type>ExtbaseTeam\BlogExample\Domain\Model\Administrator</tx_extbase_type>
</fe_users>
</dataset>
diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/IsDirtyTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/IsDirtyTest.php
new file mode 100644
index 000000000000..96a86231e0d5
--- /dev/null
+++ b/typo3/sysext/extbase/Tests/Functional/Persistence/IsDirtyTest.php
@@ -0,0 +1,151 @@
+<?php
+
+namespace TYPO3\CMS\Extbase\Tests\Functional\Persistence;
+
+/*
+ * 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 ExtbaseTeam\BlogExample\Domain\Model\Administrator;
+use ExtbaseTeam\BlogExample\Domain\Repository\AdministratorRepository;
+use ExtbaseTeam\BlogExample\Domain\Repository\BlogRepository;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Extbase\Object\ObjectManager;
+use TYPO3\CMS\Extbase\Persistence\Generic\LazyLoadingProxy;
+use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
+
+class IsDirtyTest extends FunctionalTestCase
+{
+
+ /**
+ * @var array
+ */
+ protected $testExtensionsToLoad = ['typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example'];
+
+ /**
+ * @var array
+ */
+ protected $coreExtensionsToLoad = ['extbase', 'fluid'];
+
+ /**
+ * @var \TYPO3\CMS\Extbase\Object\ObjectManagerInterface The object manager
+ */
+ protected $objectManager;
+
+ /**
+ * @var \ExtbaseTeam\BlogExample\Domain\Repository\BlogRepository
+ */
+ protected $blogRepository;
+
+ /**
+ * @var \ExtbaseTeam\BlogExample\Domain\Repository\AdministratorRepository
+ */
+ protected $adminRepository;
+
+ /**
+ * Sets up this test suite.
+ */
+ protected function setUp()
+ {
+ parent::setUp();
+
+ $this->importDataSet('PACKAGE:typo3/testing-framework/Resources/Core/Functional/Fixtures/pages.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/posts.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/post-post-mm.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/tags.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/tags-mm.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/post-tag-mm.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/persons.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/fe_users.xml');
+ $this->importDataSet(ORIGINAL_ROOT . 'typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/fe_groups.xml');
+
+ $this->objectManager = GeneralUtility::makeInstance(ObjectManager::class);
+ $this->blogRepository = $this->objectManager->get(BlogRepository::class);
+ $this->adminRepository = $this->objectManager->get(AdministratorRepository::class);
+ }
+
+ /**
+ * @test
+ */
+ public function objectFetchedFromDbIsNotDirty()
+ {
+ $blog = $this->blogRepository->findByUid(3);
+ $this->assertFalse($blog->_isDirty());
+ }
+
+ /**
+ * @test
+ */
+ public function lazyLoadingProxyReplacedByRealInstanceIsNotDirty()
+ {
+ $blog = $this->blogRepository->findByUid(3);
+ $this->assertInstanceOf(LazyLoadingProxy::class, $blog->getAdministrator()); // precondition
+
+ $admin = $this->adminRepository->findByUid(3);
+ $this->assertInstanceOf(Administrator::class, $admin); // precondition
+
+ $blog->setAdministrator($admin);
+ $this->assertFalse($blog->_isDirty());
+ }
+
+ /**
+ * @test
+ */
+ public function lazyLoadingProxyReplacedByWrongInstanceIsDirty()
+ {
+ $blog = $this->blogRepository->findByUid(3);
+ $this->assertInstanceOf(LazyLoadingProxy::class, $blog->getAdministrator()); //precondition
+
+ $blog->setAdministrator(new Administrator());
+ $this->assertTrue($blog->_isDirty());
+ }
+
+ /**
+ * @test
+ */
+ public function realInstanceReplacedByLazyLoadingProxyIsNotDirty()
+ {
+ $blog = $this->blogRepository->findByUid(3);
+ $lazyLoadingProxy = $blog->getAdministrator();
+ $this->assertInstanceOf(LazyLoadingProxy::class, $lazyLoadingProxy); //precondition
+
+ $admin = $this->adminRepository->findByUid(3);
+ $this->assertInstanceOf(Administrator::class, $admin); // precondition
+
+ $blog->setAdministrator($admin);
+ $blog->_memorizeCleanState();
+
+ $blog->_setProperty('administrator', $lazyLoadingProxy);
+ $this->assertFalse($blog->_isDirty());
+ }
+
+ /**
+ * @test
+ */
+ public function lazyLoadingProxyByWrongLazyLoadingProxyIsDirtyAndUpdated()
+ {
+ $blogOne = $this->blogRepository->findByUid(3);
+ $this->assertInstanceOf(LazyLoadingProxy::class, $blogOne->getAdministrator()); //precondition
+
+ $blogTwo = $this->blogRepository->findByUid(2);
+ $this->assertInstanceOf(LazyLoadingProxy::class, $blogTwo->getAdministrator()); //precondition
+
+ $blogOne->_setProperty('administrator', $blogTwo->getAdministrator());
+ $this->assertTrue($blogOne->_isDirty());
+
+ $this->blogRepository->update($blogOne);
+
+ $updatedBlogOne = $this->blogRepository->findByUid(3);
+ $this->assertSame($updatedBlogOne->getAdministrator()->getUid(), $blogTwo->getAdministrator()->getUid());
+ }
+}
diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/OperatorTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/OperatorTest.php
index ada80bf5953c..5ca52b75b481 100644
--- a/typo3/sysext/extbase/Tests/Functional/Persistence/OperatorTest.php
+++ b/typo3/sysext/extbase/Tests/Functional/Persistence/OperatorTest.php
@@ -79,7 +79,7 @@ class OperatorTest extends \TYPO3\TestingFramework\Core\Functional\FunctionalTes
/**
* @test
*/
- public function equalsCorrectlyHandlesCaseSensivity()
+ public function equalsCorrectlyHandlesCaseSensitivity()
{
$query = $this->postRepository->createQuery();
diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php
index 0474babb9d61..3eb147a8a1f9 100644
--- a/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php
+++ b/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php
@@ -116,7 +116,7 @@ class QueryParserTest extends \TYPO3\TestingFramework\Core\Functional\Functional
);
$result = $query->execute()->toArray();
- $this->assertCount(2, $result);
+ $this->assertCount(3, $result);
}
/**
@@ -185,7 +185,7 @@ class QueryParserTest extends \TYPO3\TestingFramework\Core\Functional\Functional
$result = $query->matching($query->contains('usergroup', 1))
->execute();
- $this->assertCount(2, $result);
+ $this->assertCount(3, $result);
}
/**
--
GitLab