Fix some return types

sonata-project · Sep 8, 2021 · 9036ce4 · 9036ce4
1 parent fe7b034
commit 9036ce4
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 25 deletions.
diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md
@@ -4,6 +4,11 @@ UPGRADE 3.x
 UPGRADE FROM 3.x to 3.x
 =======================
 
+### Sonata\MediaBundle\Listener\BaseMediaEventSubscriber::getMedia() and its inheritances
+
+Method `getMedia()` returns `null` if the related medium does not implement `Sonata\MediaBundle\Model\MediaInterface`.
+Before this change, under the same situation this method was returning the invalid object.
+
 ### Sonata\MediaBundle\Filesystem\Replicate
 
 Added implementation for `Gaufrette\Adapter\FileFactory` and `Gaufrette\Adapter\StreamFactory`.

diff --git a/src/Listener/BaseMediaEventSubscriber.php b/src/Listener/BaseMediaEventSubscriber.php
@@ -42,66 +42,82 @@ public function getPool()
 
     public function postUpdate(EventArgs $args)
     {
-        if (!($provider = $this->getProvider($args))) {
+        $media = $this->getMedia($args);
+
+        if (null === $media) {
             return;
         }
 
-        $provider->postUpdate($this->getMedia($args));
+        $this->getProvider($args)->postUpdate($media);
     }
 
     public function postRemove(EventArgs $args)
     {
-        if (!($provider = $this->getProvider($args))) {
+        $media = $this->getMedia($args);
+
+        if (null === $media) {
             return;
         }
 
-        $provider->postRemove($this->getMedia($args));
+        $this->getProvider($args)->postRemove($media);
     }
 
     public function postPersist(EventArgs $args)
     {
-        if (!($provider = $this->getProvider($args))) {
+        $media = $this->getMedia($args);
+
+        if (null === $media) {
             return;
         }
 
-        $provider->postPersist($this->getMedia($args));
+        $this->getProvider($args)->postPersist($media);
     }
 
     public function preUpdate(EventArgs $args)
     {
-        if (!($provider = $this->getProvider($args))) {
+        $media = $this->getMedia($args);
+
+        if (null === $media) {
             return;
         }
 
-        $provider->transform($this->getMedia($args));
-        $provider->preUpdate($this->getMedia($args));
+        $provider = $this->getProvider($args);
+
+        $provider->transform($media);
+        $provider->preUpdate($media);
 
         $this->recomputeSingleEntityChangeSet($args);
     }
 
     public function preRemove(EventArgs $args)
     {
-        if (!($provider = $this->getProvider($args))) {
+        $media = $this->getMedia($args);
+
+        if (null === $media) {
             return;
         }
 
-        $provider->preRemove($this->getMedia($args));
+        $this->getProvider($args)->preRemove($this->getMedia($args));
     }
 
     public function prePersist(EventArgs $args)
     {
-        if (!($provider = $this->getProvider($args))) {
+        $media = $this->getMedia($args);
+
+        if (null === $media) {
             return;
         }
 
-        $provider->transform($this->getMedia($args));
-        $provider->prePersist($this->getMedia($args));
+        $provider = $this->getProvider($args);
+
+        $provider->transform($media);
+        $provider->prePersist($media);
     }
 
     abstract protected function recomputeSingleEntityChangeSet(EventArgs $args);
 
     /**
-     * @return MediaInterface
+     * @return MediaInterface|null
      */
     abstract protected function getMedia(EventArgs $args);
 
@@ -112,10 +128,6 @@ protected function getProvider(EventArgs $args)
     {
         $media = $this->getMedia($args);
 
-        if (!$media instanceof MediaInterface) {
-            return;
-        }
-
         return $this->getPool()->getProvider($media->getProviderName());
     }
 }
diff --git a/src/Listener/ODM/MediaEventSubscriber.php b/src/Listener/ODM/MediaEventSubscriber.php
@@ -16,6 +16,7 @@
 use Doctrine\Common\EventArgs;
 use Doctrine\ODM\MongoDB\Events;
 use Sonata\MediaBundle\Listener\BaseMediaEventSubscriber;
+use Sonata\MediaBundle\Model\MediaInterface;
 
 /**
  * @final since sonata-project/media-bundle 3.21.0
@@ -46,6 +47,12 @@ protected function recomputeSingleEntityChangeSet(EventArgs $args)
 
     protected function getMedia(EventArgs $args)
     {
-        return $args->getDocument();
+        $media = $args->getDocument();
+
+        if (!$media instanceof MediaInterface) {
+            return null;
+        }
+
+        return $media;
     }
 }
diff --git a/src/Listener/ORM/MediaEventSubscriber.php b/src/Listener/ORM/MediaEventSubscriber.php
@@ -62,7 +62,7 @@ protected function getMedia(EventArgs $args)
         $media = $args->getEntity();
 
         if (!$media instanceof MediaInterface) {
-            return $media;
+            return null;
         }
 
         if ($this->container->has('sonata.media.manager.category') && !$media->getCategory()) {

diff --git a/src/Listener/PHPCR/MediaEventSubscriber.php b/src/Listener/PHPCR/MediaEventSubscriber.php
@@ -14,8 +14,11 @@
 namespace Sonata\MediaBundle\Listener\PHPCR;
 
 use Doctrine\Common\EventArgs;
+use Doctrine\ODM\PHPCR\DocumentManager;
 use Doctrine\ODM\PHPCR\Event;
+use Doctrine\Persistence\Event\LifecycleEventArgs;
 use Sonata\MediaBundle\Listener\BaseMediaEventSubscriber;
+use Sonata\MediaBundle\Model\MediaInterface;
 
 /**
  * @final since sonata-project/media-bundle 3.21.0
@@ -34,19 +37,30 @@ public function getSubscribedEvents()
         ];
     }
 
+    /**
+     * @param LifecycleEventArgs $args
+     */
     protected function recomputeSingleEntityChangeSet(EventArgs $args)
     {
-        /* @var $args \Doctrine\Persistence\Event\LifecycleEventArgs */
-        /** @var $dm \Doctrine\ODM\PHPCR\DocumentManager */
+        /** @var DocumentManager $dm */
         $dm = $args->getObjectManager();
 
         $dm->getUnitOfWork()->computeSingleDocumentChangeSet(
             $this->getMedia($args)
         );
     }
 
+    /**
+     * @param LifecycleEventArgs $args
+     */
     protected function getMedia(EventArgs $args)
     {
-        return $args->getObject();
+        $media = $args->getObject();
+
+        if (!$media instanceof MediaInterface) {
+            return null;
+        }
+
+        return $media;
     }
 }
diff --git a/src/Provider/FileProvider.php b/src/Provider/FileProvider.php
@@ -210,10 +210,18 @@ public function getHelperProperties(MediaInterface $media, $format, $options = [
 
     public function generatePrivateUrl(MediaInterface $media, $format)
     {
-        if (MediaProviderInterface::FORMAT_REFERENCE === $format) {
+        if (self::FORMAT_REFERENCE === $format) {
             return $this->getReferenceImage($media);
         }
 
+        // throw new \InvalidArgumentException(sprintf(
+        //     'Argument 2 passed to "%s()" only accepts the value "%s", "%s" given.',
+        //     __METHOD__,
+        //     self::FORMAT_REFERENCE,
+        //     $format
+        // ));
+        // NEXT_MAJOR: Uncomment the previous exception and remove the `return false` statement.
+
         return false;
     }
 

diff --git a/tests/Listener/ORM/MediaEventSubscriberTest.php b/tests/Listener/ORM/MediaEventSubscriberTest.php
@@ -63,6 +63,7 @@ public function testRefetchCategoriesAfterClear(): void
 
         $media1 = $this->createMock(Media::class);
         $media1->method('getContext')->willReturn('context');
+        $media1->method('getProviderName')->willReturn('provider');
 
         $args1 = $this->createMock(LifecycleEventArgs::class);
         $args1->method('getEntity')->willReturn($media1);
@@ -73,6 +74,7 @@ public function testRefetchCategoriesAfterClear(): void
 
         $media2 = $this->createMock(Media::class);
         $media2->method('getContext')->willReturn('context');
+        $media2->method('getProviderName')->willReturn('provider');
 
         $args2 = $this->createMock(LifecycleEventArgs::class);
         $args2->method('getEntity')->willReturn($media2);