Follow same pattern in Writer subcomponent regarding GooglePlayPodcas…

…t extension This patch adapts the same pattern used in the `Reader` subcomponent with regards to conditionally loading newly introduced extensions to the `Writer` subcomponent: if the extension manager does not know of the extension, it is not registered, and an `E_USER_NOTICE` is emitted prompting the developer to add entries to their extension manager implementation.
zendframework · Jun 18, 2018 · af6dc41 · af6dc41
1 parent dfbb10f
commit af6dc41
Show file tree

Hide file tree

Showing 10 changed files with 341 additions and 52 deletions.
diff --git a/src/Reader/Reader.php b/src/Reader/Reader.php
@@ -577,16 +577,6 @@ public static function getExtensionManager()
      */
     public static function registerExtension($name)
     {
-        $manager   = static::getExtensionManager();
-        $feedName  = $name . '\Feed';
-        $entryName = $name . '\Entry';
-
-        if (static::isRegistered($name)) {
-            if ($manager->has($feedName) || $manager->has($entryName)) {
-                return;
-            }
-        }
-
         if (! static::hasExtension($name)) {
             throw new Exception\RuntimeException(sprintf(
                 'Could not load extension "%s" using Plugin Loader.'
@@ -595,9 +585,19 @@ public static function registerExtension($name)
             ));
         }
 
+        // Return early if already registered.
+        if (static::isRegistered($name)) {
+            return;
+        }
+
+        $manager   = static::getExtensionManager();
+
+        $feedName = $name . '\Feed';
         if ($manager->has($feedName)) {
             static::$extensions['feed'][] = $feedName;
         }
+
+        $entryName = $name . '\Entry';
         if ($manager->has($entryName)) {
             static::$extensions['entry'][] = $entryName;
         }

diff --git a/src/Writer/Renderer/AbstractRenderer.php b/src/Writer/Renderer/AbstractRenderer.php
@@ -222,11 +222,9 @@ protected function _loadExtensions()
         Writer\Writer::registerCoreExtensions();
         $manager = Writer\Writer::getExtensionManager();
         $all = Writer\Writer::getExtensions();
-        if (stripos(get_class($this), 'entry')) {
-            $exts = $all['entryRenderer'];
-        } else {
-            $exts = $all['feedRenderer'];
-        }
+        $exts = stripos(get_class($this), 'entry')
+            ? $all['entryRenderer']
+            : $all['feedRenderer'];
         foreach ($exts as $extension) {
             $plugin = $manager->get($extension);
             $plugin->setDataContainer($this->getDataContainer());

diff --git a/src/Writer/Writer.php b/src/Writer/Writer.php
@@ -91,40 +91,36 @@ public static function getExtensionManager()
      */
     public static function registerExtension($name)
     {
-        $feedName          = $name . '\Feed';
-        $entryName         = $name . '\Entry';
-        $feedRendererName  = $name . '\Renderer\Feed';
-        $entryRendererName = $name . '\Renderer\Entry';
-        $manager           = static::getExtensionManager();
-        if (static::isRegistered($name)) {
-            if ($manager->has($feedName)
-                || $manager->has($entryName)
-                || $manager->has($feedRendererName)
-                || $manager->has($entryRendererName)
-            ) {
-                return;
-            }
-        }
-        if (! $manager->has($feedName)
-            && ! $manager->has($entryName)
-            && ! $manager->has($feedRendererName)
-            && ! $manager->has($entryRendererName)
-        ) {
+        if (! static::hasExtension($name)) {
             throw new Exception\RuntimeException(sprintf(
-                'Could not load extension "%s" using Plugin Loader. '
-                . 'Check prefix paths are configured and extension exists.',
+                'Could not load extension "%s" using Plugin Loader.'
+                . ' Check prefix paths are configured and extension exists.',
                 $name
             ));
         }
+
+        if (static::isRegistered($name)) {
+            return;
+        }
+
+        $manager = static::getExtensionManager();
+
+        $feedName = $name . '\Feed';
         if ($manager->has($feedName)) {
             static::$extensions['feed'][] = $feedName;
         }
+
+        $entryName = $name . '\Entry';
         if ($manager->has($entryName)) {
             static::$extensions['entry'][] = $entryName;
         }
+
+        $feedRendererName = $name . '\Renderer\Feed';
         if ($manager->has($feedRendererName)) {
             static::$extensions['feedRenderer'][] = $feedRendererName;
         }
+
+        $entryRendererName = $name . '\Renderer\Entry';
         if ($manager->has($entryRendererName)) {
             static::$extensions['entryRenderer'][] = $entryRendererName;
         }
@@ -192,12 +188,56 @@ public static function registerCoreExtensions()
         static::registerExtension('WellFormedWeb');
         static::registerExtension('Threading');
         static::registerExtension('ITunes');
-        static::registerExtension('GooglePlayPodcast');
+
+        // Added in 2.10.0; check for it conditionally
+        static::hasExtension('GooglePlayPodcast')
+            ? static::registerExtension('GooglePlayPodcast')
+            : trigger_error(
+                sprintf(
+                    'Please update your %1$s\ExtensionManagerInterface implementation to add entries for'
+                    . ' %1$s\Extension\GooglePlayPodcast\Entry,'
+                    . ' %1$s\Extension\GooglePlayPodcast\Feed,'
+                    . ' %1$s\Extension\GooglePlayPodcast\Renderer\Entry,'
+                    . ' and %1$s\Extension\GooglePlayPodcast\Renderer\Feed.',
+                    __NAMESPACE__
+                ),
+                \E_USER_NOTICE
+            );
     }
 
     public static function lcfirst($str)
     {
         $str[0] = strtolower($str[0]);
         return $str;
     }
+
+    /**
+     * Does the extension manager have the named extension?
+     *
+     * This method exists to allow us to test if an extension is present in the
+     * extension manager. It may be used by registerExtension() to determine if
+     * the extension has items present in the manager, or by
+     * registerCoreExtension() to determine if the core extension has entries
+     * in the extension manager. In the latter case, this can be useful when
+     * adding new extensions in a minor release, as custom extension manager
+     * implementations may not yet have an entry for the extension, which would
+     * then otherwise cause registerExtension() to fail.
+     *
+     * @param string $name
+     * @return bool
+     */
+    protected static function hasExtension($name)
+    {
+        $manager   = static::getExtensionManager();
+
+        $feedName          = $name . '\Feed';
+        $entryName         = $name . '\Entry';
+        $feedRendererName  = $name . '\Renderer\Feed';
+        $entryRendererName = $name . '\Renderer\Entry';
+
+        return $manager->has($feedName)
+            || $manager->has($entryName)
+            || $manager->has($feedRendererName)
+            || $manager->has($entryRendererName);
+    }
 }
diff --git a/test/Writer/EntryTest.php b/test/Writer/EntryTest.php
@@ -27,6 +27,12 @@ class EntryTest extends TestCase
     public function setup()
     {
         $this->feedSamplePath = dirname(__FILE__) . '/_files';
+        Writer\Writer::reset();
+    }
+
+    public function tearDown()
+    {
+        Writer\Writer::reset();
     }
 
     public function testAddsAuthorNameFromArray()
@@ -737,4 +743,31 @@ public function testSetTitleShouldAllowAStringWithTheContentsZero()
         $entry->setTitle('0');
         $this->assertEquals('0', $entry->getTitle());
     }
+
+    public function testEntryWriterEmitsNoticeDuringFeedImportWhenGooglePlayPodcastExtensionUnavailable()
+    {
+        Writer\Writer::setExtensionManager(new TestAsset\CustomExtensionManager());
+
+        $notices = (object) [
+            'messages' => [],
+        ];
+
+        set_error_handler(function ($errno, $errstr) use ($notices) {
+            $notices->messages[] = $errstr;
+        }, \E_USER_NOTICE);
+        $writer = new Writer\Entry();
+        restore_error_handler();
+
+        $message = array_reduce($notices->messages, function ($toReturn, $message) {
+            if ('' !== $toReturn) {
+                return $toReturn;
+            }
+            return false === strstr($message, 'GooglePlayPodcast') ? '' : $message;
+        }, '');
+
+        $this->assertNotEmpty(
+            $message,
+            'GooglePlayPodcast extension was present in extension manager, but was not expected to be'
+        );
+    }
 }
