FIX Apply correct typing to $limit in BasicSearchContext (#11535)

Allows for array limits to be used correctly, and updates the expected type to match SearchContext
silverstripe · Jan 7, 2025 · 597d110 · 597d110
1 parent 06240b6
commit 597d110
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 5 deletions.
diff --git a/src/ORM/Search/BasicSearchContext.php b/src/ORM/Search/BasicSearchContext.php
@@ -36,7 +36,7 @@ class BasicSearchContext extends SearchContext
      *  the parameter name should have the dots replaced with double underscores,
      *  for example "Comments__Name" instead of the filter name "Comments.Name".
      * @param array|bool|string $sort Field to sort on.
-     * @param array|null|string $limit
+     * @param int|array|null $limit
      * @param Filterable&Sortable&Limitable $existingQuery
      */
     public function getQuery($searchParams, $sort = false, $limit = false, $existingQuery = null): Filterable&Sortable&Limitable
@@ -45,12 +45,16 @@ public function getQuery($searchParams, $sort = false, $limit = false, $existing
             throw new InvalidArgumentException('getQuery requires a pre-existing filterable/sortable/limitable list to be passed as $existingQuery.');
         }
 
-        if ((count(func_get_args()) >= 3) && (!in_array(gettype($limit), ['array', 'NULL', 'string']))) {
+        if ((count(func_get_args()) >= 3) && (!in_array(gettype($limit), ['array', 'NULL', 'integer']))) {
             Deprecation::notice(
                 '5.1.0',
-                '$limit should be type of array|string|null'
+                '$limit should be type of int|array|null'
             );
-            $limit = null;
+            if (is_string($limit) && is_numeric($limit)) {
+                $limit = (int) $limit;
+            } else {
+                $limit = null;
+            }
         }
 
         $searchParams = $this->applySearchFilters($this->normaliseSearchParams($searchParams));
@@ -67,7 +71,14 @@ public function getQuery($searchParams, $sort = false, $limit = false, $existing
         }
 
         // Limit must be last so that ArrayList results don't have an applied limit before they can be filtered/sorted.
-        $result = $result->limit($limit);
+        if (is_array($limit)) {
+            $result = $result->limit(
+                isset($limit['limit']) ? $limit['limit'] : null,
+                isset($limit['start']) ? $limit['start'] : null
+            );
+        } else {
+            $result = $result->limit($limit);
+        }
 
         return $result;
     }

diff --git a/tests/php/ORM/Search/BasicSearchContextTest.php b/tests/php/ORM/Search/BasicSearchContextTest.php
@@ -239,6 +239,71 @@ public function testGetQuery(array $searchParams, array $expected)
         $this->assertSame($expected, $results->column('Name'));
     }
 
+    public function provideGetQueryLimit(): array
+    {
+        return [
+            'no limit' => [
+                'limit' => null,
+                'expected' => [
+                    'James',
+                    'John',
+                    'Jane',
+                    'Hemi',
+                    'Sara',
+                    'MatchNothing',
+                ],
+            ],
+            'limit int' => [
+                'limit' => 3,
+                'expected' => [
+                    'James',
+                    'John',
+                    'Jane',
+                ],
+            ],
+            'paginated limit' => [
+                'limit' => [
+                    'limit' => 2,
+                    'start' => 3,
+                ],
+                'expected' => [
+                    'Hemi',
+                    'Sara',
+                ],
+            ],
+            'limit numeric string' => [
+                'limit' => '4',
+                'expected' => [
+                    'James',
+                    'John',
+                    'Jane',
+                    'Hemi',
+                ],
+            ],
+            'limit invalid string' => [
+                'limit' => 'blah',
+                'expected' => [
+                    'James',
+                    'John',
+                    'Jane',
+                    'Hemi',
+                    'Sara',
+                    'MatchNothing',
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideGetQueryLimit
+     */
+    public function testGetQueryLimit(mixed $limit, array $expected): void
+    {
+        $context = new BasicSearchContext(ArrayData::class);
+        $results = $context->getQuery([], limit: $limit, existingQuery: $this->getList());
+        $this->assertSame($expected, $results->column('Name'));
+    }
+
     public function testGeneralSearch()
     {
         $list = $this->getList();