From cfcd5fb6f802be11bbca15a855494545e3c2d413 Mon Sep 17 00:00:00 2001 From: Fabio Date: Thu, 24 May 2018 10:58:33 +0200 Subject: [PATCH 1/4] Added itunes:image support to ITunes Renderer --- src/Writer/Extension/ITunes/Entry.php | 24 +++++++++++++++++++ .../Extension/ITunes/Renderer/Entry.php | 22 +++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/Writer/Extension/ITunes/Entry.php b/src/Writer/Extension/ITunes/Entry.php index c9228307..f04b065e 100644 --- a/src/Writer/Extension/ITunes/Entry.php +++ b/src/Writer/Extension/ITunes/Entry.php @@ -9,6 +9,7 @@ namespace Zend\Feed\Writer\Extension\ITunes; +use Zend\Feed\Uri; use Zend\Feed\Writer; use Zend\Stdlib\StringUtils; use Zend\Stdlib\StringWrapper\StringWrapperInterface; @@ -142,6 +143,29 @@ public function setItunesDuration($value) return $this; } + + /** + * Set feed image (icon) + * + * @param string $value + * @return Feed + * @throws Writer\Exception\InvalidArgumentException + */ + public function setItunesImage($value) + { + if (! Uri::factory($value)->isValid()) { + throw new Writer\Exception\InvalidArgumentException('invalid parameter: "image" may only' + . ' be a valid URI/IRI'); + } + if (! in_array(substr($value, -3), ['jpg', 'png'])) { + throw new Writer\Exception\InvalidArgumentException('invalid parameter: "image" may only' + . ' use file extension "jpg" or "png" which must be the last three' + . ' characters of the URI (i.e. no query string or fragment)'); + } + $this->data['image'] = $value; + return $this; + } + /** * Set "explicit" flag * diff --git a/src/Writer/Extension/ITunes/Renderer/Entry.php b/src/Writer/Extension/ITunes/Renderer/Entry.php index 9e996e51..58c4f94a 100644 --- a/src/Writer/Extension/ITunes/Renderer/Entry.php +++ b/src/Writer/Extension/ITunes/Renderer/Entry.php @@ -36,6 +36,7 @@ public function render() $this->_setAuthors($this->dom, $this->base); $this->_setBlock($this->dom, $this->base); $this->_setDuration($this->dom, $this->base); + $this->_setImage($this->dom, $this->base); $this->_setExplicit($this->dom, $this->base); $this->_setKeywords($this->dom, $this->base); $this->_setSubtitle($this->dom, $this->base); @@ -128,6 +129,27 @@ protected function _setDuration(DOMDocument $dom, DOMElement $root) $this->called = true; } + /** + * Set feed image (icon) + * + * @param DOMDocument $dom + * @param DOMElement $root + * @return void + */ + // @codingStandardsIgnoreStart + protected function _setImage(DOMDocument $dom, DOMElement $root) + { + // @codingStandardsIgnoreEnd + $image = $this->getDataContainer()->getItunesImage(); + if (! $image) { + return; + } + $el = $dom->createElement('itunes:image'); + $el->setAttribute('href', $image); + $root->appendChild($el); + $this->called = true; + } + /** * Set explicit flag * From 4ff8b540f1f5f71c2c98cd05e56e84a24e9043a9 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 24 May 2018 10:03:53 -0500 Subject: [PATCH 2/4] Ensure itunes:image for entry and feed are tested, and both renderers will provide output The itunes:image tag may be used both at the podcast feed and episode level, so support needs to be in both locations. This also means that the renderers for each need to output the tag. --- src/Writer/Extension/ITunes/Entry.php | 50 ++++++++++++---------- src/Writer/Extension/ITunes/Feed.php | 2 +- test/Writer/Extension/ITunes/EntryTest.php | 50 ++++++++++++++++++++++ test/Writer/Extension/ITunes/FeedTest.php | 50 ++++++++++++++++++++++ 4 files changed, 128 insertions(+), 24 deletions(-) diff --git a/src/Writer/Extension/ITunes/Entry.php b/src/Writer/Extension/ITunes/Entry.php index f04b065e..562770e1 100644 --- a/src/Writer/Extension/ITunes/Entry.php +++ b/src/Writer/Extension/ITunes/Entry.php @@ -143,29 +143,6 @@ public function setItunesDuration($value) return $this; } - - /** - * Set feed image (icon) - * - * @param string $value - * @return Feed - * @throws Writer\Exception\InvalidArgumentException - */ - public function setItunesImage($value) - { - if (! Uri::factory($value)->isValid()) { - throw new Writer\Exception\InvalidArgumentException('invalid parameter: "image" may only' - . ' be a valid URI/IRI'); - } - if (! in_array(substr($value, -3), ['jpg', 'png'])) { - throw new Writer\Exception\InvalidArgumentException('invalid parameter: "image" may only' - . ' use file extension "jpg" or "png" which must be the last three' - . ' characters of the URI (i.e. no query string or fragment)'); - } - $this->data['image'] = $value; - return $this; - } - /** * Set "explicit" flag * @@ -241,6 +218,33 @@ public function setItunesSummary($value) return $this; } + /** + * Set entry image (icon) + * + * @param string $value + * @return Feed + * @throws Writer\Exception\InvalidArgumentException + */ + public function setItunesImage($value) + { + if (! is_string($value) || ! Uri::factory($value)->isValid()) { + throw new Writer\Exception\InvalidArgumentException( + 'invalid parameter: "image" may only be a valid URI/IRI' + ); + } + + if (! in_array(substr($value, -3), ['jpg', 'png'])) { + throw new Writer\Exception\InvalidArgumentException( + 'invalid parameter: "image" may only use file extension "jpg"' + . ' or "png" which must be the last three characters of the URI' + . ' (i.e. no query string or fragment)' + ); + } + + $this->data['image'] = $value; + return $this; + } + /** * Overloading to itunes specific setters * diff --git a/src/Writer/Extension/ITunes/Feed.php b/src/Writer/Extension/ITunes/Feed.php index 2acf80fc..89bf9e8f 100644 --- a/src/Writer/Extension/ITunes/Feed.php +++ b/src/Writer/Extension/ITunes/Feed.php @@ -169,7 +169,7 @@ public function setItunesCategories(array $values) */ public function setItunesImage($value) { - if (! Uri::factory($value)->isValid()) { + if (! is_string($value) || ! Uri::factory($value)->isValid()) { throw new Writer\Exception\InvalidArgumentException('invalid parameter: "image" may only' . ' be a valid URI/IRI'); } diff --git a/test/Writer/Extension/ITunes/EntryTest.php b/test/Writer/Extension/ITunes/EntryTest.php index e5e7d246..c52d8e10 100644 --- a/test/Writer/Extension/ITunes/EntryTest.php +++ b/test/Writer/Extension/ITunes/EntryTest.php @@ -188,4 +188,54 @@ public function testSetSummaryThrowsExceptionWhenValueExceeds255Chars() $entry = new Writer\Entry; $entry->setItunesSummary(str_repeat('a', 4001)); } + + public function invalidImageUrls() + { + return [ + 'null' => [null], + 'true' => [true], + 'false' => [false], + 'zero' => [0], + 'int' => [1], + 'zero-float' => [0.0], + 'float' => [1.1], + 'string' => ['scheme:/host.path'], + 'invalid-extension-gif' => ['https://example.com/image.gif', 'file extension'], + 'invalid-extension-uc' => ['https://example.com/image.PNG', 'file extension'], + 'array' => [['https://example.com/image.png']], + 'object' => [(object) ['image' => 'https://example.com/image.png']], + ]; + } + + /** + * @dataProvider invalidImageUrls + * @param mixed $url + * @param string $expectedMessage + */ + public function testSetItunesImageRaisesExceptionForInvalidUrl($url, $expectedMessage = 'valid URI') + { + $entry = new Writer\Entry(); + $this->expectException(ExceptionInterface::class); + $this->expectExceptionMessage($expectedMessage); + $entry->setItunesImage($url); + } + + public function validImageUrls() + { + return [ + 'jpg' => ['https://example.com/image.jpg'], + 'png' => ['https://example.com/image.png'], + ]; + } + + /** + * @dataProvider validImageUrls + * @param string $url + */ + public function testSetItunesImageSetsInternalDataWithValidUrl($url) + { + $entry = new Writer\Entry(); + $entry->setItunesImage($url); + $this->assertEquals($url, $entry->getItunesImage()); + } } diff --git a/test/Writer/Extension/ITunes/FeedTest.php b/test/Writer/Extension/ITunes/FeedTest.php index 67be98af..cd129b56 100644 --- a/test/Writer/Extension/ITunes/FeedTest.php +++ b/test/Writer/Extension/ITunes/FeedTest.php @@ -267,4 +267,54 @@ public function testSetSummaryThrowsExceptionWhenValueExceeds4000Chars() $feed = new Writer\Feed; $feed->setItunesSummary(str_repeat('a', 4001)); } + + public function invalidImageUrls() + { + return [ + 'null' => [null], + 'true' => [true], + 'false' => [false], + 'zero' => [0], + 'int' => [1], + 'zero-float' => [0.0], + 'float' => [1.1], + 'string' => ['scheme:/host.path'], + 'invalid-extension-gif' => ['https://example.com/image.gif', 'file extension'], + 'invalid-extension-uc' => ['https://example.com/image.PNG', 'file extension'], + 'array' => [['https://example.com/image.png']], + 'object' => [(object) ['image' => 'https://example.com/image.png']], + ]; + } + + /** + * @dataProvider invalidImageUrls + * @param mixed $url + * @param string $expectedMessage + */ + public function testSetItunesImageRaisesExceptionForInvalidUrl($url, $expectedMessage = 'valid URI') + { + $feed = new Writer\Feed(); + $this->expectException(ExceptionInterface::class); + $this->expectExceptionMessage($expectedMessage); + $feed->setItunesImage($url); + } + + public function validImageUrls() + { + return [ + 'jpg' => ['https://example.com/image.jpg'], + 'png' => ['https://example.com/image.png'], + ]; + } + + /** + * @dataProvider validImageUrls + * @param string $url + */ + public function testSetItunesImageSetsInternalDataWithValidUrl($url) + { + $feed = new Writer\Feed(); + $feed->setItunesImage($url); + $this->assertEquals($url, $feed->getItunesImage()); + } } From 99fa6e345cbc5d36709393c7a060a9254b1b963c Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 24 May 2018 10:15:05 -0500 Subject: [PATCH 3/4] Ensure reader podcast extension supports itunes:image at both feed and entry level We previously had support for itunes:image at the feed level, but not at the entry level; the spec supports both. --- src/Reader/Extension/Podcast/Entry.php | 32 ++++++++++++++++----- src/Reader/Extension/Podcast/Feed.php | 12 +++----- test/Reader/Integration/PodcastRss2Test.php | 12 ++++++++ test/Reader/Integration/_files/podcast.xml | 1 + 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/Reader/Extension/Podcast/Entry.php b/src/Reader/Extension/Podcast/Entry.php index e18bc387..6e26c6db 100644 --- a/src/Reader/Extension/Podcast/Entry.php +++ b/src/Reader/Extension/Podcast/Entry.php @@ -1,18 +1,14 @@ data['summary']; } + /** + * Get the entry image + * + * @return string + */ + public function getItunesImage() + { + if (isset($this->data['image'])) { + return $this->data['image']; + } + + $image = $this->xpath->evaluate('string(' . $this->getXpathPrefix() . '/itunes:image/@href)'); + + if (! $image) { + $image = null; + } + + $this->data['image'] = $image; + + return $this->data['image']; + } + /** * Register iTunes namespace * diff --git a/src/Reader/Extension/Podcast/Feed.php b/src/Reader/Extension/Podcast/Feed.php index d2d11cb4..d9a2bc3c 100644 --- a/src/Reader/Extension/Podcast/Feed.php +++ b/src/Reader/Extension/Podcast/Feed.php @@ -1,10 +1,8 @@ assertEquals($expected, $entry->getEnclosure()); } + + public function testCanRetrieveEntryImage() + { + $feed = Reader\Reader::importString( + file_get_contents($this->feedSamplePath) + ); + $entry = $feed->current(); + $this->assertEquals( + 'https://www.example.com/podcasts/everything/episode.png', + $entry->getItunesImage() + ); + } } diff --git a/test/Reader/Integration/_files/podcast.xml b/test/Reader/Integration/_files/podcast.xml index b754c24f..54dbfb89 100644 --- a/test/Reader/Integration/_files/podcast.xml +++ b/test/Reader/Integration/_files/podcast.xml @@ -50,6 +50,7 @@ 7:04 salt, pepper, shaker, exciting + From 7333501de41bfd43cd195e90577cca7cf7881a46 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 24 May 2018 10:19:55 -0500 Subject: [PATCH 4/4] Adds CHANGELOG entry for #77 --- CHANGELOG.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e4a36d3..e9ae7a1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,15 @@ All notable changes to this project will be documented in this file, in reverse ### Added -- Nothing. +- [#77](https://github.com/zendframework/zend-feed/pull/77) adds support for `itunes:image` for each of: + - `Zend\Feed\Reader\Extension\Podcast\Entry`, via `getItunesImage()`; previously only the `Feed` supported it. + - `Zend\Feed\Writer\Extension\ITunes\Entry`, via `setItunesImage()`; previously only the `Feed` supported it. + - `Zend\Feed\Writer\Extension\ITunes\Renderer\Entry`; previously on the `Feed` supported it. ### Changed -- Nothing. +- [#77](https://github.com/zendframework/zend-feed/pull/77) updates URI validation for `Zend\Feed\Writer\Extension\ITunes\Feed::setItunesImage()` to + first check that we have received a string value before proceeding. ### Deprecated