From 7fe68c8d50f327d7f0c3bd7f648e52ebfb7d0753 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 1 Sep 2014 21:45:40 +0200 Subject: [PATCH 1/3] test for changeset calculation with translated documents --- .../Functional/ChangesetCalculationTest.php | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php new file mode 100644 index 000000000..a29d6819c --- /dev/null +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php @@ -0,0 +1,148 @@ +listener = new ChangesetListener(); + $this->dm = $this->createDocumentManager(); + $this->dm->setLocaleChooserStrategy(new LocaleChooser(array('en' => array('fr'), 'fr' => array('en')), 'en')); + $this->node = $this->resetFunctionalNode($this->dm); + } + + public function testComputeChangeset() + { + $this->dm + ->getEventManager() + ->addEventListener( + array( + Event::postUpdate, + ), + $this->listener + ); + + // Create initial user + $user1 = new \Doctrine\Tests\Models\CMS\CmsUser(); + $user1->name = 'david'; + $user1->username = 'dbu'; + $user1->status = 'active'; + $this->dm->persist($user1); + + // Create additional user + $user2 = new \Doctrine\Tests\Models\CMS\CmsUser(); + $user2->name = 'lukas'; + $user2->username = 'lsmith'; + $user2->status = 'active'; + $this->dm->persist($user2); + + $this->dm->flush(); + $this->assertEquals(0, $this->listener->count); + $this->dm->clear(); + + $user1 = $this->dm->find(null, $user1->id); + $this->dm->find(null, $user2->id); + + $user1->status = 'changed'; + $this->dm->flush(); + + $this->assertEquals(1, $this->listener->count); + } + + public function testComputeChangesetTranslatable() + { + $this->dm + ->getEventManager() + ->addEventListener( + array( + Event::postUpdate, + ), + $this->listener + ); + + // Create initial user + $user1 = new \Doctrine\Tests\Models\CMS\CmsUserTranslatable(); + $user1->name = 'david'; + $user1->username = 'dbu'; + $user1->status = 'active'; + $this->dm->persist($user1); + + // Create additional user + $user2 = new \Doctrine\Tests\Models\CMS\CmsUserTranslatable(); + $user2->name = 'lukas'; + $user2->username = 'lsmith'; + $user2->status = 'active'; + $this->dm->persist($user2); + + $this->dm->flush(); + $this->assertEquals(0, $this->listener->count); + $this->dm->clear(); + + $user1 = $this->dm->find(null, $user1->id); + $this->dm->find(null, $user2->id); + + $user1->status = 'changed'; + $this->dm->flush(); + + $this->assertEquals(1, $this->listener->count); + } + + public function testComputeChangesetTranslatableFind() + { + $this->dm + ->getEventManager() + ->addEventListener( + array( + Event::postUpdate, + ), + $this->listener + ); + + // Create initial user + $user1 = new \Doctrine\Tests\Models\CMS\CmsUserTranslatable(); + $user1->name = 'david'; + $user1->username = 'dbu'; + $user1->status = 'activ'; + $this->dm->persist($user1); + $this->dm->bindTranslation($user1, 'en'); + $user1->status = 'actif'; + $this->dm->bindTranslation($user1, 'fr'); + + $this->dm->flush(); + $this->assertEquals(0, $this->listener->count); + $this->dm->clear(); + + $user1 = $this->dm->findTranslation(null, $user1->id, 'en'); + $this->dm->findTranslation(null, $user1->id, 'fr'); + + $this->dm->flush(); + + $this->assertEquals(0, $this->listener->count); + } +} + +class ChangesetListener +{ + public $count = 0; + + public function postUpdate(LifecycleEventArgs $e) + { + $this->count++; + } +} From d8839de0cc79b4fd3cfbf72ffa5f2cecfcb26f62 Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Wed, 3 Sep 2014 18:41:47 +0200 Subject: [PATCH 2/3] fix logic to determine if there were changes in translations --- lib/Doctrine/ODM/PHPCR/UnitOfWork.php | 78 ++++++++++++++++++--------- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php index c66c069d3..a415843f8 100644 --- a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php +++ b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php @@ -712,6 +712,11 @@ public function bindTranslation($document, $locale) )); } + $this->doBindTranslation($document, $locale, $class); + } + + private function doBindTranslation($document, $locale, ClassMetadata $class) + { if ($invoke = $this->eventListenersInvoker->getSubscribedSystems($class, Event::preBindTranslation)) { $this->eventListenersInvoker->invoke( $class, @@ -726,10 +731,6 @@ public function bindTranslation($document, $locale) $oid = spl_object_hash($document); - if (empty($this->documentTranslations[$oid])) { - $this->documentTranslations[$oid] = array(); - } - foreach ($class->translatableFields as $field) { $this->documentTranslations[$oid][$locale][$field] = $class->reflFields[$field]->getValue($document); } @@ -1539,24 +1540,55 @@ public function computeChangeSet(ClassMetadata $class, $document) $fields = array_intersect_key($changeSet, $class->mappings); + if ($this->isDocumentTranslatable($class)) { + $locale = $this->getCurrentLocale($document, $class); + $oid = spl_object_hash($document); + + if (empty($this->documentTranslations[$oid]) + || (isset($this->documentLocales[$oid]['current']) + && $locale !== $this->documentLocales[$oid]['current'] + && !array_key_exists($locale, $this->documentTranslations[$oid]) + ) + ) { + $this->doBindTranslation($document, $locale, $class); + } + } + if ($isNew) { $this->documentChangesets[$oid]['fields'] = $fields; $this->scheduledInserts[$oid] = $document; return; } - // when locale changes, we should keep all field changes - $keepAllChanges = isset($this->documentLocales[$oid]) && $this->documentLocales[$oid]['current'] !== $this->documentLocales[$oid]['original']; - foreach ($fields as $fieldName => $fieldValue) { - $keepChange = $keepAllChanges; - if (!$keepChange) { - if ($fieldValue instanceof ReferenceManyCollection || $fieldValue instanceof ReferrersCollection) { - if ($fieldValue->changed()) { - $keepChange = true; + $translationChanges = false; + if ($this->isDocumentTranslatable($class)) { + if ($class->localeMapping && array_key_exists($class->localeMapping, $fields)) { + unset($fields[$class->localeMapping]); + } + + $oid = spl_object_hash($document); + if (isset($this->documentTranslations[$oid])) { + // a translation was added + if (count($this->documentTranslations[$oid]) > 1) { + $translationChanges = true; + } else { + // a translation was removed + $data = reset($this->documentTranslations[$oid]); + if (empty($data)) { + $translationChanges = true; } - } elseif ($this->originalData[$oid][$fieldName] !== $fieldValue) { + } + } + } + + foreach ($fields as $fieldName => $fieldValue) { + $keepChange = false; + if ($fieldValue instanceof ReferenceManyCollection || $fieldValue instanceof ReferrersCollection) { + if ($fieldValue->changed()) { $keepChange = true; } + } elseif ($this->originalData[$oid][$fieldName] !== $fieldValue) { + $keepChange = true; } if ($keepChange) { @@ -1566,7 +1598,7 @@ public function computeChangeSet(ClassMetadata $class, $document) } } - if (!empty($fields)) { + if (!empty($fields) || $translationChanges) { $this->documentChangesets[$oid]['fields'] = $fields; $this->originalData[$oid] = $actualData; $this->scheduledUpdates[$oid] = $document; @@ -3171,19 +3203,7 @@ private function doSaveTranslation($document, NodeInterface $node, $metadata) return; } - $locale = $this->getCurrentLocale($document, $metadata); - $oid = spl_object_hash($document); - // handle case for initial persisting - if (empty($this->documentTranslations[$oid])) { - $this->bindTranslation($document, $locale); - // handle case when locale in the mapped property changed - } elseif (isset($this->documentLocales[$oid]['current']) - && $locale !== $this->documentLocales[$oid]['current'] - ) { - $this->bindTranslation($document, $locale); - } - if (!empty($this->documentTranslations[$oid])) { $strategy = $this->dm->getTranslationStrategy($metadata->translator); foreach ($this->documentTranslations[$oid] as $locale => $data) { @@ -3330,6 +3350,7 @@ public function doLoadTranslation($document, ClassMetadata $metadata, $locale = } $currentLocale = $this->getCurrentLocale($document, $metadata); + // if no locale is specified, we reset the current translation $locale = $locale ?: $currentLocale; @@ -3341,6 +3362,11 @@ public function doLoadTranslation($document, ClassMetadata $metadata, $locale = $this->setLocale($document, $metadata, $localeUsed); + $oid = spl_object_hash($document); + foreach ($metadata->translatableFields as $fieldName) { + $this->originalData[$oid][$fieldName] = $metadata->getFieldValue($document, $fieldName); + } + if ($metadata->parentMapping) { $parent = $metadata->reflFields[$metadata->parentMapping]->getValue($document); $this->cascadeDoLoadTranslation($parent, $metadata->mappings[$metadata->parentMapping], $locale); From 702a8856aec74e99cfbe7156bcabb372995311de Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Wed, 3 Sep 2014 18:42:19 +0200 Subject: [PATCH 3/3] make it possible to load multiple translations from a previously persisted documents --- lib/Doctrine/ODM/PHPCR/UnitOfWork.php | 57 +++++++++++++------ .../Functional/ChangesetCalculationTest.php | 13 +++++ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php index a415843f8..cde68cb0b 100644 --- a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php +++ b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php @@ -144,6 +144,11 @@ class UnitOfWork */ private $originalData = array(); + /** + * @var array + */ + private $originalTranslatedData = array(); + /** * @var array */ @@ -715,9 +720,9 @@ public function bindTranslation($document, $locale) $this->doBindTranslation($document, $locale, $class); } - private function doBindTranslation($document, $locale, ClassMetadata $class) + private function doBindTranslation($document, $locale, ClassMetadata $class, $supressEvents = false) { - if ($invoke = $this->eventListenersInvoker->getSubscribedSystems($class, Event::preBindTranslation)) { + if (!$supressEvents && $invoke = $this->eventListenersInvoker->getSubscribedSystems($class, Event::preBindTranslation)) { $this->eventListenersInvoker->invoke( $class, Event::preBindTranslation, @@ -735,7 +740,7 @@ private function doBindTranslation($document, $locale, ClassMetadata $class) $this->documentTranslations[$oid][$locale][$field] = $class->reflFields[$field]->getValue($document); } - if ($invoke = $this->eventListenersInvoker->getSubscribedSystems($class, Event::postBindTranslation)) { + if (!$supressEvents && $invoke = $this->eventListenersInvoker->getSubscribedSystems($class, Event::postBindTranslation)) { $this->eventListenersInvoker->invoke( $class, Event::postBindTranslation, @@ -1544,11 +1549,10 @@ public function computeChangeSet(ClassMetadata $class, $document) $locale = $this->getCurrentLocale($document, $class); $oid = spl_object_hash($document); + // check if the document has not been bound or if the locale has not yet been bound + // (note array_key_exists is necessary to handle removed translations) if (empty($this->documentTranslations[$oid]) - || (isset($this->documentLocales[$oid]['current']) - && $locale !== $this->documentLocales[$oid]['current'] - && !array_key_exists($locale, $this->documentTranslations[$oid]) - ) + || !array_key_exists($locale, $this->documentTranslations[$oid]) ) { $this->doBindTranslation($document, $locale, $class); } @@ -1562,23 +1566,33 @@ public function computeChangeSet(ClassMetadata $class, $document) $translationChanges = false; if ($this->isDocumentTranslatable($class)) { - if ($class->localeMapping && array_key_exists($class->localeMapping, $fields)) { - unset($fields[$class->localeMapping]); - } - $oid = spl_object_hash($document); if (isset($this->documentTranslations[$oid])) { - // a translation was added - if (count($this->documentTranslations[$oid]) > 1) { - $translationChanges = true; - } else { + foreach ($this->documentTranslations[$oid] as $localeToCheck => $data) { // a translation was removed - $data = reset($this->documentTranslations[$oid]); if (empty($data)) { $translationChanges = true; + break; + } + // a translation was added + if (empty($this->originalTranslatedData[$oid][$localeToCheck])) { + $translationChanges = true; + break; + } + // a translation was changed + foreach ($data as $fieldName => $fieldValue) { + if ($this->originalTranslatedData[$oid][$localeToCheck][$fieldName] !== $fieldValue) { + $translationChanges = true; + break; + } } } } + + // ensure that locale changes are not considered a change in the document + if ($class->localeMapping && array_key_exists($class->localeMapping, $fields)) { + unset($fields[$class->localeMapping]); + } } foreach ($fields as $fieldName => $fieldValue) { @@ -2999,6 +3013,7 @@ private function unregisterDocument($document) $this->scheduledReorders[$oid], $this->scheduledInserts[$oid], $this->originalData[$oid], + $this->originalTranslatedData[$oid], $this->documentIds[$oid], $this->documentState[$oid], $this->documentTranslations[$oid], @@ -3135,6 +3150,7 @@ public function clear() $this->documentLocales = $this->nonMappedData = $this->originalData = + $this->originalTranslatedData = $this->documentChangesets = $this->changesetComputed = $this->scheduledUpdates = @@ -3208,6 +3224,10 @@ private function doSaveTranslation($document, NodeInterface $node, $metadata) $strategy = $this->dm->getTranslationStrategy($metadata->translator); foreach ($this->documentTranslations[$oid] as $locale => $data) { if ($data) { + foreach ($data as $fieldName => $fieldValue) { + $this->originalTranslatedData[$oid][$locale][$fieldName] = $fieldValue; + } + $strategy->saveTranslation($data, $node, $metadata, $locale); } else { $strategy->removeTranslation($document, $node, $metadata, $locale); @@ -3360,11 +3380,12 @@ public function doLoadTranslation($document, ClassMetadata $metadata, $locale = $localeUsed = $this->doLoadDatabaseTranslation($document, $metadata, $locale, $fallback, $refresh); } - $this->setLocale($document, $metadata, $localeUsed); + $this->doBindTranslation($document, $localeUsed, $metadata, true); $oid = spl_object_hash($document); foreach ($metadata->translatableFields as $fieldName) { - $this->originalData[$oid][$fieldName] = $metadata->getFieldValue($document, $fieldName); + $this->originalData[$oid][$fieldName] = $this->originalTranslatedData[$oid][$localeUsed][$fieldName] + = $metadata->getFieldValue($document, $fieldName); } if ($metadata->parentMapping) { diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php index a29d6819c..720a558cd 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/ChangesetCalculationTest.php @@ -134,6 +134,19 @@ public function testComputeChangesetTranslatableFind() $this->dm->flush(); $this->assertEquals(0, $this->listener->count); + + $user1 = $this->dm->findTranslation(null, $user1->id, 'en'); + $user1->status = 'active'; + $this->dm->findTranslation(null, $user1->id, 'fr'); + + $this->dm->flush(); + + $this->assertEquals(1, $this->listener->count); + $this->dm->clear(); + + $user1 = $this->dm->findTranslation(null, $user1->id, 'en'); + $this->assertEquals('active', $user1->status); + } }