Skip to content

Commit 4c2d6f5

Browse files
committed
MAGETWO-90863: Error handling responses from MBI
1 parent 824b922 commit 4c2d6f5

File tree

7 files changed

+42
-20
lines changed

7 files changed

+42
-20
lines changed

app/code/Magento/Analytics/Model/Connector/Http/ConverterInterface.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,9 @@ public function toBody(array $data);
3030
* @return string
3131
*/
3232
public function getContentTypeHeader();
33+
34+
/**
35+
* @return string
36+
*/
37+
public function getContentMediaType(): string ;
3338
}

app/code/Magento/Analytics/Model/Connector/Http/JsonConverter.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento\Analytics\Model\Connector\Http;
79

810
use Magento\Framework\Serialize\Serializer\Json;
@@ -14,9 +16,16 @@ class JsonConverter implements ConverterInterface
1416
{
1517
/**
1618
* Content-Type HTTP header for json.
19+
* @deprecated
20+
* @see CONTENT_MEDIA_TYPE
1721
*/
1822
const CONTENT_TYPE_HEADER = 'Content-Type: application/json';
1923

24+
/**
25+
* Media-Type corresponding to this converter.
26+
*/
27+
const CONTENT_MEDIA_TYPE = 'application/json';
28+
2029
/**
2130
* @var Json
2231
*/
@@ -56,6 +65,14 @@ public function toBody(array $data)
5665
*/
5766
public function getContentTypeHeader()
5867
{
59-
return self::CONTENT_TYPE_HEADER;
68+
return sprintf('Content-Type: %s', self::CONTENT_MEDIA_TYPE);
69+
}
70+
71+
/**
72+
* @inheritdoc
73+
*/
74+
public function getContentMediaType(): string
75+
{
76+
return self::CONTENT_MEDIA_TYPE;
6077
}
6178
}

app/code/Magento/Analytics/Model/Connector/Http/ResponseResolver.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ public function __construct(ConverterInterface $converter, array $responseHandle
3838
public function getResult(\Zend_Http_Response $response)
3939
{
4040
$result = false;
41-
preg_match('#(?:Content-Type:\s*)(\w\S+)#i', $this->converter->getContentTypeHeader(), $contentType);
42-
$converterContentType = $contentType[1];
41+
$converterMediaType = $this->converter->getContentMediaType();
4342

44-
if ($response->getBody() && is_int(strripos($response->getHeader('Content-Type'), $converterContentType))) {
43+
/** Content-Type header may not only contain media-type declaration */
44+
if ($response->getBody() && is_int(strripos($response->getHeader('Content-Type'), $converterMediaType))) {
4545
$responseBody = $this->converter->fromBody($response->getBody());
4646
} else {
4747
$responseBody = [];

app/code/Magento/Analytics/Test/Unit/Model/Connector/Http/Client/CurlTest.php

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ public function getTestData()
9797
'version' => '1.1',
9898
'body'=> ['name' => 'value'],
9999
'url' => 'http://www.mystore.com',
100-
'headers' => [JsonConverter::CONTENT_TYPE_HEADER],
101100
'method' => \Magento\Framework\HTTP\ZendClient::POST,
102101
]
103102
]
@@ -118,7 +117,7 @@ public function testRequestSuccess(array $data)
118117
$data['method'],
119118
$data['url'],
120119
$data['version'],
121-
$data['headers'],
120+
[$this->converterMock->getContentTypeHeader()],
122121
json_encode($data['body'])
123122
);
124123
$this->curlAdapterMock->expects($this->once())
@@ -139,7 +138,7 @@ public function testRequestSuccess(array $data)
139138
$data['method'],
140139
$data['url'],
141140
$data['body'],
142-
$data['headers'],
141+
[$this->converterMock->getContentTypeHeader()],
143142
$data['version']
144143
)
145144
);
@@ -158,7 +157,7 @@ public function testRequestError(array $data)
158157
$data['method'],
159158
$data['url'],
160159
$data['version'],
161-
$data['headers'],
160+
[$this->converterMock->getContentTypeHeader()],
162161
json_encode($data['body'])
163162
);
164163
$this->curlAdapterMock->expects($this->once())
@@ -184,7 +183,7 @@ public function testRequestError(array $data)
184183
$data['method'],
185184
$data['url'],
186185
$data['body'],
187-
$data['headers'],
186+
[$this->converterMock->getContentTypeHeader()],
188187
$data['version']
189188
)
190189
);
@@ -195,14 +194,13 @@ public function testRequestError(array $data)
195194
*/
196195
private function createJsonConverter()
197196
{
198-
$converterMock = $this->getMockBuilder(ConverterInterface::class)
199-
->getMockForAbstractClass();
197+
$converterMock = $this->getMockBuilder(JsonConverter::class)
198+
->setMethodsExcept(['getContentTypeHeader'])
199+
->disableOriginalConstructor()
200+
->getMock();
200201
$converterMock->expects($this->any())->method('toBody')->willReturnCallback(function ($value) {
201202
return json_encode($value);
202203
});
203-
$converterMock->expects($this->any())
204-
->method('getContentTypeHeader')
205-
->willReturn(JsonConverter::CONTENT_TYPE_HEADER);
206204
return $converterMock;
207205
}
208206
}

app/code/Magento/Analytics/Test/Unit/Model/Connector/Http/JsonConverterTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ protected function setUp()
3939

4040
public function testConverterContainsHeader()
4141
{
42-
$this->assertEquals(JsonConverter::CONTENT_TYPE_HEADER, $this->converter->getContentTypeHeader());
42+
$this->assertEquals(
43+
'Content-Type: ' . JsonConverter::CONTENT_MEDIA_TYPE,
44+
$this->converter->getContentTypeHeader()
45+
);
4346
}
4447

4548
/**

app/code/Magento/Analytics/Test/Unit/Model/Connector/Http/ResponseResolverTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ public function testGetResultHandleResponseSuccess()
7373
$expectedBody = ['test' => 'testValue'];
7474
$response = new \Zend_Http_Response(201, ['Content-Type' => 'application/json'], json_encode($expectedBody));
7575
$this->converterMock
76-
->method('getContentTypeHeader')
77-
->willReturn('Content-Type: application/json');
76+
->method('getContentMediaType')
77+
->willReturn('application/json');
7878

7979
$this->successResponseHandlerMock
8080
->expects($this->once())
@@ -99,8 +99,8 @@ public function testGetResultHandleResponseUnexpectedContentType()
9999
$expectedBody = 'testString';
100100
$response = new \Zend_Http_Response(201, ['Content-Type' => 'plain/text'], $expectedBody);
101101
$this->converterMock
102-
->method('getContentTypeHeader')
103-
->willReturn('Content-Type: application/json');
102+
->method('getContentMediaType')
103+
->willReturn('application/json');
104104
$this->converterMock
105105
->expects($this->never())
106106
->method('fromBody');

app/code/Magento/Analytics/Test/Unit/Model/Connector/SignUpCommandTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ private function getTestData()
163163
'url' => 'http://www.mystore.com',
164164
'access-token' => 'thisisaccesstoken',
165165
'integration-token' => 'thisisintegrationtoken',
166-
'headers' => [JsonConverter::CONTENT_TYPE_HEADER],
167166
'method' => \Magento\Framework\HTTP\ZendClient::POST,
168167
'body'=> ['token' => 'thisisintegrationtoken','url' => 'http://www.mystore.com'],
169168
];

0 commit comments

Comments
 (0)