Skip to content

Commit

Permalink
MC-23881: When category URL page param is larger than the pagination,…
Browse files Browse the repository at this point in the history
… the page will break with elastic search 2
  • Loading branch information
Viktor Kopin committed Jan 8, 2021
1 parent 6f79389 commit c1812b9
Show file tree
Hide file tree
Showing 4 changed files with 280 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
*/
class Builder
{
private const ELASTIC_INT_MAX = 2147483647;

/**
* @var Config
* @since 100.2.2
Expand Down Expand Up @@ -83,18 +85,18 @@ public function initQuery(RequestInterface $request)
{
$dimension = current($request->getDimensions());
$storeId = $this->scopeResolver->getScope($dimension->getValue())->getId();

$searchQuery = [
'index' => $this->searchIndexNameResolver->getIndexName($storeId, $request->getIndex()),
'type' => $this->clientConfig->getEntityType(),
'body' => [
'from' => $request->getFrom(),
'from' => $this->getFrom($request),
'size' => $request->getSize(),
'stored_fields' => ['_id', '_score'],
'sort' => $this->sortBuilder->getSort($request),
'query' => [],
],
];

return $searchQuery;
}

Expand All @@ -112,4 +114,17 @@ public function initAggregations(
) {
return $this->aggregationBuilder->build($request, $searchQuery);
}


/**
* Limit from by elastic integer max value
*
* @param RequestInterface $request
* @return int|null
*/
private function getFrom(RequestInterface $request): int
{
$requestFrom = (int)$request->getFrom();
return $requestFrom > self::ELASTIC_INT_MAX ? self::ELASTIC_INT_MAX : $requestFrom;
}
}
16 changes: 15 additions & 1 deletion app/code/Magento/Elasticsearch/SearchAdapter/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
*/
class Builder extends Elasticsearch5Builder
{
private const ELASTIC_INT_MAX = 2147483647;

/**
* @var Sort
*/
Expand Down Expand Up @@ -61,7 +63,7 @@ public function initQuery(RequestInterface $request)
'index' => $this->searchIndexNameResolver->getIndexName($storeId, $request->getIndex()),
'type' => $this->clientConfig->getEntityType(),
'body' => [
'from' => $request->getFrom(),
'from' => $this->getFrom($request),
'size' => $request->getSize(),
'fields' => ['_id', '_score'],
'sort' => $this->sortBuilder->getSort($request),
Expand All @@ -70,4 +72,16 @@ public function initQuery(RequestInterface $request)
];
return $searchQuery;
}

