Merge pull request #192 from KnpLabs/fix/elastic-sort

Fix Elastica & Solarium sort
KnpLabs · Mar 20, 2018 · 9e8df46 · 9e8df46
2 parents b513b79 + bf57220
commit 9e8df46
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 10 deletions.
diff --git a/src/Knp/Component/Pager/Event/ItemsEvent.php b/src/Knp/Component/Pager/Event/ItemsEvent.php
@@ -57,6 +57,13 @@ public function getCustomPaginationParameters()
         return $this->customPaginationParams;
     }
 
+    public function unsetCustomPaginationParameter($name)
+    {
+        if (isset($this->customPaginationParams[$name])) {
+            unset($this->customPaginationParams[$name]);
+        }
+    }
+
     public function getLimit()
     {
         return $this->limit;

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Paginate/ElasticaQuerySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Paginate/ElasticaQuerySubscriber.php
@@ -19,16 +19,15 @@ public function items(ItemsEvent $event)
             list($searchable, $query) = $event->target;
 
             $query->setFrom($event->getOffset());
-            $query->setLimit($event->getLimit());
+            $query->setSize($event->getLimit());
             $results = $searchable->search($query);
 
             $event->count = $results->getTotalHits();
-            //Faceting is being replaced by aggregations
+
             if ($results->hasAggregations()) {
                 $event->setCustomPaginationParameter('aggregations', $results->getAggregations());
-            } elseif ($results->hasFacets()) {
-                $event->setCustomPaginationParameter('facets', $results->getFacets());
             }
+
             $event->setCustomPaginationParameter('resultSet', $results);
             $event->items = $results->getResults();
             $event->stopPropagation();

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Sortable/ArraySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Sortable/ArraySubscriber.php
@@ -36,10 +36,18 @@ public function __construct(PropertyAccessorInterface $accessor = null)
 
     public function items(ItemsEvent $event)
     {
+        // Check if the result has already been sorted by an other sort subscriber
+        $customPaginationParameters = $event->getCustomPaginationParameters();
+        if (!empty($customPaginationParameters['sorted']) ) {
+            return;
+        }
+
         if (!is_array($event->target) || empty($_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]])) {
             return;
         }
 
