From 2ec4ad118177fa1cacf0dc2f834ef3c0ad3102e0 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 25 Apr 2020 22:15:59 +0100 Subject: [PATCH] Tweaked ListingResponseBuilder to help avoid future issues - Updated so none of the method mutate the query throughout the function so that the query can be handled in a sane way, Since we were already encountering issues due to internal method call order. --- app/Api/ListingResponseBuilder.php | 32 ++++++++++++++++-------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/app/Api/ListingResponseBuilder.php b/app/Api/ListingResponseBuilder.php index 5245a5d90..df4cb8bf1 100644 --- a/app/Api/ListingResponseBuilder.php +++ b/app/Api/ListingResponseBuilder.php @@ -36,10 +36,10 @@ class ListingResponseBuilder */ public function toResponse() { - $this->applyFiltering($this->query); + $filteredQuery = $this->filterQuery($this->query); - $total = $this->query->count(); - $data = $this->fetchData(); + $total = $filteredQuery->count(); + $data = $this->fetchData($filteredQuery); return response()->json([ 'data' => $data, @@ -50,22 +50,22 @@ class ListingResponseBuilder /** * Fetch the data to return in the response. */ - protected function fetchData(): Collection + protected function fetchData(Builder $query): Collection { - $this->applyCountAndOffset($this->query); - $this->applySorting($this->query); - - return $this->query->get($this->fields); + $query = $this->countAndOffsetQuery($query); + $query = $this->sortQuery($query); + return $query->get($this->fields); } /** * Apply any filtering operations found in the request. */ - protected function applyFiltering(Builder $query) + protected function filterQuery(Builder $query): Builder { + $query = clone $query; $requestFilters = $this->request->get('filter', []); if (!is_array($requestFilters)) { - return; + return $query; } $queryFilters = collect($requestFilters)->map(function ($value, $key) { @@ -74,7 +74,7 @@ class ListingResponseBuilder return !is_null($value); })->values()->toArray(); - $query->where($queryFilters); + return $query->where($queryFilters); } /** @@ -102,8 +102,9 @@ class ListingResponseBuilder * Apply sorting operations to the query from given parameters * otherwise falling back to the first given field, ascending. */ - protected function applySorting(Builder $query) + protected function sortQuery(Builder $query): Builder { + $query = clone $query; $defaultSortName = $this->fields[0]; $direction = 'asc'; @@ -117,20 +118,21 @@ class ListingResponseBuilder $sortName = $defaultSortName; } - $query->orderBy($sortName, $direction); + return $query->orderBy($sortName, $direction); } /** * Apply count and offset for paging, based on params from the request while falling * back to system defined default, taking the max limit into account. */ - protected function applyCountAndOffset(Builder $query) + protected function countAndOffsetQuery(Builder $query): Builder { + $query = clone $query; $offset = max(0, $this->request->get('offset', 0)); $maxCount = config('api.max_item_count'); $count = $this->request->get('count', config('api.default_item_count')); $count = max(min($maxCount, $count), 1); - $query->skip($offset)->take($count); + return $query->skip($offset)->take($count); } }