Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

Commit dfbb10f

Browse files
committed
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.
1 parent 875414e commit dfbb10f

File tree

3 files changed

+131
-5
lines changed

3 files changed

+131
-5
lines changed

src/Reader/Reader.php

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -577,19 +577,24 @@ public static function getExtensionManager()
577577
*/
578578
public static function registerExtension($name)
579579
{
580+
$manager = static::getExtensionManager();
580581
$feedName = $name . '\Feed';
581582
$entryName = $name . '\Entry';
582-
$manager = static::getExtensionManager();
583+
583584
if (static::isRegistered($name)) {
584585
if ($manager->has($feedName) || $manager->has($entryName)) {
585586
return;
586587
}
587588
}
588589

589-
if (! $manager->has($feedName) && ! $manager->has($entryName)) {
590-
throw new Exception\RuntimeException('Could not load extension: ' . $name
591-
. ' using Plugin Loader. Check prefix paths are configured and extension exists.');
590+
if (! static::hasExtension($name)) {
591+
throw new Exception\RuntimeException(sprintf(
592+
'Could not load extension "%s" using Plugin Loader.'
593+
. ' Check prefix paths are configured and extension exists.',
594+
$name
595+
));
592596
}
597+
593598
if ($manager->has($feedName)) {
594599
static::$extensions['feed'][] = $feedName;
595600
}
@@ -672,7 +677,18 @@ protected static function registerCoreExtensions()
672677
static::registerExtension('WellFormedWeb');
673678
static::registerExtension('Thread');
674679
static::registerExtension('Podcast');
675-
static::registerExtension('GooglePlayPodcast');
680+
681+
// Added in 2.10.0; check for it conditionally
682+
static::hasExtension('GooglePlayPodcast')
683+
? static::registerExtension('GooglePlayPodcast')
684+
: trigger_error(
685+
sprintf(
686+
'Please update your %1$s\ExtensionManagerInterface implementation to add entries for'
687+
. ' %1$s\Extension\GooglePlayPodcast\Entry and %1$s\Extension\GooglePlayPodcast\Feed.',
688+
__NAMESPACE__
689+
),
690+
\E_USER_NOTICE
691+
);
676692
}
677693

678694
/**
@@ -693,4 +709,28 @@ public static function arrayUnique(array $array)
693709
}
694710
return $array;
695711
}
712+
713+
/**
714+
* Does the extension manager have the named extension?
715+
*
716+
* This method exists to allow us to test if an extension is present in the
717+
* extension manager. It may be used by registerExtension() to determine if
718+
* the extension has items present in the manager, or by
719+
* registerCoreExtension() to determine if the core extension has entries
720+
* in the extension manager. In the latter case, this can be useful when
721+
* adding new extensions in a minor release, as custom extension manager
722+
* implementations may not yet have an entry for the extension, which would
723+
* then otherwise cause registerExtension() to fail.
724+
*
725+
* @param string $name
726+
* @return bool
727+
*/
728+
protected static function hasExtension($name)
729+
{
730+
$feedName = $name . '\Feed';
731+
$entryName = $name . '\Entry';
732+
$manager = static::getExtensionManager();
733+
734+
return $manager->has($feedName) || $manager->has($entryName);
735+
}
696736
}

test/Reader/ReaderTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,35 @@ public function testSetHttpClientThrowsException()
353353
Reader\Reader::setHttpClient(new stdClass);
354354
}
355355

356+
public function testReaderEmitsNoticeDuringFeedImportWhenGooglePlayPodcastExtensionUnavailable()
357+
{
358+
Reader\Reader::setExtensionManager(new TestAsset\CustomExtensionManager());
359+
360+
$notices = (object) [
361+
'messages' => [],
362+
];
363+
364+
set_error_handler(function ($errno, $errstr) use ($notices) {
365+
$notices->messages[] = $errstr;
366+
}, \E_USER_NOTICE);
367+
$feed = Reader\Reader::importFile(
368+
dirname(__FILE__) . '/Entry/_files/Atom/title/plain/atom10.xml'
369+
);
370+
restore_error_handler();
371+
372+
$message = array_reduce($notices->messages, function ($toReturn, $message) {
373+
if ('' !== $toReturn) {
374+
return $toReturn;
375+
}
376+
return false === strstr($message, 'GooglePlayPodcast') ? '' : $message;
377+
}, '');
378+
379+
$this->assertNotEmpty(
380+
$message,
381+
'GooglePlayPodcast extension was present in extension manager, but was not expected to be'
382+
);
383+
}
384+
356385
// @codingStandardsIgnoreStart
357386
protected function _getTempDirectory()
358387
{
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
/**
3+
* @see https://github.com/zendframework/zend-feed for the canonical source repository
4+
* @copyright Copyright (c) 2018 Zend Technologies USA Inc. (https://www.zend.com)
5+
* @license https://github.com/zendframework/zend-feed/blob/master/LICENSE.md New BSD License
6+
*/
7+
8+
namespace ZendTest\Feed\Reader\TestAsset;
9+
10+
use Zend\Feed\Reader\Exception\InvalidArgumentException;
11+
use Zend\Feed\Reader\Extension;
12+
use Zend\Feed\Reader\ExtensionManagerInterface;
13+
14+
/**
15+
* Standalone extension manager that omits any extensions added after the 2.9 series.
16+
*/
17+
class CustomExtensionManager implements ExtensionManagerInterface
18+
{
19+
private $extensions = [
20+
'Atom\Entry' => Extension\Atom\Entry::class,
21+
'Atom\Feed' => Extension\Atom\Feed::class,
22+
'Content\Entry' => Extension\Content\Entry::class,
23+
'CreativeCommons\Entry' => Extension\CreativeCommons\Entry::class,
24+
'CreativeCommons\Feed' => Extension\CreativeCommons\Feed::class,
25+
'DublinCore\Entry' => Extension\DublinCore\Entry::class,
26+
'DublinCore\Feed' => Extension\DublinCore\Feed::class,
27+
'Podcast\Entry' => Extension\Podcast\Entry::class,
28+
'Podcast\Feed' => Extension\Podcast\Feed::class,
29+
'Slash\Entry' => Extension\Slash\Entry::class,
30+
'Syndication\Feed' => Extension\Syndication\Feed::class,
31+
'Thread\Entry' => Extension\Thread\Entry::class,
32+
'WellFormedWeb\Entry' => Extension\WellFormedWeb\Entry::class,
33+
];
34+
35+
/**
36+
* Do we have the extension?
37+
*
38+
* @param string $extension
39+
* @return bool
40+
*/
41+
public function has($extension)
42+
{
43+
return array_key_exists($extension, $this->extensions);
44+
}
45+
46+
/**
47+
* Retrieve the extension
48+
*
49+
* @param string $extension
50+
* @return Extension\AbstractEntry|Extension\AbstractFeed
51+
*/
52+
public function get($extension)
53+
{
54+
$class = $this->extensions[$extension];
55+
return new $class();
56+
}
57+
}

0 commit comments

Comments
 (0)