Fix bug create tag with same name multiple times (#180)

Author: alexander-schranz

Content/Infrastructure/Doctrine/TagFactory.php

@@ -52,22 +52,41 @@ public function create(array $tagNames): array
         // sort tags by the given names order
         $excerptTags = [];
         foreach ($tags as $tag) {
-            $excerptTags[array_search($tag->getName(), $tagNames, true)] = $tag;
+            $index = array_search($tag->getName(), $tagNames, true);
+            $excerptTags[$index] = $tag;
+            unset($tagNames[$index]);
         }
 
-        // create tags which not exist yet
-        foreach ($tagNames as $key => $tagName) {
-            if (isset($excerptTags[$key])) {
-                continue;
+        // check if a tag with the same name was yet persisted and use that instead of create one
+        // this avoids a unique constraint error to create multiple tag with same name
+        if (\count($tagNames)) {
+            // we use here the unitOfWork instead of an own cache this avoid us listing for
+            // flush, clear or deletion events and so we don't need to invalid an cache ourselves
+            foreach ($this->entityManager->getUnitOfWork()->getScheduledEntityInsertions() as $object) {
+                if (!$object instanceof TagInterface) {
+                    continue;
+                }
+
+                $index = array_search($object->getName(), $tagNames, true);
+
+                if (false === $index) {
+                    continue;
+                }
+
+                $excerptTags[$index] = $object;
+                unset($tagNames[$index]);
             }
+        }
 
+        // create missing tags which not exist yet
+        foreach ($tagNames as $index => $tagName) {
             /** @var TagInterface $tag */
             $tag = $this->tagRepository->createNew();
             $tag->setName($tagName);
 
             $this->entityManager->persist($tag);
 
-            $excerptTags[$key] = $tag;
+            $excerptTags[$index] = $tag;
         }
 
         return $excerptTags;

Tests/Functional/Content/Infrastructure/Doctrine/TagFactoryTest.php

@@ -13,39 +13,25 @@
 
 namespace Sulu\Bundle\ContentBundle\Tests\Functional\Content\Infrastructure\Doctrine;
 
+use Sulu\Bundle\ContactBundle\Entity\Contact;
 use Sulu\Bundle\ContentBundle\Content\Domain\Factory\TagFactoryInterface;
 use Sulu\Bundle\TagBundle\Tag\TagInterface;
 use Sulu\Bundle\TagBundle\Tag\TagRepositoryInterface;
 use Sulu\Bundle\TestBundle\Testing\SuluTestCase;
 
 class TagFactoryTest extends SuluTestCase
 {
+    /**
+     * @var TagFactoryInterface
+     */
+    private $tagFactory;
+
     protected function setUp(): void
     {
         self::bootKernel();
         self::purgeDatabase();
-    }
-
-    /**
-     * @param string[] $existTagNames
-     */
-    protected function createTagFactory(array $existTagNames = []): TagFactoryInterface
-    {
-        /** @var TagRepositoryInterface $tagRepository */
-        $tagRepository = self::$container->get('sulu.repository.tag');
-
-        foreach ($existTagNames as $existTagName) {
-            $existTag = $tagRepository->createNew();
-            $existTag->setName($existTagName);
-            self::getEntityManager()->persist($existTag);
-        }
-
-        if (\count($existTagNames)) {
-            self::getEntityManager()->flush();
-            self::getEntityManager()->clear();
-        }
 
-        return self::$container->get('sulu_content.tag_factory');
+        $this->tagFactory = self::$container->get('sulu_content.tag_factory');
     }
 
     /**
@@ -56,19 +42,49 @@ protected function createTagFactory(array $existTagNames = []): TagFactoryInterf
      */
     public function testCreate(array $tagNames, array $existTags): void
     {
-        $tagFactory = $this->createTagFactory($existTags);
+        $this->createTags($existTags);
+
+        $tags = $this->tagFactory->create($tagNames);
 
         $this->assertSame(
             $tagNames,
             array_map(
                 function (TagInterface $tag) {
                     return $tag->getName();
                 },
-                $tagFactory->create($tagNames)
+                $tags
             )
         );
     }
 
+    public function testCreateSameTagTwice(): void
+    {
+        $tags1 = $this->tagFactory->create(['Tag 1']);
+        $tags2 = $this->tagFactory->create(['Tag 1']);
+
+        $this->assertSame($tags1, $tags2);
+
+        $this->getEntityManager()->flush();
+    }
+
+    public function testCreateSameTagTwiceWithOtherEntityInUnitOfWork(): void
+    {
+        $this->getEntityManager()->persist($this->createOtherEntity());
+
+        /** @var TagRepositoryInterface $tagRepository */
+        $tagRepository = self::$container->get('sulu.repository.tag');
+        $tag = $tagRepository->createNew();
+        $tag->setName('Other Tag');
+        $this->getEntityManager()->persist($tag);
+
+        $tags1 = $this->tagFactory->create(['Tag 1']);
+        $tags2 = $this->tagFactory->create(['Tag 1']);
+
+        $this->assertSame($tags1, $tags2);
+
+        $this->getEntityManager()->flush();
+    }
+
     /**
      * @return \Generator<mixed[]>
      */
@@ -100,6 +116,7 @@ public function dataProvider(): \Generator
             ],
             [
                 'Exist Tag 1',
+                'Other Exist 3',
             ],
         ];
 
@@ -114,4 +131,33 @@ public function dataProvider(): \Generator
             ],
         ];
     }
+
+    /**
+     * @param string[] $existTagNames
+     */
+    private function createTags(array $existTagNames = []): void
+    {
+        /** @var TagRepositoryInterface $tagRepository */
+        $tagRepository = self::$container->get('sulu.repository.tag');
+
+        foreach ($existTagNames as $existTagName) {
+            $existTag = $tagRepository->createNew();
+            $existTag->setName($existTagName);
+            self::getEntityManager()->persist($existTag);
+        }
+
+        if (\count($existTagNames)) {
+            self::getEntityManager()->flush();
+            self::getEntityManager()->clear();
+        }
+    }
+
+    private function createOtherEntity(): object
+    {
+        $contact = new Contact();
+        $contact->setFirstName('Dummy');
+        $contact->setLastName('Entity');
+
+        return $contact;
+    }
 }

composer.json

@@ -78,7 +78,8 @@
             "Tests/Application/bin/adminconsole sulu:build dev --env dev"
         ],
         "bootstrap-test-environment": [
-            "Tests/Application/bin/adminconsole doctrine:database:create --if-not-exists --env test",
+            "Tests/Application/bin/adminconsole doctrine:database:drop --if-exists --force --env test",
+            "Tests/Application/bin/adminconsole doctrine:database:create --env test",
             "Tests/Application/bin/adminconsole doctrine:schema:update --force --env test"
         ],
         "lint": [