Skip to content

Commit a25e103

Browse files
karyna-tkaryna-t
committed
refactor code that were reported by phpcs and improve tests
1 parent cbf9754 commit a25e103

File tree

6 files changed

+49
-151
lines changed

6 files changed

+49
-151
lines changed

‎app/code/Magento/Elasticsearch/Elasticsearch5/SearchAdapter/Mapper.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ private function processFilterQuery(
201201
MatchQueryBuilder::QUERY_CONDITION_MUST_NOT : $conditionType;
202202
$filterQuery = $this->filterBuilder->build($query->getReference(), $conditionType);
203203
foreach ($filterQuery['bool'] as $condition => $filter) {
204-
$selectQuery['bool'][$condition]= array_merge(
204+
//phpcs:ignore Magento2.Performance.ForeachArrayMerge
205+
$selectQuery['bool'][$condition] = array_merge(
205206
$selectQuery['bool'][$condition] ?? [],
206207
$filter
207208
);

‎dev/tests/integration/testsuite/Magento/Framework/Search/Request/MapperTest.php

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
*/
66
namespace Magento\Framework\Search\Request;
77

8+
use Magento\Framework\Search\Request\Aggregation\DynamicBucket;
9+
use Magento\Framework\Search\Request\BucketInterface;
10+
811
class MapperTest extends \PHPUnit\Framework\TestCase
912
{
1013
/**
@@ -37,25 +40,31 @@ public function testGet()
3740
);
3841
}
3942

40-
public function testGetBuckets()
43+
public function testGetBuckets(): void
4144
{
4245
$buckets = $this->mapper->getBuckets();
43-
$this->assertCount(2, $buckets);
46+
$this->assertCount(3, $buckets);
4447

4548
$this->assertInstanceOf(\Magento\Framework\Search\Request\Aggregation\TermBucket::class, $buckets[0]);
4649
$this->assertEquals('category_bucket', $buckets[0]->getName());
4750
$this->assertEquals('category', $buckets[0]->getField());
48-
$this->assertEquals(\Magento\Framework\Search\Request\BucketInterface::TYPE_TERM, $buckets[0]->getType());
51+
$this->assertEquals(BucketInterface::TYPE_TERM, $buckets[0]->getType());
4952
$metrics = $buckets[0]->getMetrics();
5053
$this->assertInstanceOf(\Magento\Framework\Search\Request\Aggregation\Metric::class, $metrics[0]);
5154

5255
$this->assertInstanceOf(\Magento\Framework\Search\Request\Aggregation\RangeBucket::class, $buckets[1]);
5356
$this->assertEquals('price_bucket', $buckets[1]->getName());
5457
$this->assertEquals('price', $buckets[1]->getField());
55-
$this->assertEquals(\Magento\Framework\Search\Request\BucketInterface::TYPE_RANGE, $buckets[1]->getType());
58+
$this->assertEquals(BucketInterface::TYPE_RANGE, $buckets[1]->getType());
5659
$metrics = $buckets[1]->getMetrics();
5760
$ranges = $buckets[1]->getRanges();
5861
$this->assertInstanceOf(\Magento\Framework\Search\Request\Aggregation\Metric::class, $metrics[0]);
5962
$this->assertInstanceOf(\Magento\Framework\Search\Request\Aggregation\Range::class, $ranges[0]);
63+
64+
$this->assertInstanceOf(DynamicBucket::class, $buckets[2]);
65+
$this->assertEquals('dynamic_bucket', $buckets[2]->getName());
66+
$this->assertEquals('price', $buckets[2]->getField());
67+
$this->assertEquals(BucketInterface::TYPE_DYNAMIC, $buckets[2]->getType());
68+
$this->assertSame('auto', $buckets[2]->getMethod());
6069
}
6170
}

‎dev/tests/integration/testsuite/Magento/Framework/Search/_files/search_request.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
<range from="100" to=""/>
5656
</ranges>
5757
</bucket>
58+
<bucket xsi:type="dynamicBucket" method="auto" name="dynamic_bucket" field="price" />
5859
</aggregations>
5960
<from>10</from>
6061
<size>10</size>

‎dev/tests/integration/testsuite/Magento/Framework/Search/_files/search_request_config.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@
135135
],
136136
"type" => "rangeBucket",
137137
],
138+
'dynamic_bucket' => [
139+
'name' => 'dynamic_bucket',
140+
'field' => 'price',
141+
'method' => 'auto',
142+
'type' => 'dynamicBucket'
143+
]
138144
],
139145
"from" => "10",
140146
"size" => "10",

‎lib/internal/Magento/Framework/Search/Request/Mapper.php

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ public function getRootQuery()
125125
private function mapQuery($queryName)
126126
{
127127
if (!isset($this->queries[$queryName])) {
128+
// phpcs:ignore Magento2.Exceptions.DirectThrow
128129
throw new Exception('Query ' . $queryName . ' does not exist');
129130
} elseif (in_array($queryName, $this->mappedQueries)) {
130131
throw new StateException(new Phrase(
@@ -154,6 +155,7 @@ private function mapQuery($queryName)
154155
$reference = $this->mapFilter($query['filterReference'][0]['ref']);
155156
$referenceType = Filter::REFERENCE_FILTER;
156157
} else {
158+
// phpcs:ignore Magento2.Exceptions.DirectThrow
157159
throw new Exception('Reference is not provided');
158160
}
159161
$query = $this->objectManager->create(
@@ -195,6 +197,7 @@ private function mapQuery($queryName)
195197
private function mapFilter($filterName)
196198
{
197199
if (!isset($this->filters[$filterName])) {
200+
// phpcs:ignore Magento2.Exceptions.DirectThrow
198201
throw new Exception('Filter ' . $filterName . ' does not exist');
199202
} elseif (in_array($filterName, $this->mappedFilters)) {
200203
throw new StateException(
@@ -287,6 +290,8 @@ private function aggregateQueriesByType($data)
287290
}
288291

289292
/**
293+
* Validate queries and filters.
294+
*
290295
* @return void
291296
* @throws StateException
292297
*/
@@ -297,6 +302,8 @@ private function validate()
297302
}
298303

299304
/**
305+
* Check if queries are not used.
306+
*
300307
* @return void
301308
* @throws StateException
302309
*/
@@ -310,6 +317,8 @@ private function validateQueries()
310317
}
311318

312319
/**
320+
* Validate elements that are not used.
321+
*
313322
* @param array $elements
314323
* @param string[] $mappedElements
315324
* @param string $errorMessage
@@ -326,6 +335,8 @@ private function validateNotUsed($elements, $mappedElements, $errorMessage)
326335
}
327336

328337
/**
338+
* Check if filters are not used.
339+
*
329340
* @return void
330341
* @throws StateException
331342
*/
@@ -355,38 +366,19 @@ public function getBuckets()
355366
];
356367
switch ($bucketData['type']) {
357368
case BucketInterface::TYPE_TERM:
358-
$bucket = $this->objectManager->create(
359-
TermBucket::class,
360-
array_merge(
361-
$arguments,
362-
[
363-
'parameters' => array_column($bucketData['parameter'] ?? [], 'value', 'name'),
364-
],
365-
)
366-
);
369+
$arguments['parameters'] = array_column($bucketData['parameter'] ?? [], 'value', 'name');
370+
$bucket = $this->objectManager->create(TermBucket::class, $arguments);
367371
break;
368372
case BucketInterface::TYPE_RANGE:
369-
$bucket = $this->objectManager->create(
370-
RangeBucket::class,
371-
array_merge(
372-
$arguments,
373-
['ranges' => $this->mapRanges($bucketData)]
374-
)
375-
);
373+
$arguments['ranges'] = $this->mapRanges($bucketData);
374+
$bucket = $this->objectManager->create(RangeBucket::class, $arguments);
376375
break;
377376
case BucketInterface::TYPE_DYNAMIC:
378-
$bucket = $this->objectManager->create(
379-
DynamicBucket::class,
380-
array_merge(
381-
$arguments,
382-
['method' => $bucketData['method']]
383-
)
384-
);
377+
$arguments['method'] = $bucketData['method'];
378+
$bucket = $this->objectManager->create(DynamicBucket::class, $arguments);
385379
break;
386380
default:
387-
throw new StateException(new Phrase(
388-
'The bucket type is invalid. Verify and try again.'
389-
));
381+
throw new StateException(new Phrase('The bucket type is invalid. Verify and try again.'));
390382
}
391383
$buckets[] = $bucket;
392384
}

‎lib/internal/Magento/Framework/Search/Test/Unit/Request/MapperTest.php

Lines changed: 9 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ public function testGetQueryBool($queries): void
326326
'someClause' => ['someQueryMatch' => $this->queryMatch]
327327
]
328328
]
329-
)->willReturnOnConsecutiveCalls($this->queryMatch, $this->queryBool);
329+
)
330+
->willReturnOnConsecutiveCalls($this->queryMatch, $this->queryBool);
330331

331332
/** @var Mapper $mapper */
332333
$mapper = $this->helper->getObject(
@@ -988,7 +989,7 @@ public function getQueryBoolProvider(): array
988989
/**
989990
* @return void
990991
*/
991-
public function testGetBucketsTermBucket(): void
992+
public function testGetBucketsInvalidBucket(): void
992993
{
993994
$queries = [
994995
self::ROOT_QUERY => [
@@ -998,127 +999,12 @@ public function testGetBucketsTermBucket(): void
998999
'match' => 'someMatches'
9991000
]
10001001
];
1001-
1002-
$bucket = [
1003-
"name" => "category_bucket",
1004-
"field" => "category",
1005-
"metric" => [
1006-
["type" => "sum"],
1007-
["type" => "count"],
1008-
["type" => "min"],
1009-
["type" => "max"],
1010-
],
1011-
"type" => "termBucket"
1012-
];
1013-
$metricClass = Metric::class;
1014-
$bucketClass = TermBucket::class;
1015-
$queryClass = MatchQuery::class;
1016-
$queryArguments = [
1017-
'name' => $queries[self::ROOT_QUERY]['name'],
1018-
'value' => $queries[self::ROOT_QUERY]['value'],
1019-
'boost' => 1,
1020-
'matches' => $queries[self::ROOT_QUERY]['match'],
1021-
];
1022-
$arguments = [
1023-
'name' => $bucket['name'],
1024-
'field' => $bucket['field'],
1025-
'metrics' => [null, null, null, null]
1026-
];
1027-
$this->objectManager->expects($this->any())->method('create')
1028-
->withConsecutive(
1029-
[$this->equalTo($queryClass), $this->equalTo($queryArguments)],
1030-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][0]['type']])],
1031-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][1]['type']])],
1032-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][2]['type']])],
1033-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][3]['type']])],
1034-
[$this->equalTo($bucketClass), $this->equalTo($arguments)]
1035-
)
1036-
->willReturn(null);
1037-
1038-
/** @var Mapper $mapper */
1039-
$mapper = $this->helper->getObject(
1040-
Mapper::class,
1041-
[
1042-
'objectManager' => $this->objectManager,
1043-
'queries' => $queries,
1044-
'rootQueryName' => self::ROOT_QUERY,
1045-
'aggregation' => [$bucket]
1046-
]
1047-
);
1048-
$mapper->getBuckets();
1049-
}
1050-
1051-
/**
1052-
* @return void
1053-
*/
1054-
public function testGetBucketsRangeBucket(): void
1055-
{
1056-
$queries = [
1057-
self::ROOT_QUERY => [
1058-
'type' => QueryInterface::TYPE_MATCH,
1059-
'value' => 'someValue',
1060-
'name' => 'someName',
1061-
'match' => 'someMatches'
1062-
]
1063-
];
1064-
10651002
$bucket = [
10661003
"name" => "price_bucket",
10671004
"field" => "price",
1068-
"metric" => [
1069-
["type" => "sum"],
1070-
["type" => "count"],
1071-
["type" => "min"],
1072-
["type" => "max"]
1073-
],
1074-
"range" => [
1075-
["from" => "", "to" => "50"],
1076-
["from" => "50", "to" => "100"],
1077-
["from" => "100", "to" => ""]
1078-
],
1079-
"type" => "rangeBucket"
1080-
];
1081-
$metricClass = Metric::class;
1082-
$bucketClass = RangeBucket::class;
1083-
$rangeClass = \Magento\Framework\Search\Request\Aggregation\Range::class;
1084-
$queryClass = MatchQuery::class;
1085-
$queryArguments = [
1086-
'name' => $queries[self::ROOT_QUERY]['name'],
1087-
'value' => $queries[self::ROOT_QUERY]['value'],
1088-
'boost' => 1,
1089-
'matches' => $queries[self::ROOT_QUERY]['match']
1090-
];
1091-
$arguments = [
1092-
'name' => $bucket['name'],
1093-
'field' => $bucket['field'],
1094-
'metrics' => [null, null, null, null],
1095-
'ranges' => [null, null, null]
1005+
"method" => "test",
1006+
"type" => "invalidBucket"
10961007
];
1097-
$this->objectManager->expects($this->any())->method('create')
1098-
->withConsecutive(
1099-
[$this->equalTo($queryClass), $this->equalTo($queryArguments)],
1100-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][0]['type']])],
1101-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][1]['type']])],
1102-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][2]['type']])],
1103-
[$this->equalTo($metricClass), $this->equalTo(['type' => $bucket['metric'][3]['type']])],
1104-
[
1105-
$this->equalTo($rangeClass),
1106-
$this->equalTo(['from' => $bucket['range'][0]['from'], 'to' => $bucket['range'][0]['to']])
1107-
],
1108-
[
1109-
$this->equalTo($rangeClass),
1110-
$this->equalTo(['from' => $bucket['range'][1]['from'], 'to' => $bucket['range'][1]['to']])
1111-
],
1112-
[
1113-
$this->equalTo($rangeClass),
1114-
$this->equalTo(['from' => $bucket['range'][2]['from'], 'to' => $bucket['range'][2]['to']])
1115-
],
1116-
[
1117-
$this->equalTo($bucketClass),
1118-
$this->equalTo($arguments)
1119-
]
1120-
)
1121-
->willReturn(null);
11221008

11231009
/** @var Mapper $mapper */
11241010
$mapper = $this->helper->getObject(
@@ -1127,9 +1013,12 @@ public function testGetBucketsRangeBucket(): void
11271013
'objectManager' => $this->objectManager,
11281014
'queries' => $queries,
11291015
'rootQueryName' => self::ROOT_QUERY,
1130-
'aggregation' => [$bucket]
1016+
'aggregations' => [$bucket]
11311017
]
11321018
);
1019+
1020+
$this->expectException(StateException::class);
1021+
$this->expectExceptionMessage('The bucket type is invalid. Verify and try again.');
11331022
$mapper->getBuckets();
11341023
}
11351024
}

0 commit comments

Comments
 (0)