+        $event->setCustomPaginationParameter('sorted', true);
+
         if (isset($event->options[PaginatorInterface::SORT_FIELD_WHITELIST]) && !in_array($_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]], $event->options[PaginatorInterface::SORT_FIELD_WHITELIST])) {
             throw new \UnexpectedValueException("Cannot sort by: [{$_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]]}] this field is not in whitelist");
         }

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Sortable/Doctrine/ODM/MongoDB/QuerySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Sortable/Doctrine/ODM/MongoDB/QuerySubscriber.php
@@ -11,7 +11,15 @@ class QuerySubscriber implements EventSubscriberInterface
 {
     public function items(ItemsEvent $event)
     {
+        // Check if the result has already been sorted by an other sort subscriber
+        $customPaginationParameters = $event->getCustomPaginationParameters();
+        if (!empty($customPaginationParameters['sorted']) ) {
+            return;
+        }
+
         if ($event->target instanceof Query) {
+            $event->setCustomPaginationParameter('sorted', true);
+
             if (isset($_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]])) {
                 $field = $_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]];
                 $dir = strtolower($_GET[$event->options[PaginatorInterface::SORT_DIRECTION_PARAMETER_NAME]]) == 'asc' ? 1 : -1;

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Sortable/Doctrine/ORM/QuerySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Sortable/Doctrine/ORM/QuerySubscriber.php
@@ -13,7 +13,15 @@ class QuerySubscriber implements EventSubscriberInterface
 {
     public function items(ItemsEvent $event)
     {
+        // Check if the result has already been sorted by an other sort subscriber
+        $customPaginationParameters = $event->getCustomPaginationParameters();
+        if (!empty($customPaginationParameters['sorted']) ) {
+            return;
+        }
+
         if ($event->target instanceof Query) {
+            $event->setCustomPaginationParameter('sorted', true);
+
             if (isset($_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]])) {
                 $dir = isset($_GET[$event->options[PaginatorInterface::SORT_DIRECTION_PARAMETER_NAME]]) && strtolower($_GET[$event->options[PaginatorInterface::SORT_DIRECTION_PARAMETER_NAME]]) === 'asc' ? 'asc' : 'desc';
 

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Sortable/ElasticaQuerySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Sortable/ElasticaQuerySubscriber.php
@@ -12,7 +12,15 @@ class ElasticaQuerySubscriber implements EventSubscriberInterface
 {
     public function items(ItemsEvent $event)
     {
+        // Check if the result has already been sorted by an other sort subscriber
+        $customPaginationParameters = $event->getCustomPaginationParameters();
+        if (!empty($customPaginationParameters['sorted']) ) {
+            return;
+        }
+
         if (is_array($event->target) && 2 === count($event->target) && reset($event->target) instanceof SearchableInterface && end($event->target) instanceof Query) {
+            $event->setCustomPaginationParameter('sorted', true);
+
             list($searchable, $query) = $event->target;
 
             if (isset($_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]])) {

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Sortable/PropelQuerySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Sortable/PropelQuerySubscriber.php
@@ -10,8 +10,16 @@ class PropelQuerySubscriber implements EventSubscriberInterface
 {
     public function items(ItemsEvent $event)
     {
+        // Check if the result has already been sorted by an other sort subscriber
+        $customPaginationParameters = $event->getCustomPaginationParameters();
+        if (!empty($customPaginationParameters['sorted']) ) {
+            return;
+        }
+
         $query = $event->target;
         if ($query instanceof \ModelCriteria) {
+            $event->setCustomPaginationParameter('sorted', true);
+
             if (isset($_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]])) {
                 $part = $_GET[$event->options[PaginatorInterface::SORT_FIELD_PARAMETER_NAME]];
                 $directionParam = $event->options[PaginatorInterface::SORT_DIRECTION_PARAMETER_NAME];

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Sortable/SolariumQuerySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Sortable/SolariumQuerySubscriber.php
@@ -15,7 +15,15 @@ class SolariumQuerySubscriber implements EventSubscriberInterface
 {
     public function items(ItemsEvent $event)
     {
+        // Check if the result has already been sorted by an other sort subscriber
+        $customPaginationParameters = $event->getCustomPaginationParameters();
+        if (!empty($customPaginationParameters['sorted']) ) {
+            return;
+        }
+
         if (is_array($event->target) && 2 == count($event->target)) {
+            $event->setCustomPaginationParameter('sorted', true);
+
             $values = array_values($event->target);
             list($client, $query) = $values;
 

diff --git a/tests/Test/Pager/Subscriber/Sortable/ArraySubscriberTest.php b/tests/Test/Pager/Subscriber/Sortable/ArraySubscriberTest.php
@@ -23,12 +23,17 @@ public function shouldSort()
         $itemsEvent = new ItemsEvent(0, 10);
         $itemsEvent->target = &$array;
         $itemsEvent->options = array(PaginatorInterface::SORT_FIELD_PARAMETER_NAME => 'sort', PaginatorInterface::SORT_DIRECTION_PARAMETER_NAME => 'ord');
-        $_GET = array('sort' => '[entry][sortProperty]', 'ord' => 'asc');
 
-        $this->assertEquals(2, $array[0]['entry']['sortProperty']);
         $arraySubscriber = new ArraySubscriber();
+
+        // test asc sort
+        $_GET = array('sort' => '[entry][sortProperty]', 'ord' => 'asc');
         $arraySubscriber->items($itemsEvent);
         $this->assertEquals(1, $array[0]['entry']['sortProperty']);
+
+        $itemsEvent->unsetCustomPaginationParameter('sorted');
+
+        // test desc sort
         $_GET ['ord'] = 'desc';
         $arraySubscriber->items($itemsEvent);
         $this->assertEquals(3, $array[0]['entry']['sortProperty']);
@@ -52,20 +57,26 @@ public function shouldSortWithCustomCallback()
             PaginatorInterface::SORT_DIRECTION_PARAMETER_NAME => 'ord',
             'sortFunction' => function (&$target, $sortField, $sortDirection) {
                 usort($target, function($object1, $object2) use ($sortField, $sortDirection) {
-                    if ($object1[$sortField] == $object2[$sortField]) {
+                    if ($object1[$sortField] === $object2[$sortField]) {
                         return 0;
                     }
 
-                    return ($object1[$sortField] == 'hot' ? 1 : -1) * ($sortDirection == 'asc' ? 1 : -1);
+                    return ($object1[$sortField] === 'hot' ? 1 : -1) * ($sortDirection === 'asc' ? 1 : -1);
                 });
             },
         );
-        $_GET = array('sort' => '.name', 'ord' => 'asc');
 
-        $this->assertEquals('hot', $array[0]['name']);
         $arraySubscriber = new ArraySubscriber();
+
+        // test asc sort
+        $_GET = array('sort' => '.name', 'ord' => 'asc');
         $arraySubscriber->items($itemsEvent);
         $this->assertEquals('cold', $array[0]['name']);
+
+
+        $itemsEvent->unsetCustomPaginationParameter('sorted');
+
+        // test desc sort
         $_GET['ord'] = 'desc';
         $arraySubscriber->items($itemsEvent);
         $this->assertEquals('hot', $array[0]['name']);