diff --git a/test/Writer/FeedTest.php b/test/Writer/FeedTest.php
@@ -28,6 +28,12 @@ class FeedTest extends TestCase
     public function setup()
     {
         $this->feedSamplePath = dirname(__FILE__) . '/Writer/_files';
+        Writer\Writer::reset();
+    }
+
+    public function tearDown()
+    {
+        Writer\Writer::reset();
     }
 
     public function testAddsAuthorNameFromArray()
@@ -1088,4 +1094,31 @@ public function testSetTitleShouldAllowAStringWithTheContentsZero()
         $feed->setTitle('0');
         $this->assertEquals('0', $feed->getTitle());
     }
+
+    public function testFeedWriterEmitsNoticeDuringFeedImportWhenGooglePlayPodcastExtensionUnavailable()
+    {
+        Writer\Writer::setExtensionManager(new TestAsset\CustomExtensionManager());
+
+        $notices = (object) [
+            'messages' => [],
+        ];
+
+        set_error_handler(function ($errno, $errstr) use ($notices) {
+            $notices->messages[] = $errstr;
+        }, \E_USER_NOTICE);
+        $writer = new Writer\Feed();
+        restore_error_handler();
+
+        $message = array_reduce($notices->messages, function ($toReturn, $message) {
+            if ('' !== $toReturn) {
+                return $toReturn;
+            }
+            return false === strstr($message, 'GooglePlayPodcast') ? '' : $message;
+        }, '');
+
+        $this->assertNotEmpty(
+            $message,
+            'GooglePlayPodcast extension was present in extension manager, but was not expected to be'
+        );
+    }
 }
