diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aeedbd4..0e8aa95a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 index 585985de..794d4459 100644 --- 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 index eac39bd6..1e5e7288 100644 --- 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 index 521ea65a..dc80d10b 100644 --- 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,7 +188,21 @@ 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) @@ -200,4 +210,34 @@ 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 index cdd2bc03..00f00e8d 100644 --- 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 new file mode 100644 index 00000000..a937513f --- /dev/null +++ b/test/Reader/TestAsset/CustomExtensionManager.php @@ -0,0 +1,57 @@ + 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 index b4047ba9..59be94cf 100644 --- 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 index 5c94aabc..b2971b04 100644 --- 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 index 16257307..3fa34868 100644 --- a/test/Writer/Renderer/Entry/AtomTest.php +++ b/test/Writer/Renderer/Entry/AtomTest.php @@ -1,10 +1,8 @@ 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' + ); + } } diff --git a/test/Writer/Renderer/Entry/RssTest.php b/test/Writer/Renderer/Entry/RssTest.php index 88b0ce73..6e053bd6 100644 --- a/test/Writer/Renderer/Entry/RssTest.php +++ b/test/Writer/Renderer/Entry/RssTest.php @@ -1,10 +1,8 @@ validWriter = new Writer\Feed; $this->validWriter->setType('rss'); @@ -42,6 +42,7 @@ public function setUp() public function tearDown() { + Writer\Writer::reset(); $this->validWriter = null; $this->validEntry = null; } @@ -386,4 +387,34 @@ public function testCategoriesCharDataEncoding() ]; $this->assertEquals($expected, (array) $entry->getCategories()); } + + 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\Rss($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' + ); + } } diff --git a/test/Writer/Renderer/Feed/AtomTest.php b/test/Writer/Renderer/Feed/AtomTest.php index b4cfe24c..39cbbc78 100644 --- a/test/Writer/Renderer/Feed/AtomTest.php +++ b/test/Writer/Renderer/Feed/AtomTest.php @@ -15,6 +15,7 @@ use Zend\Feed\Writer\Exception\ExceptionInterface; use Zend\Feed\Writer\Feed; use Zend\Feed\Writer\Renderer; +use ZendTest\Feed\Writer\TestAsset; /** * @group Zend_Feed @@ -26,6 +27,7 @@ class AtomTest extends TestCase public function setUp() { + Writer\Writer::reset(); $this->validWriter = new Writer\Feed; $this->validWriter->setTitle('This is a test feed.'); $this->validWriter->setDescription('This is a test description.'); @@ -41,6 +43,7 @@ public function setUp() public function tearDown() { + Writer\Writer::reset(); $this->validWriter = null; } @@ -420,4 +423,34 @@ public function testImageCanBeSet() ]; $this->assertEquals($expected, $feed->getImage()); } + + public function testFeedRendererEmitsNoticeDuringFeedImportWhenGooglePlayPodcastExtensionUnavailable() + { + // 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\Feed\Atom($this->validWriter); + 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/Feed/RssTest.php b/test/Writer/Renderer/Feed/RssTest.php index 165fcc92..8189a8a8 100644 --- a/test/Writer/Renderer/Feed/RssTest.php +++ b/test/Writer/Renderer/Feed/RssTest.php @@ -1,10 +1,8 @@ validWriter = new Writer\Feed; $this->validWriter->setTitle('This is a test feed.'); $this->validWriter->setDescription('This is a test description.'); @@ -38,6 +38,7 @@ public function setUp() public function tearDown() { + Writer\Writer::reset(); $this->validWriter = null; } @@ -565,4 +566,34 @@ public function testFeedSetDateCreated() $myDate = new DateTime('@' . 1234567890); $this->assertEquals($myDate, $feed->getDateCreated()); } + + public function testFeedRendererEmitsNoticeDuringFeedImportWhenGooglePlayPodcastExtensionUnavailable() + { + // 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\Feed\Rss($this->validWriter); + 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/TestAsset/CustomExtensionManager.php b/test/Writer/TestAsset/CustomExtensionManager.php new file mode 100644 index 00000000..44905852 --- /dev/null +++ b/test/Writer/TestAsset/CustomExtensionManager.php @@ -0,0 +1,59 @@ + Extension\Atom\Renderer\Feed::class, + 'Content\Renderer\Entry' => Extension\Content\Renderer\Entry::class, + 'DublinCore\Renderer\Entry' => Extension\DublinCore\Renderer\Entry::class, + 'DublinCore\Renderer\Feed' => Extension\DublinCore\Renderer\Feed::class, + 'ITunes\Entry' => Extension\ITunes\Entry::class, + 'ITunes\Feed' => Extension\ITunes\Feed::class, + 'ITunes\Renderer\Entry' => Extension\ITunes\Renderer\Entry::class, + 'ITunes\Renderer\Feed' => Extension\ITunes\Renderer\Feed::class, + 'Slash\Renderer\Entry' => Extension\Slash\Renderer\Entry::class, + 'Threading\Renderer\Entry' => Extension\Threading\Renderer\Entry::class, + 'WellFormedWeb\Renderer\Entry' => Extension\WellFormedWeb\Renderer\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 mixed + */ + public function get($extension) + { + $class = $this->has($extension) ? $this->extensions[$extension] : false; + if (! $class) { + throw new InvalidArgumentException(sprintf( + 'Cannot fetch extension "%s"; class "%s" does not exist', + $extension, + $class + )); + } + return new $class(); + } +}