Skip to content

Commit ea41340

Browse files
author
vpaladiychuk
committed
MAGETWO-2204: "HEADERS ALREADY SENT" When Controller Action Outputs Directly
1 parent 34d941b commit ea41340

File tree

5 files changed

+188
-130
lines changed

5 files changed

+188
-130
lines changed

‎app/code/Magento/Backup/Controller/Adminhtml/Index/Download.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function execute()
7373

7474
$fileName = $this->_objectManager->get('Magento\Backup\Helper\Data')->generateBackupDownloadName($backup);
7575

76-
$this->_response = $this->_fileFactory->create(
76+
$this->_fileFactory->create(
7777
$fileName,
7878
null,
7979
DirectoryList::VAR_DIR,

‎app/code/Magento/Customer/Controller/Adminhtml/Index/Viewfile.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,6 @@ public function execute()
162162
throw new NotFoundException();
163163
}
164164

165-
/** @var \Magento\Framework\Controller\Result\Raw $resultRaw */
166-
$resultRaw = $this->resultRawFactory->create();
167165
if ($plain) {
168166
$extension = pathinfo($path, PATHINFO_EXTENSION);
169167
switch (strtolower($extension)) {
@@ -184,20 +182,22 @@ public function execute()
184182
$contentLength = $stat['size'];
185183
$contentModify = $stat['mtime'];
186184

185+
/** @var \Magento\Framework\Controller\Result\Raw $resultRaw */
186+
$resultRaw = $this->resultRawFactory->create();
187187
$resultRaw->setHttpResponseCode(200)
188188
->setHeader('Pragma', 'public', true)
189189
->setHeader('Content-type', $contentType, true)
190190
->setHeader('Content-Length', $contentLength)
191191
->setHeader('Last-Modified', date('r', $contentModify));
192192
$resultRaw->setContents($directory->readFile($fileName));
193+
return $resultRaw;
193194
} else {
194195
$name = pathinfo($path, PATHINFO_BASENAME);
195-
$this->_response = $this->_fileFactory->create(
196+
$this->_fileFactory->create(
196197
$name,
197198
['type' => 'filename', 'value' => $fileName],
198199
DirectoryList::MEDIA
199200
);
200201
}
201-
return $resultRaw;
202202
}
203203
}

‎dev/tests/unit/testsuite/Magento/Backup/Controller/Adminhtml/Index/DownloadTest.php

Lines changed: 61 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,46 @@
11
<?php
22
/**
3-
* @copyright Copyright (c) 2014 X.commerce, Inc. (http://www.magentocommerce.com)
3+
* Copyright © 2015 Magento. All rights reserved.
4+
* See COPYING.txt for license details.
45
*/
56
namespace Magento\Backup\Controller\Adminhtml\Index;
67

78
class DownloadTest extends \PHPUnit_Framework_TestCase
89
{
10+
/**
11+
* @var \Magento\Framework\App\ResponseInterface|\PHPUnit_Framework_MockObject_MockObject
12+
*/
13+
protected $response;
14+
15+
/**
16+
* @var \Magento\Backup\Model\BackupFactory|\PHPUnit_Framework_MockObject_MockObject
17+
*/
18+
protected $backupModelFactory;
19+
20+
/**
21+
* @var \Magento\Backup\Model\Backup|\PHPUnit_Framework_MockObject_MockObject
22+
*/
23+
protected $backup;
24+
25+
/**
26+
* @var \Magento\Framework\App\RequestInterface||\PHPUnit_Framework_MockObject_MockObject
27+
*/
28+
protected $request;
29+
30+
public function setUp()
31+
{
32+
$this->backup = $this->getMock(
33+
'\Magento\Backup\Model\Backup',
34+
['getTime', 'exists', 'getSize', 'output'],
35+
[],
36+
'',
37+
false
38+
);
39+
$this->request = $this->getMock('\Magento\Framework\App\RequestInterface', [], [], '', false);
40+
$this->backupModelFactory = $this->getMock('\Magento\Backup\Model\BackupFactory', [], [], '', false);
41+
$this->response = $this->getMock('\Magento\Framework\App\ResponseInterface', [], [], '', false);
42+
}
43+
944
public function testExecuteBackupFound()
1045
{
1146
$time = 1;
@@ -14,44 +49,33 @@ public function testExecuteBackupFound()
1449
$size = 10;
1550
$output = 'test';
1651

17-
$backup = $this->getMock(
18-
'\Magento\Backup\Model\Backup',
19-
['getTime', 'exists', 'getSize', 'output'],
20-
[],
21-
'',
22-
false
23-
);
24-
$backup->expects($this->once())->method('getTime')->will($this->returnValue($time));
25-
$backup->expects($this->once())->method('exists')->will($this->returnValue(true));
26-
$backup->expects($this->once())->method('getSize')->will($this->returnValue($size));
27-
$backup->expects($this->once())->method('output')->will($this->returnValue($output));
52+
$this->backup->expects($this->once())->method('getTime')->will($this->returnValue($time));
53+
$this->backup->expects($this->once())->method('exists')->will($this->returnValue(true));
54+
$this->backup->expects($this->once())->method('getSize')->will($this->returnValue($size));
55+
$this->backup->expects($this->once())->method('output')->will($this->returnValue($output));
2856

29-
$request = $this->getMock('\Magento\Framework\App\RequestInterface', [], [], '', false);
30-
$request->expects($this->at(0))->method('getParam')->with('time')->will($this->returnValue($time));
31-
$request->expects($this->at(1))->method('getParam')->with('type')->will($this->returnValue($type));
57+
$this->request->expects($this->at(0))->method('getParam')->with('time')->will($this->returnValue($time));
58+
$this->request->expects($this->at(1))->method('getParam')->with('type')->will($this->returnValue($type));
3259

33-
$backupModelFactory = $this->getMock('\Magento\Backup\Model\BackupFactory', [], [], '', false);
34-
$backupModelFactory->expects($this->once())->method('create')->with($time, $type)
35-
->will($this->returnValue($backup));
60+
$this->backupModelFactory->expects($this->once())->method('create')->with($time, $type)
61+
->will($this->returnValue($this->backup));
3662

3763
$helper = $this->getMock('Magento\Backup\Helper\Data', [], [], '', false);
38-
$helper->expects($this->once())->method('generateBackupDownloadName')->with($backup)
64+
$helper->expects($this->once())->method('generateBackupDownloadName')->with($this->backup)
3965
->will($this->returnValue($filename));
4066

4167
$objectManager = $this->getMock('\Magento\Framework\ObjectManagerInterface', [], [], '', false);
4268
$objectManager->expects($this->once())->method('get')->with('Magento\Backup\Helper\Data')
4369
->will($this->returnValue($helper));
4470

45-
$response = $this->getMock('\Magento\Framework\App\ResponseInterface', [], [], '', false);
46-
4771
$fileFactory = $this->getMock('\Magento\Framework\App\Response\Http\FileFactory', [], [], '', false);
4872
$fileFactory->expects($this->once())->method('create')->with(
4973
$filename,
5074
null,
5175
\Magento\Framework\App\Filesystem\DirectoryList::VAR_DIR,
5276
'application/octet-stream',
5377
$size
54-
)->will($this->returnValue($response));
78+
)->will($this->returnValue($this->response));
5579

5680
$resultRaw = $this->getMock('\Magento\Framework\Controller\Result\Raw', [], [], '', false);
5781
$resultRaw->expects($this->once())->method('setContents')->with($output);
@@ -66,15 +90,15 @@ public function testExecuteBackupFound()
6690
$resultRawFactory->expects($this->once())->method('create')->will($this->returnValue($resultRaw));
6791

6892
$context = $this->getMock('\Magento\Backend\App\Action\Context', [], [], '', false);
69-
$context->expects($this->once())->method('getRequest')->will($this->returnValue($request));
93+
$context->expects($this->once())->method('getRequest')->will($this->returnValue($this->request));
7094
$context->expects($this->once())->method('getObjectManager')->will($this->returnValue($objectManager));
71-
$context->expects($this->once())->method('getResponse')->will($this->returnValue($response));
95+
$context->expects($this->once())->method('getResponse')->will($this->returnValue($this->response));
7296

7397
/** @var Download|\PHPUnit_Framework_MockObject_MockObject $controller */
7498
$controller = (new \Magento\TestFramework\Helper\ObjectManager($this))->getObject(
7599
'Magento\Backup\Controller\Adminhtml\Index\Download',
76100
[
77-
'backupModelFactory' => $backupModelFactory,
101+
'backupModelFactory' => $this->backupModelFactory,
78102
'context' => $context,
79103
'fileFactory' => $fileFactory,
80104
'resultRawFactory' => $resultRawFactory
@@ -84,7 +108,7 @@ public function testExecuteBackupFound()
84108
}
85109

86110
/**
87-
* @dataProvider providerExecuteBackupNotFound
111+
* @dataProvider executeBackupNotFoundDataProvider
88112
* @param string $time
89113
* @param bool $exists
90114
* @param \PHPUnit_Framework_MockObject_Matcher_InvokedCount $existsCount
@@ -93,23 +117,19 @@ public function testExecuteBackupNotFound($time, $exists, $existsCount)
93117
{
94118
$type = 'db';
95119

96-
$backup = $this->getMock('\Magento\Backup\Model\Backup', ['getTime', 'exists'], [], '', false);
97-
$backup->expects($this->once())->method('getTime')->will($this->returnValue($time));
98-
$backup->expects($existsCount)->method('exists')->will($this->returnValue($exists));
99-
100-
$response = $this->getMock('\Magento\Framework\App\ResponseInterface', [], [], '', false);
120+
$this->backup->expects($this->once())->method('getTime')->will($this->returnValue($time));
121+
$this->backup->expects($existsCount)->method('exists')->will($this->returnValue($exists));
101122

102-
$request = $this->getMock('\Magento\Framework\App\RequestInterface', [], [], '', false);
103-
$request->expects($this->at(0))->method('getParam')->with('time')->will($this->returnValue($time));
104-
$request->expects($this->at(1))->method('getParam')->with('type')->will($this->returnValue($type));
123+
$this->request = $this->getMock('\Magento\Framework\App\RequestInterface', [], [], '', false);
124+
$this->request->expects($this->at(0))->method('getParam')->with('time')->will($this->returnValue($time));
125+
$this->request->expects($this->at(1))->method('getParam')->with('type')->will($this->returnValue($type));
105126

106127
$context = $this->getMock('\Magento\Backend\App\Action\Context', [], [], '', false);
107-
$context->expects($this->once())->method('getRequest')->will($this->returnValue($request));
108-
$context->expects($this->once())->method('getResponse')->will($this->returnValue($response));
128+
$context->expects($this->once())->method('getRequest')->will($this->returnValue($this->request));
129+
$context->expects($this->once())->method('getResponse')->will($this->returnValue($this->response));
109130

110-
$backupModelFactory = $this->getMock('\Magento\Backup\Model\BackupFactory', [], [], '', false);
111-
$backupModelFactory->expects($this->once())->method('create')->with($time, $type)
112-
->will($this->returnValue($backup));
131+
$this->backupModelFactory->expects($this->once())->method('create')->with($time, $type)
132+
->will($this->returnValue($this->backup));
113133

114134
$resultRedirect = $this->getMock('Magento\Backend\Model\View\Result\Redirect', [], [], '', false);
115135
$resultRedirect->expects($this->once())->method('setPath')->with('backup/*');
@@ -128,7 +148,7 @@ public function testExecuteBackupNotFound($time, $exists, $existsCount)
128148
'Magento\Backup\Controller\Adminhtml\Index\Download',
129149
[
130150
'context' => $context,
131-
'backupModelFactory' => $backupModelFactory,
151+
'backupModelFactory' => $this->backupModelFactory,
132152
'resultRedirectFactory' => $resultRedirectFactory
133153
]
134154
);
@@ -138,7 +158,7 @@ public function testExecuteBackupNotFound($time, $exists, $existsCount)
138158
/**
139159
* @return array
140160
*/
141-
public function providerExecuteBackupNotFound()
161+
public function executeBackupNotFoundDataProvider()
142162
{
143163
return [
144164
[1, false, $this->once()],

‎dev/tests/unit/testsuite/Magento/Backup/Model/BackupTest.php

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,4 @@ public function testOutput()
4848
$backup->setTime($time);
4949
$this->assertEquals($contents, $backup->output());
5050
}
51-
52-
public function provider()
53-
{
54-
return [
55-
[true, ]
56-
];
57-
}
58-
}
51+
}

0 commit comments

Comments
 (0)