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

Commit 5253f94

Browse files
weierophinneyweierophinney
committed
Merge branch 'hotfix/google-play-podcast-extension-entry'
Close #81 Fixes #80
2 parents 875414e + c67833a commit 5253f94

File tree

13 files changed

+482
-55
lines changed

13 files changed

+482
-55
lines changed

‎CHANGELOG.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
All notable changes to this project will be documented in this file, in reverse chronological order by release.
44

5-
## 2.10.2 - TBD
5+
## 2.10.2 - 2018-06-18
66

77
### Added
88

@@ -22,7 +22,19 @@ All notable changes to this project will be documented in this file, in reverse
2222

2323
### Fixed
2424

25-
- Nothing.
25+
- [#81](https://github.com/zendframework/zend-feed/pull/81) updates the `Zend\Feed\Reader\Reader` and `Zend\Feed\Writer\Writer` classes to
26+
conditionally register their respective "GooglePlayPodcast" extensions only if
27+
their extension managers are aware of it. This is done due to the fact that
28+
existing `ExtensionManagerInterface` implementations may not register it by
29+
default as the extension did not exist in releases prior to 2.10.0. By having
30+
the registration conditional, we prevent an exception from being raised; users
31+
are not impacted by its absence, as the extension features were not exposed
32+
previously.
33+
34+
Both `Reader` and `Writer` emit an `E_USER_NOTICE` when the extension is not
35+
found in the extension manager, indicating that the
36+
`ExtensionManagerInterface` implementation should be updated to add entries
37+
for the "GooglePlayPodcast" entry, feed, and/or renderer classes.
2638

2739
## 2.10.1 - 2018-06-05
2840

‎src/Reader/Reader.php

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -577,22 +577,27 @@ public static function getExtensionManager()
577577
*/
578578
public static function registerExtension($name)
579579
{
580-
$feedName = $name . '\Feed';
581-
$entryName = $name . '\Entry';
582-
$manager = static::getExtensionManager();
583-
if (static::isRegistered($name)) {
584-
if ($manager->has($feedName) || $manager->has($entryName)) {
585-
return;
586-
}
580+
if (! static::hasExtension($name)) {
581+
throw new Exception\RuntimeException(sprintf(
582+
'Could not load extension "%s" using Plugin Loader.'
583+
. ' Check prefix paths are configured and extension exists.',
584+
$name
585+
));
587586
}
588587

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.');
588+
// Return early if already registered.
589+
if (static::isRegistered($name)) {
590+
return;
592591
}
592+
593+
$manager = static::getExtensionManager();
594+
595+
$feedName = $name . '\Feed';
593596
if ($manager->has($feedName)) {
594597
static::$extensions['feed'][] = $feedName;
595598
}
599+
600+
$entryName = $name . '\Entry';
596601
if ($manager->has($entryName)) {
597602
static::$extensions['entry'][] = $entryName;
598603
}
@@ -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
}

‎src/Writer/Renderer/AbstractRenderer.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,9 @@ protected function _loadExtensions()
222222
Writer\Writer::registerCoreExtensions();
223223
$manager = Writer\Writer::getExtensionManager();
224224
$all = Writer\Writer::getExtensions();
225-
if (stripos(get_class($this), 'entry')) {
226-
$exts = $all['entryRenderer'];
227-
} else {
228-
$exts = $all['feedRenderer'];
229-
}
225+
$exts = stripos(get_class($this), 'entry')
226+
? $all['entryRenderer']
227+
: $all['feedRenderer'];
230228
foreach ($exts as $extension) {
231229
$plugin = $manager->get($extension);
232230
$plugin->setDataContainer($this->getDataContainer());

‎src/Writer/Writer.php

Lines changed: 62 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -91,40 +91,36 @@ public static function getExtensionManager()
9191
*/
9292
public static function registerExtension($name)
9393
{
94-
$feedName = $name . '\Feed';
95-
$entryName = $name . '\Entry';
96-
$feedRendererName = $name . '\Renderer\Feed';
97-
$entryRendererName = $name . '\Renderer\Entry';
98-
$manager = static::getExtensionManager();
99-
if (static::isRegistered($name)) {
100-
if ($manager->has($feedName)
101-
|| $manager->has($entryName)
102-
|| $manager->has($feedRendererName)
103-
|| $manager->has($entryRendererName)
104-
) {
105-
return;
106-
}
107-
}
108-
if (! $manager->has($feedName)
109-
&& ! $manager->has($entryName)
110-
&& ! $manager->has($feedRendererName)
111-
&& ! $manager->has($entryRendererName)
112-
) {
94+
if (! static::hasExtension($name)) {
11395
throw new Exception\RuntimeException(sprintf(
114-
'Could not load extension "%s" using Plugin Loader. '
115-
. 'Check prefix paths are configured and extension exists.',
96+
'Could not load extension "%s" using Plugin Loader.'
97+
. ' Check prefix paths are configured and extension exists.',
11698
$name
11799
));
118100
}
101+
102+
if (static::isRegistered($name)) {
103+
return;
104+
}
105+
106+
$manager = static::getExtensionManager();
107+
108+
$feedName = $name . '\Feed';
119109
if ($manager->has($feedName)) {
120110
static::$extensions['feed'][] = $feedName;
121111
}
112+
113+
$entryName = $name . '\Entry';
122114
if ($manager->has($entryName)) {
123115
static::$extensions['entry'][] = $entryName;
124116
}
117+
118+
$feedRendererName = $name . '\Renderer\Feed';
125119
if ($manager->has($feedRendererName)) {
126120
static::$extensions['feedRenderer'][] = $feedRendererName;
127121
}
122+
123+
$entryRendererName = $name . '\Renderer\Entry';
128124
if ($manager->has($entryRendererName)) {
129125
static::$extensions['entryRenderer'][] = $entryRendererName;
130126
}
@@ -192,12 +188,56 @@ public static function registerCoreExtensions()
192188
static::registerExtension('WellFormedWeb');
193189
static::registerExtension('Threading');
194190
static::registerExtension('ITunes');
195-
static::registerExtension('GooglePlayPodcast');
191+
192+
// Added in 2.10.0; check for it conditionally
193+
static::hasExtension('GooglePlayPodcast')
194+
? static::registerExtension('GooglePlayPodcast')
195+
: trigger_error(
196+
sprintf(
197+
'Please update your %1$s\ExtensionManagerInterface implementation to add entries for'
198+
. ' %1$s\Extension\GooglePlayPodcast\Entry,'
199+
. ' %1$s\Extension\GooglePlayPodcast\Feed,'
200+
. ' %1$s\Extension\GooglePlayPodcast\Renderer\Entry,'
201+
. ' and %1$s\Extension\GooglePlayPodcast\Renderer\Feed.',
202+
__NAMESPACE__
203+
),
204+
\E_USER_NOTICE
205+
);
196206
}
197207

198208
public static function lcfirst($str)
199209
{
200210
$str[0] = strtolower($str[0]);
201211
return $str;
202212
}
213+
214+
/**
215+
* Does the extension manager have the named extension?
216+
*
217+
* This method exists to allow us to test if an extension is present in the
218+
* extension manager. It may be used by registerExtension() to determine if
219+
* the extension has items present in the manager, or by
220+
* registerCoreExtension() to determine if the core extension has entries
221+
* in the extension manager. In the latter case, this can be useful when
222+
* adding new extensions in a minor release, as custom extension manager
223+
* implementations may not yet have an entry for the extension, which would
224+
* then otherwise cause registerExtension() to fail.
225+
*
226+
* @param string $name
227+
* @return bool
228+
*/
229+
protected static function hasExtension($name)
230+
{
231+
$manager = static::getExtensionManager();
232+
233+
$feedName = $name . '\Feed';
234+
$entryName = $name . '\Entry';
235+
$feedRendererName = $name . '\Renderer\Feed';
236+
$entryRendererName = $name . '\Renderer\Entry';
237+
238+
return $manager->has($feedName)
239+
|| $manager->has($entryName)
240+
|| $manager->has($feedRendererName)
241+
|| $manager->has($entryRendererName);
242+
}
203243
}

‎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
{

‎test/Reader/TestAsset/CustomExtensionManager.php

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+
}

‎test/Writer/EntryTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ class EntryTest extends TestCase
2727
public function setup()
2828
{
2929
$this->feedSamplePath = dirname(__FILE__) . '/_files';
30+
Writer\Writer::reset();
31+
}
32+
33+
public function tearDown()
34+
{
35+
Writer\Writer::reset();
3036
}
3137

3238
public function testAddsAuthorNameFromArray()
@@ -737,4 +743,31 @@ public function testSetTitleShouldAllowAStringWithTheContentsZero()
737743
$entry->setTitle('0');
738744
$this->assertEquals('0', $entry->getTitle());
739745
}
746+
747+
public function testEntryWriterEmitsNoticeDuringFeedImportWhenGooglePlayPodcastExtensionUnavailable()
748+
{
749+
Writer\Writer::setExtensionManager(new TestAsset\CustomExtensionManager());
750+
751+
$notices = (object) [
752+
'messages' => [],
753+
];
754+
755+
set_error_handler(function ($errno, $errstr) use ($notices) {
756+
$notices->messages[] = $errstr;
757+
}, \E_USER_NOTICE);
758+
$writer = new Writer\Entry();
759+
restore_error_handler();
760+
761+
$message = array_reduce($notices->messages, function ($toReturn, $message) {
762+
if ('' !== $toReturn) {
763+
return $toReturn;
764+
}
765+
return false === strstr($message, 'GooglePlayPodcast') ? '' : $message;
766+
}, '');
767+
768+
$this->assertNotEmpty(
769+
$message,
770+
'GooglePlayPodcast extension was present in extension manager, but was not expected to be'
771+
);
772+
}
740773
}

0 commit comments

Comments
 (0)