Merge branch 'hotfix/google-play-podcast-extension-entry'

Close #81 Fixes #80
zendframework · Jun 18, 2018 · 5253f94 · 5253f94
2 parents 875414e + c67833a
commit 5253f94
Show file tree

Hide file tree

Showing 13 changed files with 482 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,7 @@
 
 All notable changes to this project will be documented in this file, in reverse chronological order by release.
 
-## 2.10.2 - TBD
+## 2.10.2 - 2018-06-18
 
 ### Added
 
@@ -22,7 +22,19 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Fixed
 
-- Nothing.
+- [#81](https://github.com/zendframework/zend-feed/pull/81) updates the `Zend\Feed\Reader\Reader` and `Zend\Feed\Writer\Writer` classes to
+  conditionally register their respective "GooglePlayPodcast" extensions only if
+  their extension managers are aware of it. This is done due to the fact that
+  existing `ExtensionManagerInterface` implementations may not register it by
+  default as the extension did not exist in releases prior to 2.10.0. By having
+  the registration conditional, we prevent an exception from being raised; users
+  are not impacted by its absence, as the extension features were not exposed
+  previously.
+
+  Both `Reader` and `Writer` emit an `E_USER_NOTICE` when the extension is not
+  found in the extension manager, indicating that the
+  `ExtensionManagerInterface` implementation should be updated to add entries
+  for the "GooglePlayPodcast" entry, feed, and/or renderer classes.
 
 ## 2.10.1 - 2018-06-05
 

diff --git a/src/Reader/Reader.php b/src/Reader/Reader.php
@@ -577,22 +577,27 @@ public static function getExtensionManager()
      */
     public static function registerExtension($name)
     {
-        $feedName  = $name . '\Feed';
-        $entryName = $name . '\Entry';
-        $manager   = static::getExtensionManager();
-        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.'
+                . ' Check prefix paths are configured and extension exists.',
+                $name
+            ));
         }
 
-        if (! $manager->has($feedName) && ! $manager->has($entryName)) {
-            throw new Exception\RuntimeException('Could not load extension: ' . $name
-                . ' using Plugin Loader. Check prefix paths are configured and extension exists.');
+        // 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;
         }
@@ -672,7 +677,18 @@ protected static function registerCoreExtensions()
         static::registerExtension('WellFormedWeb');
         static::registerExtension('Thread');
         static::registerExtension('Podcast');
-        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 and %1$s\Extension\GooglePlayPodcast\Feed.',
+                    __NAMESPACE__
+                ),
+                \E_USER_NOTICE
+            );
     }
 
     /**
@@ -693,4 +709,28 @@ public static function arrayUnique(array $array)
         }
         return $array;
     }
+
+    /**
+     * 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)
+    {
+        $feedName  = $name . '\Feed';
+        $entryName = $name . '\Entry';
+        $manager   = static::getExtensionManager();
+
+        return $manager->has($feedName) || $manager->has($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/Reader/ReaderTest.php b/test/Reader/ReaderTest.php
@@ -353,6 +353,35 @@ public function testSetHttpClientThrowsException()
         Reader\Reader::setHttpClient(new stdClass);
     }
 
+    public function testReaderEmitsNoticeDuringFeedImportWhenGooglePlayPodcastExtensionUnavailable()
+    {
+        Reader\Reader::setExtensionManager(new TestAsset\CustomExtensionManager());
+
+        $notices = (object) [
+            'messages' => [],
+        ];
+
+        set_error_handler(function ($errno, $errstr) use ($notices) {
+            $notices->messages[] = $errstr;
+        }, \E_USER_NOTICE);
+        $feed = Reader\Reader::importFile(
+            dirname(__FILE__) . '/Entry/_files/Atom/title/plain/atom10.xml'
+        );
+        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'
+        );
+    }
+
     // @codingStandardsIgnoreStart
     protected function _getTempDirectory()
     {

diff --git a/test/Reader/TestAsset/CustomExtensionManager.php b/test/Reader/TestAsset/CustomExtensionManager.php
@@ -0,0 +1,57 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-feed for the canonical source repository
+ * @copyright Copyright (c) 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\Reader\TestAsset;
+
+use Zend\Feed\Reader\Exception\InvalidArgumentException;
+use Zend\Feed\Reader\Extension;
+use Zend\Feed\Reader\ExtensionManagerInterface;
+
+/**
+ * Standalone extension manager that omits any extensions added after the 2.9 series.
+ */
+class CustomExtensionManager implements ExtensionManagerInterface
+{
+    private $extensions = [
+        'Atom\Entry'            => Extension\Atom\Entry::class,
+        'Atom\Feed'             => Extension\Atom\Feed::class,
+        'Content\Entry'         => Extension\Content\Entry::class,
+        'CreativeCommons\Entry' => Extension\CreativeCommons\Entry::class,
+        'CreativeCommons\Feed'  => Extension\CreativeCommons\Feed::class,
+        'DublinCore\Entry'      => Extension\DublinCore\Entry::class,
+        'DublinCore\Feed'       => Extension\DublinCore\Feed::class,
+        'Podcast\Entry'         => Extension\Podcast\Entry::class,
+        'Podcast\Feed'          => Extension\Podcast\Feed::class,
+        'Slash\Entry'           => Extension\Slash\Entry::class,
+        'Syndication\Feed'      => Extension\Syndication\Feed::class,
+        'Thread\Entry'          => Extension\Thread\Entry::class,
+        'WellFormedWeb\Entry'   => Extension\WellFormedWeb\Entry::class,
+    ];
+
+    /**
+     * Do we have the extension?
+     *
+     * @param  string $extension
+     * @return bool
+     */
+    public function has($extension)
+    {
+        return array_key_exists($extension, $this->extensions);
+    }
+
+    /**
+     * Retrieve the extension
+     *
+     * @param  string $extension
+     * @return Extension\AbstractEntry|Extension\AbstractFeed
+     */
+    public function get($extension)
+    {
+        $class = $this->extensions[$extension];
+        return new $class();
+    }
+}
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'
+        );
+    }
 }