Properly apply given default sort

src/Compat/CompatController.php

@@ -10,6 +10,7 @@
 use ipl\Html\HtmlDocument;
 use ipl\Html\HtmlString;
 use ipl\Html\ValidHtml;
+use ipl\Orm\Common\SortUtil;
 use ipl\Orm\Query;
 use ipl\Stdlib\Contract\Paginatable;
 use ipl\Web\Control\LimitControl;
@@ -294,15 +295,25 @@
 
         $this->params->shift($sortControl->getSortParam());
 
-        $sortControl->handleRequest($this->getServerRequest());
-
         $defaultSort = null;
-
         if (func_num_args() === 3) {
             $defaultSort = func_get_args()[2];
         }
 
-        return $sortControl->apply($query, $defaultSort);
+        $default = $defaultSort ?? $query->getModel()->getDefaultSort();
+        if (! empty($default)) {
+            $sortControl->setDefault(SortUtil::normalizeSortSpec($default));
+
+            $columns = $sortControl->getColumns();
+            $default = $sortControl->getDefault();
+            if (! empty($defaultSort) && ! isset($columns[$default])) {
+                throw new InvalidArgumentException(sprintf('Invalid default sort "%s" given', $default));
+            }
+        }
+
+        $sortControl->handleRequest($this->getServerRequest());
+
+        return $sortControl->apply($query);
     }
 
     /**

src/Control/SortControl.php

@@ -1,4 +1,4 @@
 <?php
 
 namespace ipl\Web\Control;
 
@@ -197,23 +197,17 @@
      * Sort the given query according to the request
      *
      * @param Query $query
-     * @param ?array|string $defaultSort
      *
      * @return $this
      */
-    public function apply(Query $query, $defaultSort = null): self
+    public function apply(Query $query): self
     {
         if ($this->getRequest() === null) {
             // handleRequest() has not been called yet
             // TODO: Remove this once everything using this requires ipl v0.12.0
             $this->handleRequest(ServerRequest::fromGlobals());
         }
 
-        $default = $defaultSort ?? (array) $query->getModel()->getDefaultSort();
-        if (! empty($default)) {
-            $this->setDefault(SortUtil::normalizeSortSpec($default));
-        }
-
         $sort = $this->getSort();
         if (! empty($sort)) {
             $query->orderBy(SortUtil::createOrderBy($sort));

tests/CompatControllerTest.php

@@ -0,0 +1,124 @@
+<?php
+
+namespace ipl\Tests\Web;
+
+use Icinga\Web\UrlParams;
+use InvalidArgumentException;
+use ipl\Orm\Model;
+use ipl\Orm\Query;
+use ipl\Web\Compat\CompatController;
+use Zend_Controller_Response_Abstract;
+use Zend_Controller_Request_Abstract;
+
+/**
+ * @aarunTestsInSeparateProcesses
+ */
+class CompatControllerTest extends TestCase
+{
+    protected $controller;
+
+    protected $query;
+
+    public function setUp(): void
+    {
+        if (! class_exists('\Icinga\Web\Controller')) {
+            $this->markTestSkipped('CompatControllerTest only runs locally');
+        }
+
+        class_alias('ipl\Tests\Web\SortControl', 'ipl\Web\Control\SortControl');
+
+        $this->controller = new class extends CompatController {
+
+            protected $params;
+
+            public function __construct(
+                Zend_Controller_Request_Abstract $request = null,
+                Zend_Controller_Response_Abstract $response = null,
+                array $invokeArgs = []
+            ) {
+                $this->params = new UrlParams();
+            }
+        };
+
+        $self = (new self());
+        $model = $self->createMock(Model::class);
+        $model->method('getDefaultSort')->willReturn(['age']);
+
+        $this->query = $self->createMock(Query::class);
+        $this->query->method('getModel')->willReturn($model);
+    }
+
+    public function testCreateSortControlUsesDefaultSortFromModel(): void
+    {
+        $sortControl = $this->controller->createSortControl(
+            $this->query,
+            [
+                'name' => 'Name',
+                'age' => 'Age',
+                'city' => 'City'
+            ]
+        );
+
+        $this->assertSame('age', $sortControl->getDefault());
+    }
+
+    public function testCreateSortControlUsesDefaultSortFromModelWhichIsNotPresentInProvidedColumns(): void
+    {
+        $sortControl = $this->controller->createSortControl(
+           $this->query,
+            [
+                'name' => 'Name',
+                'surname' => 'Surname',
+                'city' => 'City'
+            ]
+        );
+
+        $this->assertSame('age', $sortControl->getDefault());
+    }
+
+    public function testCreateSortControlUsesProvidedThirdParamAsString(): void
+    {
+        $sortControl = $this->controller->createSortControl(
+            $this->query,
+            [
+                'name' => 'Name',
+                'age' => 'age',
+                'city' => 'City'
+            ],
+            'city'
+        );
+
+        $this->assertSame('city', $sortControl->getDefault());
+    }
+
+    public function testCreateSortControlUsesProvidedThirdParamAsArray(): void
+    {
+        $sortControl = $this->controller->createSortControl(
+            $this->query,
+            [
+                'name' => 'Name',
+                'age' => 'age',
+                'city' => 'City'
+            ],
+            ['city']
+        );
+
+        $this->assertSame('city', $sortControl->getDefault());
+    }
+
+    public function testCreateSortControlThrowsExceptionWhenProvidedThirdParamIsNotPresentInProvidedColumns(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Invalid default sort "car" given');
+
+       $this->controller->createSortControl(
+            $this->query,
+            [
+                'name' => 'Name',
+                'age' => 'age',
+                'city' => 'City'
+            ],
+            ['car']
+        );
+    }
+}

tests/SortControl.php

@@ -0,0 +1,71 @@
+<?php
+
+namespace ipl\Tests\Web;
+
+use ipl\Orm\Common\SortUtil;
+
+/**
+ * Imitates the behavior of {@see \ipl\Web\Control\SortControl} class
+ *
+ * Reduces the {@see self::__construct()} and {@see self::create()} method to only required functionality
+ */
+class SortControl {
+
+    protected $columns = [];
+
+    protected $default;
+
+    private function __construct(array $columns)
+    {
+        $this->setColumns($columns);
+    }
+
+    public static function create(array $options)
+    {
+        $normalized = [];
+        foreach ($options as $spec => $label) {
+            $normalized[SortUtil::normalizeSortSpec($spec)] = $label;
+        }
+
+        return new static($normalized);
+    }
+
+    public function setColumns($columns)
+    {
+        $this->columns = array_change_key_case($columns, CASE_LOWER);
+
+        return $this;
+    }
+
+    public function getColumns()
+    {
+        return $this->columns;
+    }
+
+    public function getDefault(): ?string
+    {
+        return $this->default;
+    }
+
+    public function setDefault(string $default): self
+    {
+        // We're working with lowercase keys throughout the sort control
+        $this->default = strtolower($default);
+
+        return $this;
+    }
+
+    public function getSortParam()
+    {
+        return '';
+    }
+
+    public function handleRequest()
+    {
+    }
+
+    public function apply()
+    {
+        return $this;
+    }
+}