Skip to content

Commit

Permalink
API Add strong-typing to SearchContext::getQuery() $limit param
Browse files Browse the repository at this point in the history
Seems the two search context implementations had different ideas about
what was correct for the limit parameter, and the basic one wasn't even
handling the values correctly.
  • Loading branch information
GuySartorelli committed Jan 8, 2025
1 parent 6d59a7d commit 34e8730
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 26 deletions.
16 changes: 2 additions & 14 deletions src/ORM/Search/BasicSearchContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,16 @@ class BasicSearchContext extends SearchContext
* If a filter is applied to a relationship in dot notation,
* 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|false|string $sort Field to sort on.
* @param int|array|null $limit
* @param SS_List $existingQuery
*/
public function getQuery($searchParams, $sort = false, $limit = false, $existingQuery = null): SS_List
public function getQuery($searchParams, $sort = false, int|array|null $limit = null, $existingQuery = null): SS_List
{
if (!$existingQuery || !is_a($existingQuery, SS_List::class)) {
throw new InvalidArgumentException('getQuery requires a pre-existing SS_List list to be passed as $existingQuery.');
}

if ((count(func_get_args()) >= 3) && (!in_array(gettype($limit), ['array', 'NULL', 'integer']))) {
Deprecation::notice(
'5.1.0',
'$limit should be type of int|array|null'
);
if (is_string($limit) && is_numeric($limit)) {
$limit = (int) $limit;
} else {
$limit = null;
}
}

$searchParams = $this->applySearchFilters($this->normaliseSearchParams($searchParams));
$result = $this->applyGeneralSearchField($searchParams, $existingQuery);

Expand Down
14 changes: 2 additions & 12 deletions src/ORM/Search/SearchContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,22 +142,15 @@ protected function applyBaseTableFields()
* If a filter is applied to a relationship in dot notation,
* 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 Database column to sort on.
* @param array|false|string $sort Database column to sort on.
* Falls back to {@link DataObject::$default_sort} if not provided.
* @param int|array|null $limit
* @param DataList $existingQuery
* @return DataList<T>
* @throws Exception
*/
public function getQuery($searchParams, $sort = false, $limit = false, $existingQuery = null)
public function getQuery($searchParams, $sort = false, int|array|null $limit = null, $existingQuery = null)
{
if ((count(func_get_args()) >= 3) && (!in_array(gettype($limit), ['integer', 'array', 'NULL']))) {
Deprecation::notice(
'5.1.0',
'$limit should be type of int|array|null'
);
$limit = null;
}
$this->setSearchParams($searchParams);
$query = $this->prepareQuery($sort, $limit, $existingQuery);
return $this->search($query);
Expand Down Expand Up @@ -197,9 +190,6 @@ private function prepareQuery($sort, $limit, ?DataList $existingQuery): DataList
}
$query = null;
if ($existingQuery) {
if (!($existingQuery instanceof DataList)) {
throw new InvalidArgumentException("existingQuery must be DataList");
}
if ($existingQuery->dataClass() != $this->modelClass) {
throw new InvalidArgumentException("existingQuery's dataClass is " . $existingQuery->dataClass()
. ", $this->modelClass expected.");
Expand Down

0 comments on commit 34e8730

Please sign in to comment.