/**
* Limit from by elastic integer max value
*
* @param RequestInterface $request
* @return int|null
*/
private function getFrom(RequestInterface $request): int
{
$requestFrom = (int)$request->getFrom();
return $requestFrom > self::ELASTIC_INT_MAX ? self::ELASTIC_INT_MAX : $requestFrom;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Elasticsearch\Test\Unit\Elasticsearch5\SearchAdapter\Query;

use Magento\Elasticsearch\Elasticsearch5\SearchAdapter\Query\Builder;
use Magento\Elasticsearch\Model\Config;
use Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation as AggregationBuilder;
use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
use Magento\Framework\App\ScopeInterface;
use Magento\Framework\App\ScopeResolverInterface;
use Magento\Framework\Search\Request\Dimension;
use Magento\Framework\Search\RequestInterface;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

/**
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class BuilderTest extends TestCase
{
/**
* @var Builder
*/
protected $model;

/**
* @var Config|MockObject
*/
protected $clientConfig;

/**
* @var SearchIndexNameResolver|MockObject
*/
protected $searchIndexNameResolver;

/**
* @var AggregationBuilder|MockObject
*/
protected $aggregationBuilder;

/**
* @var RequestInterface|MockObject
*/
protected $request;

/**
* @var ScopeResolverInterface|MockObject
*/
protected $scopeResolver;

/**
* @var ScopeInterface|MockObject
*/
protected $scopeInterface;

/**
* Setup method
*
* @return void
*/
protected function setUp(): void
{
$this->clientConfig = $this->getMockBuilder(Config::class)
->onlyMethods(['getEntityType'])
->disableOriginalConstructor()
->getMock();
$this->searchIndexNameResolver = $this
->getMockBuilder(SearchIndexNameResolver::class)
->onlyMethods(['getIndexName'])
->disableOriginalConstructor()
->getMock();
$this->aggregationBuilder = $this
->getMockBuilder(\Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation::class)
->onlyMethods(['build'])
->disableOriginalConstructor()
->getMock();
$this->request = $this->getMockBuilder(RequestInterface::class)
->disableOriginalConstructor()
->getMockForAbstractClass();
$this->scopeResolver = $this->getMockForAbstractClass(
ScopeResolverInterface::class,
[],
'',
false
);
$this->scopeInterface = $this->getMockForAbstractClass(
ScopeInterface::class,
[],
'',
false
);

$this->model = new Builder(
$this->clientConfig,
$this->searchIndexNameResolver,
$this->aggregationBuilder,
$this->scopeResolver,
);
}

/**
* Test initQuery() method
*/
public function testInitQuery()
{
$dimensionValue = 1;
$dimension = $this->getMockBuilder(Dimension::class)
->onlyMethods(['getValue'])
->disableOriginalConstructor()
->getMock();

$this->request->expects($this->once())
->method('getDimensions')
->willReturn([$dimension]);
$dimension->expects($this->once())
->method('getValue')
->willReturn($dimensionValue);
$this->scopeResolver->expects($this->once())
->method('getScope')
->willReturn($this->scopeInterface);
$this->scopeInterface->expects($this->once())
->method('getId')
->willReturn($dimensionValue);
$this->request->expects($this->once())
->method('getFrom')
->willReturn(0);
$this->request->expects($this->once())
->method('getSize')
->willReturn(10);
$this->request->expects($this->once())
->method('getIndex')
->willReturn('catalogsearch_fulltext');
$this->searchIndexNameResolver->expects($this->once())
->method('getIndexName')
->willReturn('indexName');
$this->clientConfig->expects($this->once())
->method('getEntityType')
->willReturn('document');
$this->model->initQuery($this->request);
}

/**
* Test initQuery() method with update from value
*/
public function testInitQueryLimitFrom()
{
$dimensionValue = 1;
$dimension = $this->getMockBuilder(Dimension::class)
->onlyMethods(['getValue'])
->disableOriginalConstructor()
->getMock();

$this->request->expects($this->once())
->method('getDimensions')
->willReturn([$dimension]);
$dimension->expects($this->once())
->method('getValue')
->willReturn($dimensionValue);
$this->scopeResolver->expects($this->once())
->method('getScope')
->willReturn($this->scopeInterface);
$this->scopeInterface->expects($this->once())
->method('getId')
->willReturn($dimensionValue);
$this->request->expects($this->once())
->method('getFrom')
->willReturn(PHP_INT_MAX);
$this->request->expects($this->once())
->method('getSize')
->willReturn(10);
$this->request->expects($this->once())
->method('getIndex')
->willReturn('catalogsearch_fulltext');
$this->searchIndexNameResolver->expects($this->once())
->method('getIndexName')
->willReturn('indexName');
$this->clientConfig->expects($this->once())
->method('getEntityType')
->willReturn('document');
$query = $this->model->initQuery($this->request);
$this->assertLessThanOrEqual(2147483647, $query['body']['from']);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Magento\Elasticsearch\Model\Config;
use Magento\Elasticsearch\SearchAdapter\Query\Builder;
use Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation as AggregationBuilder;
use Magento\Elasticsearch\SearchAdapter\Query\Builder\Sort;
use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
use Magento\Framework\App\ScopeInterface;
use Magento\Framework\App\ScopeResolverInterface;
Expand Down Expand Up @@ -66,17 +67,17 @@ class BuilderTest extends TestCase
protected function setUp(): void
{
$this->clientConfig = $this->getMockBuilder(Config::class)
->setMethods(['getEntityType'])
->onlyMethods(['getEntityType'])
->disableOriginalConstructor()
->getMock();
$this->searchIndexNameResolver = $this
->getMockBuilder(SearchIndexNameResolver::class)
->setMethods(['getIndexName'])
->onlyMethods(['getIndexName'])
->disableOriginalConstructor()
->getMock();
$this->aggregationBuilder = $this
->getMockBuilder(\Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation::class)
->setMethods(['build'])
->onlyMethods(['build'])
->disableOriginalConstructor()
->getMock();
$this->request = $this->getMockBuilder(RequestInterface::class)
Expand All @@ -94,16 +95,19 @@ protected function setUp(): void
'',
false
);
$sortBuilder = $this->getMockForAbstractClass(
Sort::class,
[],
'',
false
);

$objectManagerHelper = new ObjectManagerHelper($this);
$this->model = $objectManagerHelper->getObject(
Builder::class,
[
'clientConfig' => $this->clientConfig,
'searchIndexNameResolver' => $this->searchIndexNameResolver,
'aggregationBuilder' => $this->aggregationBuilder,
'scopeResolver' => $this->scopeResolver
]
$this->model = new Builder(
$this->clientConfig,
$this->searchIndexNameResolver,
$this->aggregationBuilder,
$this->scopeResolver,
$sortBuilder,
);
}

Expand All @@ -114,7 +118,7 @@ public function testInitQuery()
{
$dimensionValue = 1;
$dimension = $this->getMockBuilder(Dimension::class)
->setMethods(['getValue'])
->onlyMethods(['getValue'])
->disableOriginalConstructor()
->getMock();

Expand Down Expand Up @@ -148,6 +152,48 @@ public function testInitQuery()
$this->model->initQuery($this->request);
}

/**
* Test initQuery() method with update from value
*/
public function testInitQueryLimitFrom()
{
$dimensionValue = 1;
$dimension = $this->getMockBuilder(Dimension::class)
->onlyMethods(['getValue'])
->disableOriginalConstructor()
->getMock();

$this->request->expects($this->once())
->method('getDimensions')
->willReturn([$dimension]);
$dimension->expects($this->once())
->method('getValue')
->willReturn($dimensionValue);
$this->scopeResolver->expects($this->once())
->method('getScope')
->willReturn($this->scopeInterface);
$this->scopeInterface->expects($this->once())
->method('getId')
->willReturn($dimensionValue);
$this->request->expects($this->once())
->method('getFrom')
->willReturn(PHP_INT_MAX);
$this->request->expects($this->once())
->method('getSize')
->willReturn(10);
$this->request->expects($this->once())
->method('getIndex')
->willReturn('catalogsearch_fulltext');
$this->searchIndexNameResolver->expects($this->once())
->method('getIndexName')
->willReturn('indexName');
$this->clientConfig->expects($this->once())
->method('getEntityType')
->willReturn('document');
$query = $this->model->initQuery($this->request);
$this->assertLessThanOrEqual(2147483647, $query['body']['from']);
}

/**
* Test initQuery() method
*/
Expand Down

0 comments on commit c1812b9

Please sign in to comment.