Use onFlush rather than prePersist / postUpdate to comply with Gedmo …

…Sluggable.
umanit · Feb 14, 2019 · 5f9e46b · 5f9e46b
1 parent 05d6e8d
commit 5f9e46b
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 62 deletions.
diff --git a/src/Doctrine/EventSubscriber/UrlHistoryWriter.php b/src/Doctrine/EventSubscriber/UrlHistoryWriter.php
@@ -3,6 +3,7 @@
 namespace Umanit\SeoBundle\Doctrine\EventSubscriber;
 
 use Doctrine\ORM\Event\LoadClassMetadataEventArgs;
+use Doctrine\ORM\Event\OnFlushEventArgs;
 use Doctrine\ORM\Event\PostFlushEventArgs;
 use Psr\SimpleCache\CacheInterface;
 use Ramsey\Uuid\Uuid;
@@ -70,9 +71,8 @@ public function getSubscribedEvents()
     {
         return [
             Events::preUpdate,
+            Events::onFlush,
             Events::prePersist,
-            Events::postUpdate,
-            Events::postRemove,
             Events::postFlush,
             Events::loadClassMetadata,
         ];
@@ -213,93 +213,115 @@ function (array &$changedFieldValue, string $changedFieldKey) use ($changeSet, $
     }
 
     /**
-     * Creates the url reference of an entity.
+     * On prePersist, associate a fresh UrlRef to an entity.
      *
      * @param LifecycleEventArgs $args
      *
      * @throws \ReflectionException
      */
-    public function prePersist(LifecycleEventArgs $args): void
+    public function prePersist(LifecycleEventArgs $args)
     {
         $entity = $args->getEntity();
         if (!$entity instanceof UrlHistorizedInterface) {
             return;
         }
-
-        if ($entity->getUrlRef() !== null) {
-            return;
-        }
         try {
-            $route  = $this->getSeoRouteAnnotation($entity);
-            $urlRef = (new UrlRef())
-                ->setSeoUuid(Uuid::uuid4())
-                ->setLocale(method_exists($entity, 'getLocale') ? $entity->getLocale() : $this->defaultLocale)
-                ->setUrl($this->urlBuilder->url($entity))
-                ->setRoute($route->getRouteName())
-            ;
-
-            $entity->setUrlRef($urlRef);
+            if (null === $entity->getUrlRef()) {
+                $route  = $this->getSeoRouteAnnotation($entity);
+                $urlRef = (new UrlRef())
+                    ->setSeoUuid(Uuid::uuid4())
+                    ->setLocale(method_exists($entity, 'getLocale') ? $entity->getLocale() : $this->defaultLocale)
+                    ->setRoute($route->getRouteName())
+                ;
+                $entity->setUrlRef($urlRef);
+            }
         } catch (NotSeoRouteEntityException $e) {
             // Do nothing
         }
     }
 
     /**
-     * Updates the url reference of an entity.
-     * Updates the child entities when a parent entity slug is updated.
+     * We use on flush rather than prePersist
+     * and postUpdate so the history works
+     * with DoctrineExtensions Sluggable.
      *
-     * @param LifecycleEventArgs $args
+     * @param OnFlushEventArgs $args
      *
-     * @throws NotSeoRouteEntityException
+     * @return void
      */
-    public function postUpdate(LifecycleEventArgs $args): void
+    public function onFlush(OnFlushEventArgs $args)
     {
-        $entity = $args->getEntity();
-
-        if ($entity instanceof UrlHistorizedInterface) {
+        $em  = $args->getEntityManager();
+        $uow = $em->getUnitOfWork();
+
+        // process all objects being inserted, using scheduled insertions instead
+        // of prePersist in case if record will be changed before flushing this will
+        // ensure correct result. No additional overhead is encountered
+        foreach ($uow->getScheduledEntityInsertions() as $entity) {
+            if (!$entity instanceof UrlHistorizedInterface) {
+                continue;
+            }
             try {
-                /** @var UrlRef $urlRef */
-                $urlRef = $entity->getUrlRef();
-                if (null === $urlRef) {
-                    return;
-                }
-                $newUrl = $this->urlBuilder->url($entity);
-                if ($urlRef->getUrl() !== $newUrl) {
-                    $urlRef->setUrl($newUrl);
-                    $args->getEntityManager()->persist($urlRef);
-                    $args->getEntityManager()->flush($urlRef);
-                }
+                $urlRef = $entity
+                    ->getUrlRef()
+                    ->setUrl($this->urlBuilder->url($entity))
+                ;
+                $uow->recomputeSingleEntityChangeSet($em->getClassMetadata($this->getClass($urlRef)), $urlRef);
+                $uow->persist($urlRef);
+
             } catch (NotSeoRouteEntityException $e) {
                 // Do nothing
             }
         }
+        // we use onFlush and not preUpdate event to let other
+        // event listeners be nested together
+        foreach ($uow->getScheduledEntityUpdates() as $entity) {
+            if ($entity instanceof UrlHistorizedInterface) {
+                try {
+                    /** @var UrlRef $urlRef */
+                    $urlRef = $entity->getUrlRef();
+                    if (null === $urlRef) {
+                        continue;
+                    }
+                    $newUrl = $this->urlBuilder->url($entity);
+                    if ($urlRef->getUrl() !== $newUrl) {
+                        $urlRef->setUrl($newUrl);
+                        $uow->recomputeSingleEntityChangeSet($em->getClassMetadata($this->getClass($urlRef)), $urlRef);
+                        $uow->persist($urlRef);
+                    }
+                } catch (NotSeoRouteEntityException $e) {
+                    // Do nothing
+                }
+            }
 
-        // Look inside the cache if any dependent entity has to be historised
-        $cache = $this->cache->get(self::ENTITY_DEPENDENCY_CACHE_KEY, []);
-        if (!array_key_exists($this->getClass($entity), $cache)) {
-            return;
-        }
+            // Look inside the cache if any dependent entity has to be historised
+            $cache = $this->cache->get(self::ENTITY_DEPENDENCY_CACHE_KEY, []);
+            if (!array_key_exists($this->getClass($entity), $cache)) {
+                continue;
+            }
 
-        foreach ($cache[$this->getClass($entity)] as $dependantEntityClass) {
-            // Fetches the current url of the entities
-            $query = $args
-                ->getEntityManager()->createQueryBuilder()
-                ->select('dependant', 'url_ref')
-                ->from($dependantEntityClass, 'dependant')
-                ->innerJoin('dependant.urlRef', 'url_ref')
-            ;
-            // Regenerate route for all entities if different
-            foreach ($query->getQuery()->getResult() as $dependantEntity) {
-                $urlRef = $dependantEntity->getUrlRef() ?? new UrlRef();
-
-                $newUrl = $this->urlBuilder->url($dependantEntity);
-                $oldUrl = $urlRef->getUrl();
-
-                if ($newUrl !== $oldUrl) {
-                    // Add the redirection to the pool
-                    $this->urlPool->add($oldUrl, $newUrl, $dependantEntity);
-                    $urlRef->setUrl($newUrl);
-                    $args->getEntityManager()->persist($urlRef);
+            foreach ($cache[$this->getClass($entity)] as $dependantEntityClass) {
+                // Fetches the current url of the entities
+                $query = $args
+                    ->getEntityManager()->createQueryBuilder()
+                    ->select('dependant', 'url_ref')
+                    ->from($dependantEntityClass, 'dependant')
+                    ->innerJoin('dependant.urlRef', 'url_ref')
+                ;
+                // Regenerate route for all entities if different
+                foreach ($query->getQuery()->getResult() as $dependantEntity) {
+                    $urlRef = $dependantEntity->getUrlRef();
+
+                    $newUrl = $this->urlBuilder->url($dependantEntity);
+                    $oldUrl = $urlRef->getUrl();
+
+                    if ($newUrl !== $oldUrl) {
+                        // Add the redirection to the pool
+                        $this->urlPool->add($oldUrl, $newUrl, $dependantEntity);
+                        $urlRef->setUrl($newUrl);
+                        $uow->recomputeSingleEntityChangeSet($em->getClassMetadata($this->getClass($urlRef)), $urlRef);
+                        $uow->persist($urlRef);
+                    }
                 }
             }
         }

diff --git a/src/Doctrine/Model/UrlHistorizedTrait.php b/src/Doctrine/Model/UrlHistorizedTrait.php
@@ -13,6 +13,7 @@ trait UrlHistorizedTrait
     /**
      * @var UrlRef
      * @ORM\OneToOne(targetEntity="Umanit\SeoBundle\Entity\UrlRef", cascade={"all"}, orphanRemoval=true)
+     * @ORM\JoinColumn(name="seo_url_reference", nullable=false)
      */
     protected $urlRef;
 

diff --git a/src/Form/Type/SeoMetadataType.php b/src/Form/Type/SeoMetadataType.php
@@ -152,7 +152,7 @@ protected function setSubFormOption(FormInterface $parentForm, string $childName
     public function finishView(FormView $view, FormInterface $form, array $options)
     {
         $entity = $form->getParent()->getData();
-        if ($entity instanceof UrlHistorizedInterface) {
+        if ($entity instanceof UrlHistorizedInterface && $entity->getUrlRef()) {
             $view->vars['url_history'] = $this->em->getRepository(UrlHistory::class)->findBy(['seoUuid' => $entity->getUrlRef()->getSeoUuid()], ['id' => 'ASC']);
         }
     }