diff --git a/test/Writer/Renderer/Entry/AtomTest.php b/test/Writer/Renderer/Entry/AtomTest.php
@@ -1,10 +1,8 @@
 <?php
 /**
- * Zend Framework (http://framework.zend.com/)
- *
- * @link      http://github.com/zendframework/zf2 for the canonical source repository
- * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
- * @license   http://framework.zend.com/license/new-bsd New BSD License
+ * @see       https://github.com/zendframework/zend-feed for the canonical source repository
+ * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-feed/blob/master/LICENSE.md New BSD License
  */
 
 namespace ZendTest\Feed\Writer\Renderer\Entry;
@@ -14,6 +12,7 @@
 use Zend\Feed\Writer;
 use Zend\Feed\Writer\Exception\ExceptionInterface;
 use Zend\Feed\Writer\Renderer;
+use ZendTest\Feed\Writer\TestAsset;
 
 /**
  * @group      Zend_Feed
@@ -26,6 +25,7 @@ class AtomTest extends TestCase
 
     public function setUp()
     {
+        Writer\Writer::reset();
         $this->validWriter = new Writer\Feed;
 
         $this->validWriter->setType('atom');
@@ -53,6 +53,7 @@ public function setUp()
 
     public function tearDown()
     {
+        Writer\Writer::reset();
         $this->validWriter = null;
         $this->validEntry  = null;
     }
@@ -298,4 +299,34 @@ public function testCommentFeedLinksRendered()
         //$this->assertEquals('http://www.example.com/rss/id/1', $entry->getCommentFeedLink('rss'));
         $this->assertEquals('http://www.example.com/atom/id/1', $entry->getCommentFeedLink('atom'));
     }
+
+    public function testEntryRendererEmitsNoticeDuringInstantiationWhenGooglePlayPodcastExtensionUnavailable()
+    {
+        // Since we create feed and entry writer instances in the test constructor,
+        // we need to reset it _now_ before creating a new renderer.
+        Writer\Writer::reset();
+        Writer\Writer::setExtensionManager(new TestAsset\CustomExtensionManager());
+
+        $notices = (object) [
+            'messages' => [],
+        ];
+
+        set_error_handler(function ($errno, $errstr) use ($notices) {
+            $notices->messages[] = $errstr;
+        }, \E_USER_NOTICE);
+        $renderer = new Renderer\Entry\Atom($this->validEntry);
+        restore_error_handler();
+
+        $message = array_reduce($notices->messages, function ($toReturn, $message) {
+            if ('' !== $toReturn) {
+                return $toReturn;
+            }
+            return false === strstr($message, 'GooglePlayPodcast') ? '' : $message;
+        }, '');
+
+        $this->assertNotEmpty(
+            $message,
+            'GooglePlayPodcast extension was present in extension manager, but was not expected to be'
+        );
+    }
 }