From 362031345eaf8355fba213959ce1a5ccf12fe95e Mon Sep 17 00:00:00 2001 From: serhii balko Date: Mon, 11 Dec 2017 18:15:00 +0200 Subject: [PATCH 1/2] #8647: [GitHub] Order of how arguments are merged in multiple di.xml-files causes unexpected results --- .../Webapi/Rest/Response/RendererFactory.php | 5 +++ .../Rest/Response/RendererFactoryTest.php | 37 +++++++++++++++---- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php b/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php index c86b97b0fa8e7..938d4ef6d1bed 100644 --- a/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php +++ b/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php @@ -71,7 +71,12 @@ protected function _getRendererClass() if (!is_array($acceptTypes)) { $acceptTypes = [$acceptTypes]; } + // If Accept type = '*/*' then return default renderer. + $defaultRenderer = isset($this->_renders['default']) ? $this->_renders['default'] : null; foreach ($acceptTypes as $acceptType) { + if ($acceptType == '*/*' && $defaultRenderer) { + return $defaultRenderer['model']; + } foreach ($this->_renders as $rendererConfig) { $rendererType = $rendererConfig['type']; if ($acceptType == $rendererType || $acceptType == current( diff --git a/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php b/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php index ba460c5a5f6e6..7b50a6f0224c0 100644 --- a/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php +++ b/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php @@ -26,11 +26,18 @@ protected function setUp() )->disableOriginalConstructor()->getMock(); $renders = [ - 'default' => ['type' => '*/*', 'model' => \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class], + 'application_xml' => [ + 'type' => 'application/xml', + 'model' => \Magento\Framework\Webapi\Rest\Response\Renderer\Xml::class, + ], 'application_json' => [ 'type' => 'application/json', 'model' => \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class, ], + 'default' => [ + 'type' => '*/*', + 'model' => \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class + ], ]; $this->_factory = new \Magento\Framework\Webapi\Rest\Response\RendererFactory( @@ -42,29 +49,43 @@ protected function setUp() /** * Test GET method. + * + * @param array $acceptTypes + * @param string $model + * @dataProvider getTestDataProvider */ - public function testGet() + public function testGet($acceptTypes, $model) { - $acceptTypes = ['application/json']; - /** Mock request getAcceptTypes method to return specified value. */ $this->_requestMock->expects($this->once())->method('getAcceptTypes')->will($this->returnValue($acceptTypes)); /** Mock renderer. */ - $rendererMock = $this->getMockBuilder( - \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class - )->disableOriginalConstructor()->getMock(); + $rendererMock = $this->getMockBuilder($model)->disableOriginalConstructor()->getMock(); /** Mock object to return mocked renderer. */ $this->_objectManagerMock->expects( $this->once() )->method( 'get' )->with( - \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class + $model )->will( $this->returnValue($rendererMock) ); $this->_factory->get(); } + + /** + * Data provider for method testGet + * + * @return array + */ + public function getTestDataProvider() + { + return [ + [['*/*'], \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class], + [['application/json'], \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class], + [['application/xml'], \Magento\Framework\Webapi\Rest\Response\Renderer\Xml::class], + ]; + } /** * Test GET method with wrong Accept HTTP Header. From cf7477ab8dfa08285c6b8ee5561b6b572baac24e Mon Sep 17 00:00:00 2001 From: serhii balko Date: Tue, 12 Dec 2017 09:40:00 +0200 Subject: [PATCH 2/2] #8647: [GitHub] Order of how arguments are merged in multiple di.xml-files causes unexpected results --- .../Webapi/Rest/Response/RendererFactory.php | 42 +++++++++++++------ .../Rest/Response/RendererFactoryTest.php | 2 +- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php b/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php index 938d4ef6d1bed..547b8fcb03640 100644 --- a/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php +++ b/lib/internal/Magento/Framework/Webapi/Rest/Response/RendererFactory.php @@ -71,20 +71,10 @@ protected function _getRendererClass() if (!is_array($acceptTypes)) { $acceptTypes = [$acceptTypes]; } - // If Accept type = '*/*' then return default renderer. - $defaultRenderer = isset($this->_renders['default']) ? $this->_renders['default'] : null; foreach ($acceptTypes as $acceptType) { - if ($acceptType == '*/*' && $defaultRenderer) { - return $defaultRenderer['model']; - } - foreach ($this->_renders as $rendererConfig) { - $rendererType = $rendererConfig['type']; - if ($acceptType == $rendererType || $acceptType == current( - explode('/', $rendererType) - ) . '/*' || $acceptType == '*/*' - ) { - return $rendererConfig['model']; - } + $renderer = $this->getRendererConfig($acceptType); + if ($renderer !== null) { + return $renderer['model']; } } /** If server does not have renderer for any of the accepted types it SHOULD send 406 (not acceptable). */ @@ -98,4 +88,30 @@ protected function _getRendererClass() \Magento\Framework\Webapi\Exception::HTTP_NOT_ACCEPTABLE ); } + + /** + * Get renderer config by accept type. + * + * @param string $acceptType + * @return array|null + */ + private function getRendererConfig($acceptType) + { + // If Accept type = '*/*' then return default renderer. + if ($acceptType == '*/*' && isset($this->_renders['default'])) { + return $this->_renders['default']; + } + + foreach ($this->_renders as $rendererConfig) { + $rendererType = $rendererConfig['type']; + if ($acceptType == $rendererType + || $acceptType == current(explode('/', $rendererType)) . '/*' + || $acceptType == '*/*' + ) { + return $rendererConfig; + } + } + + return null; + } } diff --git a/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php b/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php index 7b50a6f0224c0..dd7aa845f3091 100644 --- a/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php +++ b/lib/internal/Magento/Framework/Webapi/Test/Unit/Rest/Response/RendererFactoryTest.php @@ -35,7 +35,7 @@ protected function setUp() 'model' => \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class, ], 'default' => [ - 'type' => '*/*', + 'type' => '*/*', 'model' => \Magento\Framework\Webapi\Rest\Response\Renderer\Json::class ], ];