From 6bbfd70a447d38605191cbcd644eda93c85f8018 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 18 Sep 2024 15:42:11 +0900 Subject: [PATCH 1/5] pagination() only used by certain models --- app/Models/BeatmapDiscussion.php | 4 +++- app/Models/BeatmapDiscussionPost.php | 3 ++- app/Models/BeatmapDiscussionVote.php | 4 +++- app/Models/BeatmapsetEvent.php | 4 +++- app/Models/Model.php | 11 +++++++++++ app/helpers.php | 12 +----------- 6 files changed, 23 insertions(+), 15 deletions(-) diff --git a/app/Models/BeatmapDiscussion.php b/app/Models/BeatmapDiscussion.php index 0e3b95e9077..702f8636e5d 100644 --- a/app/Models/BeatmapDiscussion.php +++ b/app/Models/BeatmapDiscussion.php @@ -62,6 +62,8 @@ class BeatmapDiscussion extends Model 'review' => 5, ]; + const int PER_PAGE = 20; + const RESOLVABLE_TYPES = [1, 2]; const KUDOSUABLE_TYPES = [1, 2]; @@ -71,7 +73,7 @@ class BeatmapDiscussion extends Model // FIXME: This and other static search functions should be extracted out. public static function search($rawParams = []) { - $pagination = pagination(cursor_from_params($rawParams) ?? $rawParams); + $pagination = static::pagination(cursor_from_params($rawParams) ?? $rawParams); $params = [ 'limit' => $pagination['limit'], diff --git a/app/Models/BeatmapDiscussionPost.php b/app/Models/BeatmapDiscussionPost.php index 22d8efea995..a03910d6471 100644 --- a/app/Models/BeatmapDiscussionPost.php +++ b/app/Models/BeatmapDiscussionPost.php @@ -31,6 +31,7 @@ class BeatmapDiscussionPost extends Model implements Traits\ReportableInterface const MESSAGE_LIMIT = 16_000; // column limit for 4 bytes utf8 const MESSAGE_LIMIT_TIMELINE = 750; + const int PER_PAGE = 20; protected $touches = ['beatmapDiscussion']; @@ -41,7 +42,7 @@ class BeatmapDiscussionPost extends Model implements Traits\ReportableInterface public static function search($rawParams = []) { - $pagination = pagination(cursor_from_params($rawParams) ?? $rawParams); + $pagination = static::pagination(cursor_from_params($rawParams) ?? $rawParams); $params = [ 'limit' => $pagination['limit'], diff --git a/app/Models/BeatmapDiscussionVote.php b/app/Models/BeatmapDiscussionVote.php index 0b8dd6fc8ba..a3210b1e915 100644 --- a/app/Models/BeatmapDiscussionVote.php +++ b/app/Models/BeatmapDiscussionVote.php @@ -19,6 +19,8 @@ */ class BeatmapDiscussionVote extends Model { + const int PER_PAGE = 20; + protected $touches = ['beatmapDiscussion']; public static function recentlyReceivedByUser($userId, $timeframeMonths = 3) @@ -57,7 +59,7 @@ public static function recentlyGivenByUser($userId, $timeframeMonths = 3) public static function search($rawParams = []) { - $pagination = pagination(cursor_from_params($rawParams) ?? $rawParams); + $pagination = static::pagination(cursor_from_params($rawParams) ?? $rawParams); $params = [ 'limit' => $pagination['limit'], diff --git a/app/Models/BeatmapsetEvent.php b/app/Models/BeatmapsetEvent.php index d5331a8d1d8..97ee28e88d5 100644 --- a/app/Models/BeatmapsetEvent.php +++ b/app/Models/BeatmapsetEvent.php @@ -57,6 +57,8 @@ class BeatmapsetEvent extends Model const BEATMAP_OWNER_CHANGE = 'beatmap_owner_change'; + const int PER_PAGE = 20; + public static function log($type, $user, $object, $extraData = []) { if ($object instanceof BeatmapDiscussionPost) { @@ -83,7 +85,7 @@ public static function log($type, $user, $object, $extraData = []) public static function search($rawParams = []) { - $pagination = pagination($rawParams); + $pagination = static::pagination($rawParams); $params = [ 'limit' => $pagination['limit'], diff --git a/app/Models/Model.php b/app/Models/Model.php index 49c27893450..8cecaafa9d9 100644 --- a/app/Models/Model.php +++ b/app/Models/Model.php @@ -35,6 +35,17 @@ public static function booted() static::addGlobalScope(new MacroableModelScope()); } + public static function pagination($params, $defaults = null) + { + $limit = clamp(get_int($params['limit'] ?? null) ?? $defaults['limit'] ?? static::PER_PAGE, 5, 50); + $page = max(get_int($params['page'] ?? null) ?? 1, 1); + + $offset = max_offset($page, $limit); + $page = 1 + $offset / $limit; + + return compact('limit', 'page', 'offset'); + } + public function getForeignKey() { if ($this->primaryKey === null || $this->primaryKey === 'id') { diff --git a/app/helpers.php b/app/helpers.php index 23dc6dd75c5..99039ea75cc 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -9,6 +9,7 @@ use App\Libraries\Base64Url; use App\Libraries\LocaleMeta; use App\Models\LoginAttempt; +use App\Models\Model; use Egulias\EmailValidator\EmailValidator; use Egulias\EmailValidator\Validation\NoRFCWarningsValidation; use Illuminate\Database\Migrations\Migration; @@ -651,17 +652,6 @@ function pack_str($str) return pack('ccH*', 0x0b, strlen($str), bin2hex($str)); } -function pagination($params, $defaults = null) -{ - $limit = clamp(get_int($params['limit'] ?? null) ?? $defaults['limit'] ?? 20, 5, 50); - $page = max(get_int($params['page'] ?? null) ?? 1, 1); - - $offset = max_offset($page, $limit); - $page = 1 + $offset / $limit; - - return compact('limit', 'page', 'offset'); -} - function product_quantity_options($product, $selected = null) { if ($product->stock === null) { From 50f0d75f861522136a93e6b9800d367f59e900b6 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 18 Sep 2024 15:43:12 +0900 Subject: [PATCH 2/5] defaults param not used; max(null, 1) is 1 --- app/Models/Model.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/Models/Model.php b/app/Models/Model.php index 8cecaafa9d9..37ad46987c8 100644 --- a/app/Models/Model.php +++ b/app/Models/Model.php @@ -35,10 +35,10 @@ public static function booted() static::addGlobalScope(new MacroableModelScope()); } - public static function pagination($params, $defaults = null) + public static function pagination($params) { - $limit = clamp(get_int($params['limit'] ?? null) ?? $defaults['limit'] ?? static::PER_PAGE, 5, 50); - $page = max(get_int($params['page'] ?? null) ?? 1, 1); + $limit = clamp(get_int($params['limit'] ?? null) ?? static::PER_PAGE, 5, 50); + $page = max(get_int($params['page'] ?? null), 1); $offset = max_offset($page, $limit); $page = 1 + $offset / $limit; From 50135d9ce4fbdb90c1f8fcd40074dac3aaff6c64 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 18 Sep 2024 17:58:18 +0900 Subject: [PATCH 3/5] streamline making the query and params for the search --- app/Models/BeatmapDiscussion.php | 8 +------- app/Models/BeatmapDiscussionPost.php | 8 +------- app/Models/BeatmapDiscussionVote.php | 8 +------- app/Models/BeatmapsetEvent.php | 8 +------- app/Models/Model.php | 6 ++++-- 5 files changed, 8 insertions(+), 30 deletions(-) diff --git a/app/Models/BeatmapDiscussion.php b/app/Models/BeatmapDiscussion.php index 702f8636e5d..9f925bcf14a 100644 --- a/app/Models/BeatmapDiscussion.php +++ b/app/Models/BeatmapDiscussion.php @@ -73,14 +73,8 @@ class BeatmapDiscussion extends Model // FIXME: This and other static search functions should be extracted out. public static function search($rawParams = []) { - $pagination = static::pagination(cursor_from_params($rawParams) ?? $rawParams); + [$query, $params] = static::searchQueryAndParams(cursor_from_params($rawParams) ?? $rawParams); - $params = [ - 'limit' => $pagination['limit'], - 'page' => $pagination['page'], - ]; - - $query = static::limit($params['limit'])->offset($pagination['offset']); $isModerator = $rawParams['is_moderator'] ?? false; if (present($rawParams['user'] ?? null)) { diff --git a/app/Models/BeatmapDiscussionPost.php b/app/Models/BeatmapDiscussionPost.php index a03910d6471..988b97a52ac 100644 --- a/app/Models/BeatmapDiscussionPost.php +++ b/app/Models/BeatmapDiscussionPost.php @@ -42,14 +42,8 @@ class BeatmapDiscussionPost extends Model implements Traits\ReportableInterface public static function search($rawParams = []) { - $pagination = static::pagination(cursor_from_params($rawParams) ?? $rawParams); + [$query, $params] = static::searchQueryAndParams(cursor_from_params($rawParams) ?? $rawParams); - $params = [ - 'limit' => $pagination['limit'], - 'page' => $pagination['page'], - ]; - - $query = static::limit($params['limit'])->offset($pagination['offset']); $isModerator = $rawParams['is_moderator'] ?? false; if (isset($rawParams['user'])) { diff --git a/app/Models/BeatmapDiscussionVote.php b/app/Models/BeatmapDiscussionVote.php index a3210b1e915..d6b5d1c9c02 100644 --- a/app/Models/BeatmapDiscussionVote.php +++ b/app/Models/BeatmapDiscussionVote.php @@ -59,14 +59,8 @@ public static function recentlyGivenByUser($userId, $timeframeMonths = 3) public static function search($rawParams = []) { - $pagination = static::pagination(cursor_from_params($rawParams) ?? $rawParams); + [$query, $params] = static::searchQueryAndParams(cursor_from_params($rawParams) ?? $rawParams); - $params = [ - 'limit' => $pagination['limit'], - 'page' => $pagination['page'], - ]; - - $query = static::limit($params['limit'])->offset($pagination['offset']); $isModerator = $rawParams['is_moderator'] ?? false; if (isset($rawParams['user'])) { diff --git a/app/Models/BeatmapsetEvent.php b/app/Models/BeatmapsetEvent.php index 97ee28e88d5..6ecfeb184e2 100644 --- a/app/Models/BeatmapsetEvent.php +++ b/app/Models/BeatmapsetEvent.php @@ -85,14 +85,8 @@ public static function log($type, $user, $object, $extraData = []) public static function search($rawParams = []) { - $pagination = static::pagination($rawParams); + [$query, $params] = static::searchQueryAndParams($rawParams); - $params = [ - 'limit' => $pagination['limit'], - 'page' => $pagination['page'], - ]; - - $query = static::limit($params['limit'])->offset($pagination['offset']); $searchByUser = present($rawParams['user'] ?? null); $isModerator = $rawParams['is_moderator'] ?? false; diff --git a/app/Models/Model.php b/app/Models/Model.php index 37ad46987c8..df5caf449b0 100644 --- a/app/Models/Model.php +++ b/app/Models/Model.php @@ -35,7 +35,7 @@ public static function booted() static::addGlobalScope(new MacroableModelScope()); } - public static function pagination($params) + protected static function searchQueryAndParams(array $params) { $limit = clamp(get_int($params['limit'] ?? null) ?? static::PER_PAGE, 5, 50); $page = max(get_int($params['page'] ?? null), 1); @@ -43,7 +43,9 @@ public static function pagination($params) $offset = max_offset($page, $limit); $page = 1 + $offset / $limit; - return compact('limit', 'page', 'offset'); + $query = static::limit($limit)->offset($offset); + + return [$query, compact('limit', 'page')]; } public function getForeignKey() From d37e840c025e9d4cd6027f51e53bbe3aa8838f77 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 18 Sep 2024 18:12:52 +0900 Subject: [PATCH 4/5] unused import --- app/helpers.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/helpers.php b/app/helpers.php index 99039ea75cc..51053a3aec2 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -9,7 +9,6 @@ use App\Libraries\Base64Url; use App\Libraries\LocaleMeta; use App\Models\LoginAttempt; -use App\Models\Model; use Egulias\EmailValidator\EmailValidator; use Egulias\EmailValidator\Validation\NoRFCWarningsValidation; use Illuminate\Database\Migrations\Migration; From 3be56c8311f9f7f022792518d1fea35084f7491b Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 2 Oct 2024 15:45:10 +0900 Subject: [PATCH 5/5] default to 50 --- app/Models/BeatmapDiscussion.php | 2 -- app/Models/BeatmapDiscussionPost.php | 1 - app/Models/BeatmapDiscussionVote.php | 2 -- app/Models/BeatmapsetEvent.php | 2 -- 4 files changed, 7 deletions(-) diff --git a/app/Models/BeatmapDiscussion.php b/app/Models/BeatmapDiscussion.php index 9f925bcf14a..06fb22bd261 100644 --- a/app/Models/BeatmapDiscussion.php +++ b/app/Models/BeatmapDiscussion.php @@ -62,8 +62,6 @@ class BeatmapDiscussion extends Model 'review' => 5, ]; - const int PER_PAGE = 20; - const RESOLVABLE_TYPES = [1, 2]; const KUDOSUABLE_TYPES = [1, 2]; diff --git a/app/Models/BeatmapDiscussionPost.php b/app/Models/BeatmapDiscussionPost.php index 988b97a52ac..a02f502f0e6 100644 --- a/app/Models/BeatmapDiscussionPost.php +++ b/app/Models/BeatmapDiscussionPost.php @@ -31,7 +31,6 @@ class BeatmapDiscussionPost extends Model implements Traits\ReportableInterface const MESSAGE_LIMIT = 16_000; // column limit for 4 bytes utf8 const MESSAGE_LIMIT_TIMELINE = 750; - const int PER_PAGE = 20; protected $touches = ['beatmapDiscussion']; diff --git a/app/Models/BeatmapDiscussionVote.php b/app/Models/BeatmapDiscussionVote.php index d6b5d1c9c02..7006417adcc 100644 --- a/app/Models/BeatmapDiscussionVote.php +++ b/app/Models/BeatmapDiscussionVote.php @@ -19,8 +19,6 @@ */ class BeatmapDiscussionVote extends Model { - const int PER_PAGE = 20; - protected $touches = ['beatmapDiscussion']; public static function recentlyReceivedByUser($userId, $timeframeMonths = 3) diff --git a/app/Models/BeatmapsetEvent.php b/app/Models/BeatmapsetEvent.php index 6ecfeb184e2..2d0fabf891d 100644 --- a/app/Models/BeatmapsetEvent.php +++ b/app/Models/BeatmapsetEvent.php @@ -57,8 +57,6 @@ class BeatmapsetEvent extends Model const BEATMAP_OWNER_CHANGE = 'beatmap_owner_change'; - const int PER_PAGE = 20; - public static function log($type, $user, $object, $extraData = []) { if ($object instanceof BeatmapDiscussionPost) {