From 401135852c6b25c8d5ab97beaefb02d1bd023ec9 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Sun, 25 Sep 2016 11:26:15 +0200 Subject: [PATCH] Use scheduled entity insertions to avoid tag duplicate Using `getScheduledEntityInsertions()` we can retrieve not yet flushed but already persisted entities and then avoid tags duplication on import. --- .../CoreBundle/Helper/ContentProxy.php | 25 +++++++++++++++---- .../ImportBundle/Import/PocketImport.php | 3 ++- .../ImportBundle/Import/WallabagImport.php | 3 ++- .../CoreBundle/Helper/ContentProxyTest.php | 23 +++++++++++++++++ .../ImportBundle/Import/PocketImportTest.php | 14 +++++++++++ .../Import/WallabagV1ImportTest.php | 14 +++++++++++ .../Import/WallabagV2ImportTest.php | 14 +++++++++++ 7 files changed, 89 insertions(+), 7 deletions(-) diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index 5dd684f2..a65a21e8 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -96,13 +96,24 @@ class ContentProxy * * @param Entry $entry * @param array|string $tags An array of tag or a string coma separated of tag + * @param array $entitiesReady Entities from the EntityManager which are persisted but not yet flushed + * It is mostly to fix duplicate tag on import + * @see http://stackoverflow.com/a/7879164/569101 */ - public function assignTagsToEntry(Entry $entry, $tags) + public function assignTagsToEntry(Entry $entry, $tags, array $entitiesReady = []) { if (!is_array($tags)) { $tags = explode(',', $tags); } + // keeps only Tag entity from the "not yet flushed entities" + $tagsNotYetFlushed = []; + foreach ($entitiesReady as $entity) { + if ($entity instanceof Tag) { + $tagsNotYetFlushed[$entity->getLabel()] = $entity; + } + } + foreach ($tags as $label) { $label = trim($label); @@ -111,11 +122,15 @@ class ContentProxy continue; } - $tagEntity = $this->tagRepository->findOneByLabel($label); + if (isset($tagsNotYetFlushed[$label])) { + $tagEntity = $tagsNotYetFlushed[$label]; + } else { + $tagEntity = $this->tagRepository->findOneByLabel($label); - if (is_null($tagEntity)) { - $tagEntity = new Tag(); - $tagEntity->setLabel($label); + if (is_null($tagEntity)) { + $tagEntity = new Tag(); + $tagEntity->setLabel($label); + } } // only add the tag on the entry if the relation doesn't exist diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php index e00eb44b..327e2500 100644 --- a/src/Wallabag/ImportBundle/Import/PocketImport.php +++ b/src/Wallabag/ImportBundle/Import/PocketImport.php @@ -227,7 +227,8 @@ class PocketImport extends AbstractImport if (isset($importedEntry['tags']) && !empty($importedEntry['tags'])) { $this->contentProxy->assignTagsToEntry( $entry, - array_keys($importedEntry['tags']) + array_keys($importedEntry['tags']), + $this->em->getUnitOfWork()->getScheduledEntityInsertions() ); } diff --git a/src/Wallabag/ImportBundle/Import/WallabagImport.php b/src/Wallabag/ImportBundle/Import/WallabagImport.php index 043bb0a2..3754e4a9 100644 --- a/src/Wallabag/ImportBundle/Import/WallabagImport.php +++ b/src/Wallabag/ImportBundle/Import/WallabagImport.php @@ -111,7 +111,8 @@ abstract class WallabagImport extends AbstractImport if (array_key_exists('tags', $data)) { $this->contentProxy->assignTagsToEntry( $entry, - $data['tags'] + $data['tags'], + $this->em->getUnitOfWork()->getScheduledEntityInsertions() ); } diff --git a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php index 7abb0737..5d772602 100644 --- a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php +++ b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php @@ -296,6 +296,29 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase $this->assertEquals('tag2', $entry->getTags()[1]->getLabel()); } + public function testAssignTagsNotFlushed() + { + $graby = $this->getMockBuilder('Graby\Graby') + ->disableOriginalConstructor() + ->getMock(); + + $tagRepo = $this->getTagRepositoryMock(); + $tagRepo->expects($this->never()) + ->method('__call'); + + $proxy = new ContentProxy($graby, $this->getTaggerMock(), $tagRepo, $this->getLogger()); + + $tagEntity = new Tag(); + $tagEntity->setLabel('tag1'); + + $entry = new Entry(new User()); + + $proxy->assignTagsToEntry($entry, 'tag1', [$tagEntity]); + + $this->assertCount(1, $entry->getTags()); + $this->assertEquals('tag1', $entry->getTags()[0]->getLabel()); + } + private function getTaggerMock() { return $this->getMockBuilder('Wallabag\CoreBundle\Helper\RuleBasedTagger') diff --git a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php index 952521a2..9ec7935c 100644 --- a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php +++ b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php @@ -41,6 +41,20 @@ class PocketImportTest extends \PHPUnit_Framework_TestCase ->disableOriginalConstructor() ->getMock(); + $this->uow = $this->getMockBuilder('Doctrine\ORM\UnitOfWork') + ->disableOriginalConstructor() + ->getMock(); + + $this->em + ->expects($this->any()) + ->method('getUnitOfWork') + ->willReturn($this->uow); + + $this->uow + ->expects($this->any()) + ->method('getScheduledEntityInsertions') + ->willReturn([]); + $pocket = new PocketImport( $this->em, $this->contentProxy diff --git a/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php b/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php index 5ab4ad00..82dc4c7e 100644 --- a/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php +++ b/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php @@ -26,6 +26,20 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase ->disableOriginalConstructor() ->getMock(); + $this->uow = $this->getMockBuilder('Doctrine\ORM\UnitOfWork') + ->disableOriginalConstructor() + ->getMock(); + + $this->em + ->expects($this->any()) + ->method('getUnitOfWork') + ->willReturn($this->uow); + + $this->uow + ->expects($this->any()) + ->method('getScheduledEntityInsertions') + ->willReturn([]); + $this->contentProxy = $this->getMockBuilder('Wallabag\CoreBundle\Helper\ContentProxy') ->disableOriginalConstructor() ->getMock(); diff --git a/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php b/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php index 12bd6bdd..bea89efb 100644 --- a/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php +++ b/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php @@ -26,6 +26,20 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase ->disableOriginalConstructor() ->getMock(); + $this->uow = $this->getMockBuilder('Doctrine\ORM\UnitOfWork') + ->disableOriginalConstructor() + ->getMock(); + + $this->em + ->expects($this->any()) + ->method('getUnitOfWork') + ->willReturn($this->uow); + + $this->uow + ->expects($this->any()) + ->method('getScheduledEntityInsertions') + ->willReturn([]); + $this->contentProxy = $this->getMockBuilder('Wallabag\CoreBundle\Helper\ContentProxy') ->disableOriginalConstructor() ->getMock(); -- 2.41.0