From e8aaf8bd72c43d9b28aa712cd39e3e74a71f1b28 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 18 Jun 2018 11:10:09 -0500 Subject: [PATCH] Allow usage of Reader without GooglePlayPodcast extension Custom implementations of `ExtensionManagerInterface` may not have _new core_ extensions present. As a result, when upgrading, an exception is thrown due to inability to load the new extension. This was discovered with the 2.10 release, when we added a new core extension, GooglePlayPodcast; see #80 and https://www.drupal.org/project/drupal/issues/2976335 for more details. As such, we now check for _new_ core extensions before registering them. When we discover they are not in the extension manager, we emit an `E_USER_NOTICE` indicating the user should update their `ExtensionManagerInterface` implementation to add entries for the new extension, but then continue on without registering the extension. This ensures no exception is thrown. --- src/Reader/Reader.php | 50 +++++++++- test/Reader/ReaderTest.php | 29 ++++++ .../TestAsset/CustomExtensionManager.php | 93 +++++++++++++++++++ 3 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 test/Reader/TestAsset/CustomExtensionManager.php diff --git a/src/Reader/Reader.php b/src/Reader/Reader.php index 585985de..ffd88432 100644 --- a/src/Reader/Reader.php +++ b/src/Reader/Reader.php @@ -577,19 +577,24 @@ public static function getExtensionManager() */ public static function registerExtension($name) { + $manager = static::getExtensionManager(); $feedName = $name . '\Feed'; $entryName = $name . '\Entry'; - $manager = static::getExtensionManager(); + if (static::isRegistered($name)) { if ($manager->has($feedName) || $manager->has($entryName)) { return; } } - 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.'); + 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)) { static::$extensions['feed'][] = $feedName; } @@ -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/test/Reader/ReaderTest.php b/test/Reader/ReaderTest.php index cdd2bc03..ce9ad5a9 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..6eeee701 --- /dev/null +++ b/test/Reader/TestAsset/CustomExtensionManager.php @@ -0,0 +1,93 @@ + 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(); + } + + /** + * Add an extension. + * + * @param string $name + * @param string $class + */ + public function add($name, $class) + { + if (is_string($class) + && ( + is_a($class, Extension\AbstractEntry::class, true) + || is_a($class, Extension\AbstractFeed::class, true) + ) + ) { + $this->extensions[$name] = $class; + return; + } + + throw new InvalidArgumentException(sprintf( + 'Plugin of type %s is invalid; must implement %2$s\Extension\AbstractFeed ' + . 'or %2$s\Extension\AbstractEntry', + $class, + 'Zend\Feed\Reader' + )); + } + + /** + * Remove an extension. + * + * @param string $name + */ + public function remove($name) + { + unset($this->extensions[$name]); + } +}