Fix sort bug when no existing class property is specified

KnpLabs · Mar 8, 2018 · 2b53de8 · 2b53de8
1 parent b513b79
commit 2b53de8
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 0 deletions.
diff --git a/src/Knp/Component/Pager/Event/Subscriber/Sortable/ArraySubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Sortable/ArraySubscriber.php
@@ -78,6 +78,27 @@ private function sortFunction($object1, $object2)
             throw new \UnexpectedValueException('You need symfony/property-access component to use this sorting function');
         }
 
+        // @see https://bugs.php.net/bug.php?id=50688
+        // To avoid this problem in PHP 5.x we can't use PropertyAccessor::isReadable()
+        if (version_compare(PHP_VERSION, '7.0', '<')) {
+            if (is_object($object1)) {
+                // snake to camelcase
+                $sortFieldName = ucfirst(strtr(ucwords(strtr($this->currentSortingField, array('_' => ' '))), array(' ' => '')));
+                $currentSortingFieldGetter = 'get' . $sortFieldName;
+                // Getter detection
+                $class = new \ReflectionClass(get_class($object1));
+                if (!$class->hasMethod($currentSortingFieldGetter)) {
+                    return 0;
+                }
+            }
+        } else {
+            if (!$this->propertyAccessor->isReadable($object1, $this->currentSortingField) ||
+                !$this->propertyAccessor->isReadable($object2, $this->currentSortingField)
+            ) {
+                return 0;
+            }
+        }
+
         $fieldValue1 = $this->propertyAccessor->getValue($object1, $this->currentSortingField);
         $fieldValue2 = $this->propertyAccessor->getValue($object2, $this->currentSortingField);
 

diff --git a/tests/Test/Pager/Subscriber/Sortable/ArraySubscriberTest.php b/tests/Test/Pager/Subscriber/Sortable/ArraySubscriberTest.php
@@ -7,6 +7,29 @@
 use Test\Tool\BaseTestCase;
 use Knp\Component\Pager\PaginatorInterface;
 
+class TestClass
+{
+    private $sortProperty;
+
+    /**
+     * TestClass constructor.
+     *
+     * @param $sortProperty
+     */
+    public function __construct($sortProperty)
+    {
+        $this->sortProperty = $sortProperty;
+    }
+
+    /**
+     * @return mixed
+     */
+    public function getSortProperty()
+    {
+        return $this->sortProperty;
+    }
+}
+
 class ArraySubscriberTest extends BaseTestCase
 {
     /**
@@ -34,6 +57,32 @@ public function shouldSort()
         $this->assertEquals(3, $array[0]['entry']['sortProperty']);
     }
 
+    /**
+     * @test
+     */
+    public function classShouldBeSort()
+    {
+        /** @var TestClass[] $array */
+        $array = array(
+            new TestClass(2),
+            new TestClass(3),
+            new TestClass(1),
+        );
+
+        $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' => 'sortProperty', 'ord' => 'asc');
+
+        $this->assertEquals(2, $array[0]->getSortProperty());
+        $arraySubscriber = new ArraySubscriber();
+        $arraySubscriber->items($itemsEvent);
+        $this->assertEquals(1, $array[0]->getSortProperty());
+        $_GET ['ord'] = 'desc';
+        $arraySubscriber->items($itemsEvent);
+        $this->assertEquals(3, $array[0]->getSortProperty());
+    }
+
     /**
      * @test
      */