Skip to content

Commit f8e4a0d

Browse files
committed
MAGETWO-70279: Issue with the config merging introduced for the Analytics integration
1 parent d64fe60 commit f8e4a0d

File tree

2 files changed

+114
-36
lines changed

2 files changed

+114
-36
lines changed

app/code/Magento/Analytics/ReportXml/DB/ColumnsResolver.php

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
namespace Magento\Analytics\ReportXml\DB;
88

9+
use Magento\Framework\App\ResourceConnection;
910
use Magento\Framework\DB\Sql\ColumnValueExpression;
1011

1112
/**
@@ -20,14 +21,39 @@ class ColumnsResolver
2021
*/
2122
private $nameResolver;
2223

24+
/**
25+
* @var ResourceConnection
26+
*/
27+
private $resourceConnection;
28+
29+
/**
30+
* @var \Magento\Framework\DB\Adapter\AdapterInterface
31+
*/
32+
private $connection;
33+
2334
/**
2435
* ColumnsResolver constructor.
2536
* @param NameResolver $nameResolver
2637
*/
2738
public function __construct(
28-
NameResolver $nameResolver
39+
NameResolver $nameResolver,
40+
ResourceConnection $resourceConnection
2941
) {
3042
$this->nameResolver = $nameResolver;
43+
$this->resourceConnection = $resourceConnection;
44+
}
45+
46+
/**
47+
* Returns connection
48+
*
49+
* @return \Magento\Framework\DB\Adapter\AdapterInterface
50+
*/
51+
private function getConnection()
52+
{
53+
if (!$this->connection) {
54+
$this->connection = $this->resourceConnection->getConnection();
55+
}
56+
return $this->connection;
3157
}
3258

3359
/**
@@ -54,7 +80,9 @@ public function getColumns(SelectBuilder $selectBuilder, $entityConfig)
5480
$prefix = ' DISTINCT ';
5581
}
5682
$expression = new ColumnValueExpression(
57-
strtoupper($attributeData['function']) . '(' . $prefix . $tableAlias . '.' . $columnName . ')'
83+
strtoupper($attributeData['function']) . '(' . $prefix
84+
. $this->getConnection()->quoteIdentifier($tableAlias . '.' . $columnName)
85+
. ')'
5886
);
5987
} else {
6088
$expression = $tableAlias . '.' . $columnName;

app/code/Magento/Analytics/Test/Unit/ReportXml/DB/ColumnsResolverTest.php

Lines changed: 84 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
use Magento\Analytics\ReportXml\DB\NameResolver;
1010
use Magento\Analytics\ReportXml\DB\SelectBuilder;
1111
use Magento\Framework\DB\Sql\ColumnValueExpression;
12+
use Magento\Framework\App\ResourceConnection;
13+
use Magento\Framework\DB\Adapter\AdapterInterface;
14+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
1215

1316
/**
1417
* Class ColumnsResolverTest
@@ -30,20 +33,45 @@ class ColumnsResolverTest extends \PHPUnit_Framework_TestCase
3033
*/
3134
private $columnsResolver;
3235

36+
/**
37+
* @var ResourceConnection|\PHPUnit_Framework_MockObject_MockObject
38+
*/
39+
private $resourceConnectionMock;
40+
41+
/**
42+
* @var AdapterInterface|\PHPUnit_Framework_MockObject_MockObject
43+
*/
44+
private $connectionMock;
45+
3346
/**
3447
* @return void
3548
*/
3649
protected function setUp()
3750
{
38-
$this->nameResolverMock = $this->getMockBuilder(NameResolver::class)
51+
// $this->nameResolverMock = $this->getMockBuilder(NameResolver::class)
52+
// ->disableOriginalConstructor()
53+
// ->getMock();
54+
55+
$this->selectBuilderMock = $this->getMockBuilder(SelectBuilder::class)
3956
->disableOriginalConstructor()
4057
->getMock();
4158

42-
$this->selectBuilderMock = $this->getMockBuilder(SelectBuilder::class)
59+
$this->resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class)
4360
->disableOriginalConstructor()
4461
->getMock();
4562

46-
$this->columnsResolver = new ColumnsResolver($this->nameResolverMock);
63+
$this->connectionMock = $this->getMockBuilder(AdapterInterface::class)
64+
->disableOriginalConstructor()
65+
->getMock();
66+
67+
$objectManager = new ObjectManagerHelper($this);
68+
$this->columnsResolver = $objectManager->getObject(
69+
ColumnsResolver::class,
70+
[
71+
'nameResolver' => new NameResolver(),
72+
'resourceConnection' => $this->resourceConnectionMock
73+
]
74+
);
4775
}
4876

