-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Zend feed refactoring #9347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zend feed refactoring #9347
Changes from 21 commits
ca9e582
ed33b18
c7ba65a
9d0a290
109047f
4534a2e
c413389
caa4471
66154e6
49bcf98
c0b4c7a
411d0fc
9779ebe
eba92d0
983d74f
0b51e8c
74d431a
7f73d67
2933223
64610d3
d2d397a
0770227
735c840
7e17d20
46c51ba
3a3683f
6311d5c
4462eaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ | |
| use Magento\Framework\App\ObjectManager; | ||
| use Magento\Framework\App\Rss\DataProviderInterface; | ||
| use Magento\Framework\Serialize\SerializerInterface; | ||
| use Magento\Framework\App\FeedInterface; | ||
| use Magento\Framework\App\FeedFactoryInterface; | ||
|
|
||
| /** | ||
| * Provides functionality to work with RSS feeds | ||
|
|
@@ -27,6 +29,11 @@ class Rss | |
| */ | ||
| protected $cache; | ||
|
|
||
| /** | ||
| * @var \Magento\Framework\App\FeedFactoryInterface | ||
| */ | ||
| private $feedFactory; | ||
|
|
||
| /** | ||
| * @var SerializerInterface | ||
| */ | ||
|
|
@@ -37,13 +44,16 @@ class Rss | |
| * | ||
| * @param \Magento\Framework\App\CacheInterface $cache | ||
| * @param SerializerInterface|null $serializer | ||
| * @param FeedFactoryInterface|null $feedFactory | ||
| */ | ||
| public function __construct( | ||
| \Magento\Framework\App\CacheInterface $cache, | ||
| SerializerInterface $serializer = null | ||
| SerializerInterface $serializer = null, | ||
| FeedFactoryInterface $feedFactory = null | ||
| ) { | ||
| $this->cache = $cache; | ||
| $this->serializer = $serializer ?: ObjectManager::getInstance()->get(SerializerInterface::class); | ||
| $this->feedFactory = $feedFactory ?: ObjectManager::getInstance()->get(FeedFactoryInterface::class); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -92,7 +102,13 @@ public function setDataProvider(DataProviderInterface $dataProvider) | |
| */ | ||
| public function createRssXml() | ||
| { | ||
| $rssFeedFromArray = \Zend_Feed::importArray($this->getFeeds(), 'rss'); | ||
| return $rssFeedFromArray->saveXML(); | ||
| $feed = $this->feedFactory->create( | ||
| $this->getFeeds(), | ||
| FeedFactoryInterface::DEFAULT_FORMAT | ||
| ); | ||
|
|
||
| return $feed->getFormattedContentAs( | ||
| FeedInterface::DEFAULT_FORMAT | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ class RssTest extends \PHPUnit\Framework\TestCase | |
| /** | ||
| * @var array | ||
| */ | ||
| protected $feedData = [ | ||
| private $feedData = [ | ||
| 'title' => 'Feed Title', | ||
| 'link' => 'http://magento.com/rss/link', | ||
| 'description' => 'Feed Description', | ||
|
|
@@ -33,6 +33,27 @@ class RssTest extends \PHPUnit\Framework\TestCase | |
| ], | ||
| ]; | ||
|
|
||
| /** | ||
| * @var string | ||
| */ | ||
| private $feedXml = '<?xml version="1.0" encoding="UTF-8"?> | ||
| <rss xmlns:content="http://purl.org/rss/1.0/modules/content/" version="2.0"> | ||
| <channel> | ||
| <title><![CDATA[Feed Title]]></title> | ||
| <link>http://magento.com/rss/link</link> | ||
| <description><![CDATA[Feed Description]]></description> | ||
| <pubDate>Sat, 22 Apr 2017 13:21:12 +0200</pubDate> | ||
| <generator>Zend_Feed</generator> | ||
| <docs>http://blogs.law.harvard.edu/tech/rss</docs> | ||
| <item> | ||
| <title><![CDATA[Feed 1 Title]]></title> | ||
| <link>http://magento.com/rss/link/id/1</link> | ||
| <description><![CDATA[Feed 1 Description]]></description> | ||
| <pubDate>Sat, 22 Apr 2017 13:21:12 +0200</pubDate> | ||
| </item> | ||
| </channel> | ||
| </rss>'; | ||
|
|
||
| /** | ||
| * @var ObjectManagerHelper | ||
| */ | ||
|
|
@@ -43,6 +64,16 @@ class RssTest extends \PHPUnit\Framework\TestCase | |
| */ | ||
| private $cacheMock; | ||
|
|
||
| /** | ||
| * @var \Magento\Framework\App\FeedFactoryInterface|\PHPUnit_Framework_MockObject_MockObject | ||
| */ | ||
| private $feedFactoryMock; | ||
|
|
||
| /** | ||
| * @var \Magento\Framework\App\FeedInterface|\PHPUnit_Framework_MockObject_MockObject | ||
| */ | ||
| private $feedMock; | ||
|
|
||
| /** | ||
| * @var SerializerInterface|\PHPUnit_Framework_MockObject_MockObject | ||
| */ | ||
|
|
@@ -52,11 +83,15 @@ protected function setUp() | |
| { | ||
| $this->cacheMock = $this->createMock(\Magento\Framework\App\CacheInterface::class); | ||
| $this->serializerMock = $this->createMock(SerializerInterface::class); | ||
| $this->feedFactoryMock = $this->createMock(\Magento\Framework\App\FeedFactoryInterface::class); | ||
| $this->feedMock = $this->createMock(\Magento\Framework\App\FeedInterface::class); | ||
|
|
||
| $this->objectManagerHelper = new ObjectManagerHelper($this); | ||
| $this->rss = $this->objectManagerHelper->getObject( | ||
| \Magento\Rss\Model\Rss::class, | ||
| [ | ||
| 'cache' => $this->cacheMock, | ||
| 'feedFactory' => $this->feedFactoryMock, | ||
| 'serializer' => $this->serializerMock | ||
| ] | ||
| ); | ||
|
|
@@ -116,6 +151,16 @@ public function testCreateRssXml() | |
| $dataProvider->expects($this->any())->method('getCacheLifetime')->will($this->returnValue(100)); | ||
| $dataProvider->expects($this->any())->method('getRssData')->will($this->returnValue($this->feedData)); | ||
|
|
||
| $this->feedMock->expects($this->once()) | ||
| ->method('getFormattedContentAs') | ||
| ->with(\Magento\Framework\App\FeedInterface::DEFAULT_FORMAT) | ||
| ->will($this->returnValue($this->feedXml)); | ||
|
|
||
| $this->feedFactoryMock->expects($this->once()) | ||
| ->method('create') | ||
| ->with($this->feedData, \Magento\Framework\App\FeedFactoryInterface::DEFAULT_FORMAT) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ->will($this->returnValue($this->feedMock)); | ||
|
|
||
| $this->rss->setDataProvider($dataProvider); | ||
| $result = $this->rss->createRssXml(); | ||
| $this->assertContains('<?xml version="1.0" encoding="UTF-8"?>', $result); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| <?php | ||
| /** | ||
| * Copyright © Magento, Inc. All rights reserved. | ||
| * See COPYING.txt for license details. | ||
| */ | ||
| namespace Magento\Framework\App; | ||
|
|
||
| /** | ||
| * Default XML feed class | ||
| */ | ||
| class Feed implements FeedInterface | ||
| { | ||
| /** | ||
| * @param Zend_Feed $feed | ||
| * @param array $data | ||
| */ | ||
| public function __construct( | ||
| \Zend_Feed $feed, | ||
| array $data | ||
| ) { | ||
| $this->feed = $feed; | ||
| $this->data = $data; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the formatted feed content | ||
| * | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| * @return string | ||
| */ | ||
| public function getFormattedContentAs( | ||
| $format = self::FORMAT_XML | ||
| ) { | ||
| $feed = $this->feed::importArray( | ||
| $this->data, | ||
| FeedFactoryInterface::FORMAT_RSS | ||
| ); | ||
| return $this->feed->saveXml(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| <?php | ||
| /** | ||
| * Copyright © Magento, Inc. All rights reserved. | ||
| * See COPYING.txt for license details. | ||
| */ | ||
| namespace Magento\Framework\App; | ||
|
|
||
| use Magento\Framework\App\FeedFactoryInterface; | ||
| use Magento\Framework\ObjectManagerInterface; | ||
| use Psr\Log\LoggerInterface; | ||
|
|
||
| /** | ||
| * Feed factory | ||
| */ | ||
| class FeedFactory implements FeedFactoryInterface | ||
| { | ||
| /** | ||
| * @var FeedProcessorInterface | ||
| */ | ||
| private $feedProcessor; | ||
|
|
||
| /** | ||
| * @var LoggerInterface | ||
| */ | ||
| private $logger; | ||
|
|
||
| /** | ||
| * @var ObjectManagerInterface | ||
| */ | ||
| private $objectManager; | ||
|
|
||
| /** | ||
| * @param ObjectManagerInterface $objectManger | ||
| * @param LoggerInterface $logger | ||
| * @param array $formats | ||
| */ | ||
| public function __construct( | ||
| ObjectManagerInterface $objectManger, | ||
| LoggerInterface $logger, | ||
| array $formats | ||
| ) { | ||
| $this->objectManager = $objectManger; | ||
| $this->logger = $logger; | ||
| $this->formats = $formats; | ||
| } | ||
|
|
||
| /** | ||
| * Get a new \Magento\Framework\App\FeedInterface object from a custom array | ||
| * | ||
| * @throws \Magento\Framework\Exception\InputException | ||
| * @param array $data | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interitdoc should be used here |
||
| * @param string $format | ||
| * @return \Magento\Framework\App\FeedInterface | ||
| */ | ||
| public function create( | ||
| array $data, | ||
| $format = FeedFactoryInterface::FORMAT_RSS | ||
| ) { | ||
| if (!isset($this->formats[$format])) { | ||
| throw new \Magento\Framework\Exception\InputException( | ||
| __('The format is not supported'), | ||
| $e | ||
| ); | ||
| } | ||
|
|
||
| if (!is_subclass_of($this->formats[$format], '\Magento\Framework\App\FeedInterface')) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of |
||
| throw new \Magento\Framework\Exception\InputException( | ||
| __('Wrong format handler type'), | ||
| $e | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| return $this->objectManager->create( | ||
| $this->formats[$format], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to validate in constructor that actually all $this->formats suppose to be successors of \Magento\Framework\App\FeedInterface |
||
| $data | ||
| ); | ||
| } catch (\Exception $e) { | ||
| $this->logger->error($e->getMessage()); | ||
| throw new \Magento\Framework\Exception\RuntimeException( | ||
| __('There has been an error with import'), | ||
| $e | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <?php | ||
| /** | ||
| * Copyright © Magento, Inc. All rights reserved. | ||
| * See COPYING.txt for license details. | ||
| */ | ||
| namespace Magento\Framework\App; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interface description should be added |
||
| interface FeedFactoryInterface | ||
| { | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please provide a description of this constant. |
||
| const FORMAT_RSS = 'rss'; | ||
|
|
||
| /** | ||
| * Returns FeedInterface object from a custom array | ||
| * | ||
| * @throws \Magento\Framework\Exception\InputException | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| * @param array $data | ||
| * @param string $format | ||
| * @return FeedInterface | ||
| */ | ||
| public function create( | ||
| array $data, | ||
| $format = self::FORMAT_RSS | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| <?php | ||
| /** | ||
| * Copyright © Magento, Inc. All rights reserved. | ||
| * See COPYING.txt for license details. | ||
| */ | ||
| namespace Magento\Framework\App; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please provide description for the interface |
||
| interface FeedInterface | ||
| { | ||
| const FORMAT_XML = 'xml'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please provide description of the constant |
||
|
|
||
| /** | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please provide description of input data in PHP Doc Block |
||
| * @return string | ||
| */ | ||
| public function getFormattedContentAs( | ||
| $format = self::FORMAT_XML | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, you are referencing undefined constant, as you changed the name of this constant in FeedFactoryInterface
That's why you have Red Unit and Static tests: