Skip to content

Commit 512f4bd

Browse files
author
Viktor Kopin
committed
MC-23881: When category URL page param is larger than the pagination, the page will break with elastic search 2
1 parent 63434d2 commit 512f4bd

File tree

4 files changed

+255
-16
lines changed

4 files changed

+255
-16
lines changed

‎app/code/Magento/Elasticsearch/Elasticsearch5/SearchAdapter/Query/Builder.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
*/
2323
class Builder
2424
{
25+
private const ELASTIC_INT_MAX = 2147483647;
26+
2527
/**
2628
* @var Config
2729
* @since 100.2.2
@@ -83,18 +85,18 @@ public function initQuery(RequestInterface $request)
8385
{
8486
$dimension = current($request->getDimensions());
8587
$storeId = $this->scopeResolver->getScope($dimension->getValue())->getId();
86-
8788
$searchQuery = [
8889
'index' => $this->searchIndexNameResolver->getIndexName($storeId, $request->getIndex()),
8990
'type' => $this->clientConfig->getEntityType(),
9091
'body' => [
91-
'from' => $request->getFrom(),
92+
'from' => min(self::ELASTIC_INT_MAX, $request->getFrom()),
9293
'size' => $request->getSize(),
9394
'stored_fields' => ['_id', '_score'],
9495
'sort' => $this->sortBuilder->getSort($request),
9596
'query' => [],
9697
],
9798
];
99+
98100
return $searchQuery;
99101
}
100102

‎app/code/Magento/Elasticsearch/SearchAdapter/Query/Builder.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
*/
2424
class Builder extends Elasticsearch5Builder
2525
{
26+
private const ELASTIC_INT_MAX = 2147483647;
27+
2628
/**
2729
* @var Sort
2830
*/
@@ -61,7 +63,7 @@ public function initQuery(RequestInterface $request)
6163
'index' => $this->searchIndexNameResolver->getIndexName($storeId, $request->getIndex()),
6264
'type' => $this->clientConfig->getEntityType(),
6365
'body' => [
64-
'from' => $request->getFrom(),
66+
'from' => min(self::ELASTIC_INT_MAX, $request->getFrom()),
6567
'size' => $request->getSize(),
6668
'fields' => ['_id', '_score'],
6769
'sort' => $this->sortBuilder->getSort($request),

‎app/code/Magento/Elasticsearch/Test/Unit/Elasticsearch5/SearchAdapter/Query/BuilderTest.php

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
<?php
2+
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace Magento\Elasticsearch\Test\Unit\Elasticsearch5\SearchAdapter\Query;
10+
11+
use Magento\Elasticsearch\Elasticsearch5\SearchAdapter\Query\Builder;
12+
use Magento\Elasticsearch\Model\Config;
13+
use Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation as AggregationBuilder;
14+
use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
15+
use Magento\Framework\App\ScopeInterface;
16+
use Magento\Framework\App\ScopeResolverInterface;
17+
use Magento\Framework\Search\Request\Dimension;
18+
use Magento\Framework\Search\RequestInterface;
19+
use PHPUnit\Framework\MockObject\MockObject;
20+
use PHPUnit\Framework\TestCase;
21+
22+
/**
23+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
24+
*/
25+
class BuilderTest extends TestCase
26+
{
27+
/**
28+
* @var Builder
29+
*/
30+
protected $model;
31+
32+
/**
33+
* @var Config|MockObject
34+
*/
35+
protected $clientConfig;
36+
37+
/**
38+
* @var SearchIndexNameResolver|MockObject
39+
*/
40+
protected $searchIndexNameResolver;
41+
42+
/**
43+
* @var AggregationBuilder|MockObject
44+
*/
45+
protected $aggregationBuilder;
46+
47+
/**
48+
* @var RequestInterface|MockObject
49+
*/
50+
protected $request;
51+
52+
/**
53+
* @var ScopeResolverInterface|MockObject
54+
*/
55+
protected $scopeResolver;
56+
57+
/**
58+
* @var ScopeInterface|MockObject
59+
*/
60+
protected $scopeInterface;
61+
62+
/**
63+
* Setup method
64+
*
65+
* @return void
66+
*/
67+
protected function setUp(): void
68+
{
69+
$this->clientConfig = $this->getMockBuilder(Config::class)
70+
->onlyMethods(['getEntityType'])
71+
->disableOriginalConstructor()
72+
->getMock();
73+
$this->searchIndexNameResolver = $this
74+
->getMockBuilder(SearchIndexNameResolver::class)
75+
->onlyMethods(['getIndexName'])
76+
->disableOriginalConstructor()
77+
->getMock();
78+
$this->aggregationBuilder = $this
79+
->getMockBuilder(\Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation::class)
80+
->onlyMethods(['build'])
81+
->disableOriginalConstructor()
82+
->getMock();
83+
$this->request = $this->getMockBuilder(RequestInterface::class)
84+
->disableOriginalConstructor()
85+
->getMockForAbstractClass();
86+
$this->scopeResolver = $this->getMockForAbstractClass(
87+
ScopeResolverInterface::class,
88+
[],
89+
'',
90+
false
91+
);
92+
$this->scopeInterface = $this->getMockForAbstractClass(
93+
ScopeInterface::class,
94+
[],
95+
'',
96+
false
97+
);
98+
99+
$this->model = new Builder(
100+
$this->clientConfig,
101+
$this->searchIndexNameResolver,
102+
$this->aggregationBuilder,
103+
$this->scopeResolver,
104+
);
105+
}
106+
107+
/**
108+
* Test initQuery() method
109+
*/
110+
public function testInitQuery()
111+
{
112+
$dimensionValue = 1;
113+
$dimension = $this->getMockBuilder(Dimension::class)
114+
->onlyMethods(['getValue'])
115+
->disableOriginalConstructor()
116+
->getMock();
117+
118+
$this->request->expects($this->once())
119+
->method('getDimensions')
120+
->willReturn([$dimension]);
121+
$dimension->expects($this->once())
122+
->method('getValue')
123+
->willReturn($dimensionValue);
124+
$this->scopeResolver->expects($this->once())
125+
->method('getScope')
126+
->willReturn($this->scopeInterface);
127+
$this->scopeInterface->expects($this->once())
128+
->method('getId')
129+
->willReturn($dimensionValue);
130+
$this->request->expects($this->once())
131+
->method('getFrom')
132+
->willReturn(0);
133+
$this->request->expects($this->once())
134+
->method('getSize')
135+
->willReturn(10);
136+
$this->request->expects($this->once())
137+
->method('getIndex')
138+
->willReturn('catalogsearch_fulltext');
139+
$this->searchIndexNameResolver->expects($this->once())
140+
->method('getIndexName')
141+
->willReturn('indexName');
142+
$this->clientConfig->expects($this->once())
143+
->method('getEntityType')
144+
->willReturn('document');
145+
$this->model->initQuery($this->request);
146+
}
147+
148+
/**
149+
* Test initQuery() method with update from value
150+
*/
151+
public function testInitQueryLimitFrom()
152+
{
153+
$dimensionValue = 1;
154+
$dimension = $this->getMockBuilder(Dimension::class)
155+
->onlyMethods(['getValue'])
156+
->disableOriginalConstructor()
157+
->getMock();
158+
159+
$this->request->expects($this->once())
160+
->method('getDimensions')
161+
->willReturn([$dimension]);
162+
$dimension->expects($this->once())
163+
->method('getValue')
164+
->willReturn($dimensionValue);
165+
$this->scopeResolver->expects($this->once())
166+
->method('getScope')
167+
->willReturn($this->scopeInterface);
168+
$this->scopeInterface->expects($this->once())
169+
->method('getId')
170+
->willReturn($dimensionValue);
171+
$this->request->expects($this->once())
172+
->method('getFrom')
173+
->willReturn(PHP_INT_MAX);
174+
$this->request->expects($this->once())
175+
->method('getSize')
176+
->willReturn(10);
177+
$this->request->expects($this->once())
178+
->method('getIndex')
179+
->willReturn('catalogsearch_fulltext');
180+
$this->searchIndexNameResolver->expects($this->once())
181+
->method('getIndexName')
182+
->willReturn('indexName');
183+
$this->clientConfig->expects($this->once())
184+
->method('getEntityType')
185+
->willReturn('document');
186+
$query = $this->model->initQuery($this->request);
187+
$this->assertLessThanOrEqual(2147483647, $query['body']['from']);
188+
}
189+
}

‎app/code/Magento/Elasticsearch/Test/Unit/SearchAdapter/Query/BuilderTest.php

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Magento\Elasticsearch\Model\Config;
1111
use Magento\Elasticsearch\SearchAdapter\Query\Builder;
1212
use Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation as AggregationBuilder;
13+
use Magento\Elasticsearch\SearchAdapter\Query\Builder\Sort;
1314
use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
1415
use Magento\Framework\App\ScopeInterface;
1516
use Magento\Framework\App\ScopeResolverInterface;
@@ -66,17 +67,17 @@ class BuilderTest extends TestCase
6667
protected function setUp(): void
6768
{
6869
$this->clientConfig = $this->getMockBuilder(Config::class)
69-
->setMethods(['getEntityType'])
70+
->onlyMethods(['getEntityType'])
7071
->disableOriginalConstructor()
7172
->getMock();
7273
$this->searchIndexNameResolver = $this
7374
->getMockBuilder(SearchIndexNameResolver::class)
74-
->setMethods(['getIndexName'])
75+
->onlyMethods(['getIndexName'])
7576
->disableOriginalConstructor()
7677
->getMock();
7778
$this->aggregationBuilder = $this
7879
->getMockBuilder(\Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation::class)
79-
->setMethods(['build'])
80+
->onlyMethods(['build'])
8081
->disableOriginalConstructor()
8182
->getMock();
8283
$this->request = $this->getMockBuilder(RequestInterface::class)
@@ -94,16 +95,19 @@ protected function setUp(): void
9495
'',
9596
false
9697
);
98+
$sortBuilder = $this->getMockForAbstractClass(
99+
Sort::class,
100+
[],
101+
'',
102+
false
103+
);
97104

98-
$objectManagerHelper = new ObjectManagerHelper($this);
99-
$this->model = $objectManagerHelper->getObject(
100-
Builder::class,
101-
[
102-
'clientConfig' => $this->clientConfig,
103-
'searchIndexNameResolver' => $this->searchIndexNameResolver,
104-
'aggregationBuilder' => $this->aggregationBuilder,
105-
'scopeResolver' => $this->scopeResolver
106-
]
105+
$this->model = new Builder(
106+
$this->clientConfig,
107+
$this->searchIndexNameResolver,
108+
$this->aggregationBuilder,
109+
$this->scopeResolver,
110+
$sortBuilder,
107111
);
108112
}
109113

@@ -114,7 +118,7 @@ public function testInitQuery()
114118
{
115119
$dimensionValue = 1;
116120
$dimension = $this->getMockBuilder(Dimension::class)
117-
->setMethods(['getValue'])
121+
->onlyMethods(['getValue'])
118122
->disableOriginalConstructor()
119123
->getMock();
120124

@@ -148,6 +152,48 @@ public function testInitQuery()
148152
$this->model->initQuery($this->request);
149153
}
150154

155+
/**
156+
* Test initQuery() method with update from value
157+
*/
158+
public function testInitQueryLimitFrom()
159+
{
160+
$dimensionValue = 1;
161+
$dimension = $this->getMockBuilder(Dimension::class)
162+
->onlyMethods(['getValue'])
163+
->disableOriginalConstructor()
164+
->getMock();
165+
166+
$this->request->expects($this->once())
167+
->method('getDimensions')
168+
->willReturn([$dimension]);
169+
$dimension->expects($this->once())
170+
->method('getValue')
171+
->willReturn($dimensionValue);
172+
$this->scopeResolver->expects($this->once())
173+
->method('getScope')
174+
->willReturn($this->scopeInterface);
175+
$this->scopeInterface->expects($this->once())
176+
->method('getId')
177+
->willReturn($dimensionValue);
178+
$this->request->expects($this->once())
179+
->method('getFrom')
180+
->willReturn(PHP_INT_MAX);
181+
$this->request->expects($this->once())
182+
->method('getSize')
183+
->willReturn(10);
184+
$this->request->expects($this->once())
185+
->method('getIndex')
186+
->willReturn('catalogsearch_fulltext');
187+
$this->searchIndexNameResolver->expects($this->once())
188+
->method('getIndexName')
189+
->willReturn('indexName');
190+
$this->clientConfig->expects($this->once())
191+
->method('getEntityType')
192+
->willReturn('document');
193+
$query = $this->model->initQuery($this->request);
194+
$this->assertLessThanOrEqual(2147483647, $query['body']['from']);
195+
}
196+
151197
/**
152198
* Test initQuery() method
153199
*/

0 commit comments

Comments
 (0)