4977
public function testGetColumnsWithoutAttributes()
@@ -54,41 +82,32 @@ public function testGetColumnsWithoutAttributes()
5482
/**
5583
* @dataProvider getColumnsDataProvider
5684
*/
57-
public function testGetColumns($expression, $attributeData)
85+
public function testGetColumnsWithFunction($expectedColumns, $expectedGroup, $entityConfig)
5886
{
59-
$columnAlias = 'fn';
60-
$expr = new ColumnValueExpression($expression);
61-
$expectedResult = [$columnAlias => $expr];
62-
$columns = [$columnAlias => 'name'];
63-
$entityConfig['attribute'] = ['attribute1' => $attributeData];
87+
$this->resourceConnectionMock->expects($this->any())
88+
->method('getConnection')
89+
->willReturn($this->connectionMock);
90+
$this->connectionMock->expects($this->any())
91+
->method('quoteIdentifier')
92+
->with('cpe.name')
93+
->willReturn('`cpe`.`name`');
94+
95+
// $expr = new ColumnValueExpression($expression);
6496
$this->selectBuilderMock->expects($this->once())
6597
->method('getColumns')
66-
->willReturn($columns);
67-
$this->nameResolverMock->expects($this->at(0))
68-
->method('getAlias')
69-
->with($attributeData)
70-
->willReturn($columnAlias);
71-
$this->nameResolverMock->expects($this->at(1))
72-
->method('getAlias')
73-
->with($entityConfig)
74-
->willReturn($columnAlias);
75-
$this->nameResolverMock->expects($this->once())
76-
->method('getName')
77-
->with($attributeData)
78-
->willReturn('name');
79-
$group = ['g'];
98+
->willReturn([]);
8099
$this->selectBuilderMock->expects($this->once())
81100
->method('getGroup')
82-
->willReturn($group);
101+
->willReturn([]);
83102
$this->selectBuilderMock->expects($this->once())
84103
->method('setGroup')
85-
->with(array_merge($group, $expectedResult));
104+
->with($expectedGroup);
86105
$this->assertEquals(
106+
$expectedColumns,
87107
$this->columnsResolver->getColumns(
88108
$this->selectBuilderMock,
89109
$entityConfig
90-
),
91-
$expectedResult
110+
)
92111
);
93112
}
94113

@@ -98,14 +117,45 @@ public function testGetColumns($expression, $attributeData)
98117
public function getColumnsDataProvider()
99118
{
100119
return [
101-
'TestWithFunction' => [
102-
'expression' => "SUM( DISTINCT fn.name)",
103-
'attributeData' => ['adata1', 'function' => 'SUM', 'distinct' => true, 'group' => true],
120+
'COUNT( DISTINCT `cpe`.`name`) AS name' => [
121+
'expectedColumns' => [
122+
'name' => new ColumnValueExpression('COUNT( DISTINCT `cpe`.`name`)')
123+
],
124+
'expectedGroup' => [
125+
'name' => new ColumnValueExpression('COUNT( DISTINCT `cpe`.`name`)')
126+
],
127+
'entityConfig' =>
128+
[
129+
'name' => 'catalog_product_entity',
130+
'alias' => 'cpe',
131+
'attribute' => [
132+
[
133+
'name' => 'name',
134+
'function' => 'COUNT',
135+
'distinct' => true,
136+
'group' => true
137+
]
138+
],
139+
],
140+
],
141+
'AVG(`cpe`.`name`) AS avg_name' => [
142+
'expectedColumns' => [
143+
'avg_name' => new ColumnValueExpression('AVG(`cpe`.`name`)')
104144
],
105-
'TestWithoutFunction' => [
106-
'expression' => "fn.name",
107-
'attributeData' => ['adata1', 'distinct' => true, 'group' => true],
108-
],
145+
'expectedGroup' => [],
146+
'entityConfig' =>
147+
[
148+
'name' => 'catalog_product_entity',
149+
'alias' => 'cpe',
150+
'attribute' => [
151+
[
152+
'name' => 'name',
153+
'alias' => 'avg_name',
154+
'function' => 'AVG',
155+
]
156+
],
157+
],
158+
]
109159
];
110160
}
111161
}

0 commit comments

Comments
 (0)