Skip to content

Commit ad2649c

Browse files
committed
MC-35407: Advance Reporting generated csv files are not properly escaped which causes reports to fail on MBI side.
1 parent cc7f549 commit ad2649c

File tree

2 files changed

+39
-18
lines changed

2 files changed

+39
-18
lines changed

app/code/Magento/Analytics/Model/ReportWriter.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
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;
79

810
use Magento\Analytics\ReportXml\DB\ReportValidator;
911
use Magento\Framework\Filesystem\Directory\WriteInterface;
1012

1113
/**
1214
* Writes reports in files in csv format
13-
* @inheritdoc
1415
*/
1516
class ReportWriter implements ReportWriterInterface
1617
{
@@ -54,7 +55,7 @@ public function __construct(
5455
}
5556

5657
/**
57-
* {@inheritdoc}
58+
* @inheritdoc
5859
*/
5960
public function write(WriteInterface $directory, $path)
6061
{
@@ -81,7 +82,7 @@ public function write(WriteInterface $directory, $path)
8182
$headers = array_keys($row);
8283
$stream->writeCsv($headers);
8384
}
84-
$stream->writeCsv($row);
85+
$stream->writeCsv($this->prepareRow($row));
8586
}
8687
$stream->unlock();
8788
$stream->close();
@@ -98,4 +99,18 @@ public function write(WriteInterface $directory, $path)
9899

99100
return true;
100101
}
102+
103+
/**
104+
* Replace wrong symbols in row
105+
*
106+
* @param array $row
107+
* @return array
108+
*/
109+
private function prepareRow(array $row): array
110+
{
111+
$row = preg_replace('/(?<!\\\\)"/', '\\"', $row);
112+
$row = preg_replace('/[\\\\]+/', '\\', $row);
113+
114+
return $row;
115+
}
101116
}

app/code/Magento/Analytics/Test/Unit/Model/ReportWriterTest.php

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
use Magento\Analytics\Model\ReportWriter;
1313
use Magento\Analytics\ReportXml\DB\ReportValidator;
1414
use Magento\Analytics\ReportXml\ReportProvider;
15-
use Magento\Framework\Filesystem\Directory\WriteInterface;
15+
use Magento\Framework\Filesystem\Directory\WriteInterface as DirectoryWriteInterface;
16+
use Magento\Framework\Filesystem\File\WriteInterface as FileWriteInterface;
1617
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
1718
use PHPUnit\Framework\MockObject\MockObject;
1819
use PHPUnit\Framework\TestCase;
@@ -48,7 +49,7 @@ class ReportWriterTest extends TestCase
4849
private $objectManagerHelper;
4950

5051
/**
51-
* @var WriteInterface|MockObject
52+
* @var DirectoryWriteInterface|MockObject
5253
*/
5354
private $directoryMock;
5455

@@ -82,7 +83,7 @@ protected function setUp(): void
8283
$this->reportValidatorMock = $this->createMock(ReportValidator::class);
8384
$this->providerFactoryMock = $this->createMock(ProviderFactory::class);
8485
$this->reportProviderMock = $this->createMock(ReportProvider::class);
85-
$this->directoryMock = $this->getMockBuilder(WriteInterface::class)
86+
$this->directoryMock = $this->getMockBuilder(DirectoryWriteInterface::class)
8687
->getMockForAbstractClass();
8788
$this->objectManagerHelper = new ObjectManagerHelper($this);
8889

@@ -98,16 +99,15 @@ protected function setUp(): void
9899

99100
/**
100101
* @param array $configData
102+
* @param array $fileData
103+
* @param array $expectedFileData
101104
* @return void
102105
*
103106
* @dataProvider configDataProvider
104107
*/
105-
public function testWrite(array $configData)
108+
public function testWrite(array $configData, array $fileData, array $expectedFileData): void
106109
{
107110
$errors = [];
108-
$fileData = [
109-
['number' => 1, 'type' => 'Shoes Usual']
110-
];
111111
$this->configInterfaceMock
112112
->expects($this->once())
113113
->method('get')
@@ -126,7 +126,7 @@ public function testWrite(array $configData)
126126
->with($parameterName ?: null)
127127
->willReturn($fileData);
128128
$errorStreamMock = $this->getMockBuilder(
129-
\Magento\Framework\Filesystem\File\WriteInterface::class
129+
FileWriteInterface::class
130130
)->getMockForAbstractClass();
131131
$errorStreamMock
132132
->expects($this->once())
@@ -136,8 +136,8 @@ public function testWrite(array $configData)
136136
->expects($this->exactly(2))
137137
->method('writeCsv')
138138
->withConsecutive(
139-
[array_keys($fileData[0])],
140-
[$fileData[0]]
139+
[array_keys($expectedFileData[0])],
140+
[$expectedFileData[0]]
141141
);
142142
$errorStreamMock->expects($this->once())->method('unlock');
143143
$errorStreamMock->expects($this->once())->method('close');
@@ -164,12 +164,12 @@ public function testWrite(array $configData)
164164
*
165165
* @dataProvider configDataProvider
166166
*/
167-
public function testWriteErrorFile($configData)
167+
public function testWriteErrorFile(array $configData): void
168168
{
169169
$errors = ['orders', 'SQL Error: test'];
170170
$this->configInterfaceMock->expects($this->once())->method('get')->willReturn([$configData]);
171171
$errorStreamMock = $this->getMockBuilder(
172-
\Magento\Framework\Filesystem\File\WriteInterface::class
172+
FileWriteInterface::class
173173
)->getMockForAbstractClass();
174174
$errorStreamMock->expects($this->once())->method('lock');
175175
$errorStreamMock->expects($this->once())->method('writeCsv')->with($errors);
@@ -184,7 +184,7 @@ public function testWriteErrorFile($configData)
184184
/**
185185
* @return void
186186
*/
187-
public function testWriteEmptyReports()
187+
public function testWriteEmptyReports(): void
188188
{
189189
$this->configInterfaceMock->expects($this->once())->method('get')->willReturn([]);
190190
$this->reportValidatorMock->expects($this->never())->method('validate');
@@ -195,11 +195,11 @@ public function testWriteEmptyReports()
195195
/**
196196
* @return array
197197
*/
198-
public function configDataProvider()
198+
public function configDataProvider(): array
199199
{
200200
return [
201201
'reportProvider' => [
202-
[
202+
'configData' => [
203203
'providers' => [
204204
[
205205
'name' => $this->providerName,
@@ -209,6 +209,12 @@ public function configDataProvider()
209209
],
210210
]
211211
]
212+
],
213+
'fileData' => [
214+
['number' => 1, 'type' => 'Shoes\"" Usual\\\\"']
215+
],
216+
'expectedFileData' => [
217+
['number' => 1, 'type' => 'Shoes\"\" Usual\\"']
212218
]
213219
],
214220
];

0 commit comments

Comments
 (0)