From 52f3ec7d3d45113450f0df451953b9dae9f03e0b Mon Sep 17 00:00:00 2001 From: Nicky De Maeyer Date: Tue, 7 Oct 2025 14:32:23 +0200 Subject: [PATCH] improved request handling --- app/Api/V1/Controllers/Controller.php | 2 + .../Models/Account/ShowController.php | 50 +++++----- .../Models/UserGroup/IndexController.php | 16 ++-- .../Controllers/Summary/BasicController.php | 6 +- app/Api/V1/Requests/AggregateFormRequest.php | 92 +++++++++++++++++++ app/Api/V1/Requests/ApiRequest.php | 45 +++++++++ app/Api/V1/Requests/Data/DateRequest.php | 82 ----------------- app/Api/V1/Requests/DateRangeRequest.php | 56 +++++++++++ app/Api/V1/Requests/DateRequest.php | 57 ++++++++++++ .../Models/Account/AccountTypeApiRequest.php | 57 ++++++++++++ .../Requests/Models/Account/ShowRequest.php | 76 ++------------- app/Api/V1/Requests/PaginationRequest.php | 85 +++++++++++++++++ 12 files changed, 442 insertions(+), 182 deletions(-) create mode 100644 app/Api/V1/Requests/AggregateFormRequest.php create mode 100644 app/Api/V1/Requests/ApiRequest.php delete mode 100644 app/Api/V1/Requests/Data/DateRequest.php create mode 100644 app/Api/V1/Requests/DateRangeRequest.php create mode 100644 app/Api/V1/Requests/DateRequest.php create mode 100644 app/Api/V1/Requests/Models/Account/AccountTypeApiRequest.php create mode 100644 app/Api/V1/Requests/PaginationRequest.php diff --git a/app/Api/V1/Controllers/Controller.php b/app/Api/V1/Controllers/Controller.php index 298bc20cc7..d83c713aa1 100644 --- a/app/Api/V1/Controllers/Controller.php +++ b/app/Api/V1/Controllers/Controller.php @@ -67,6 +67,7 @@ abstract class Controller extends BaseController protected bool $convertToPrimary = false; protected TransactionCurrency $primaryCurrency; + /** @deprecated use Request classes */ protected ParameterBag $parameters; /** @@ -98,6 +99,7 @@ abstract class Controller extends BaseController } /** + * @deprecated use Request classes * Method to grab all parameters from the URL. */ private function getParameters(): ParameterBag diff --git a/app/Api/V1/Controllers/Models/Account/ShowController.php b/app/Api/V1/Controllers/Models/Account/ShowController.php index 587076d0db..cb0097062e 100644 --- a/app/Api/V1/Controllers/Models/Account/ShowController.php +++ b/app/Api/V1/Controllers/Models/Account/ShowController.php @@ -71,40 +71,40 @@ class ShowController extends Controller public function index(ShowRequest $request): JsonResponse { $manager = $this->getManager(); - $params = $request->getParameters(); - $this->parameters->set('type', $params['type']); - - // types to get, page size: - $types = $this->mapAccountTypes($params['type']); + [ + 'types' => $types, + 'page' => $page, + 'limit' => $limit, + 'offset' => $offset, + 'sort' => $sort, + 'start' => $start, + 'end' => $end, + 'date' => $date, + ] = $request->attributes->all(); // get list of accounts. Count it and split it. $this->repository->resetAccountOrder(); - $collection = $this->repository->getAccountsByType($types, $params['sort']); + $collection = $this->repository->getAccountsByType($types, $sort); $count = $collection->count(); // continue sort: // TODO if the user sorts on DB dependent field there must be no slice before enrichment, only after. // TODO still need to figure out how to do this easily. - $accounts = $collection->slice(($this->parameters->get('page') - 1) * $params['limit'], $params['limit']); - - // #11007 go to the end of the previous day. - $this->parameters->set('start', $this->parameters->get('start')?->subSecond()); - // #11018 also end of the day. - $this->parameters->set('end', $this->parameters->get('end')?->endOfDay()); + $accounts = $collection->slice($offset, $limit); // enrich /** @var User $admin */ $admin = auth()->user(); $enrichment = new AccountEnrichment(); - $enrichment->setSort($params['sort']); - $enrichment->setDate($this->parameters->get('date')); - $enrichment->setStart($this->parameters->get('start')); - $enrichment->setEnd($this->parameters->get('end')); + $enrichment->setSort($sort); + $enrichment->setDate($date); + $enrichment->setStart($start); + $enrichment->setEnd($end); $enrichment->setUser($admin); $accounts = $enrichment->enrich($accounts); // make paginator: - $paginator = new LengthAwarePaginator($accounts, $count, $params['limit'], $this->parameters->get('page')); + $paginator = new LengthAwarePaginator($accounts, $count, $limit, $page); $paginator->setPath(route('api.v1.accounts.index').$this->buildParams()); /** @var AccountTransformer $transformer */ @@ -129,19 +129,19 @@ class ShowController extends Controller $this->repository->resetAccountOrder(); $account->refresh(); $manager = $this->getManager(); - - // #11007 go to the end of the previous day. - $this->parameters->set('start', $this->parameters->get('start')?->subSecond()); - // #11018 also end of the day. - $this->parameters->set('end', $this->parameters->get('end')?->endOfDay()); + [ + 'start' => $start, + 'end' => $end, + 'date' => $date, + ] = $request->attributes->all(); // enrich /** @var User $admin */ $admin = auth()->user(); $enrichment = new AccountEnrichment(); - $enrichment->setDate($this->parameters->get('date')); - $enrichment->setStart($this->parameters->get('start')); - $enrichment->setEnd($this->parameters->get('end')); + $enrichment->setDate($date); + $enrichment->setStart($start); + $enrichment->setEnd($end); $enrichment->setUser($admin); $account = $enrichment->enrichSingle($account); diff --git a/app/Api/V1/Controllers/Models/UserGroup/IndexController.php b/app/Api/V1/Controllers/Models/UserGroup/IndexController.php index e689687467..8c4ba0ddbb 100644 --- a/app/Api/V1/Controllers/Models/UserGroup/IndexController.php +++ b/app/Api/V1/Controllers/Models/UserGroup/IndexController.php @@ -25,7 +25,7 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Controllers\Models\UserGroup; use FireflyIII\Api\V1\Controllers\Controller; -use FireflyIII\Api\V1\Requests\Data\DateRequest; +use FireflyIII\Api\V1\Requests\PaginationRequest; use FireflyIII\Repositories\UserGroup\UserGroupRepositoryInterface; use FireflyIII\Transformers\UserGroupTransformer; use Illuminate\Http\JsonResponse; @@ -52,16 +52,20 @@ class IndexController extends Controller ); } - public function index(DateRequest $request): JsonResponse + public function index(PaginationRequest $request): JsonResponse { $administrations = $this->repository->get(); - $pageSize = $this->parameters->get('limit'); + [ + 'page' => $page, + 'limit' => $limit, + 'offset' => $offset, + ] = $request->attributes->all(); $count = $administrations->count(); - $administrations = $administrations->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); - $paginator = new LengthAwarePaginator($administrations, $count, $pageSize, $this->parameters->get('page')); + $administrations = $administrations->slice($offset, $limit); + $paginator = new LengthAwarePaginator($administrations, $count, $limit, $page); $transformer = new UserGroupTransformer(); - $transformer->setParameters($this->parameters); // give params to transformer + $transformer->setParameters($request->attributes); // give params to transformer return response() ->json($this->jsonApiList(self::RESOURCE_KEY, $paginator, $transformer)) diff --git a/app/Api/V1/Controllers/Summary/BasicController.php b/app/Api/V1/Controllers/Summary/BasicController.php index 64c39e5766..6822674139 100644 --- a/app/Api/V1/Controllers/Summary/BasicController.php +++ b/app/Api/V1/Controllers/Summary/BasicController.php @@ -27,7 +27,7 @@ namespace FireflyIII\Api\V1\Controllers\Summary; use Carbon\Carbon; use Exception; use FireflyIII\Api\V1\Controllers\Controller; -use FireflyIII\Api\V1\Requests\Data\DateRequest; +use FireflyIII\Api\V1\Requests\DateRangeRequest; use FireflyIII\Enums\AccountTypeEnum; use FireflyIII\Enums\TransactionTypeEnum; use FireflyIII\Helpers\Collector\GroupCollectorInterface; @@ -94,10 +94,10 @@ class BasicController extends Controller * * @throws Exception */ - public function basic(DateRequest $request): JsonResponse + public function basic(DateRangeRequest $request): JsonResponse { // parameters for boxes: - $dates = $request->getAll(); + $dates = $request->attributes->all(); $start = $dates['start']; $end = $dates['end']; $code = $request->get('currency_code'); diff --git a/app/Api/V1/Requests/AggregateFormRequest.php b/app/Api/V1/Requests/AggregateFormRequest.php new file mode 100644 index 0000000000..207ce0640d --- /dev/null +++ b/app/Api/V1/Requests/AggregateFormRequest.php @@ -0,0 +1,92 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Api\V1\Requests; + +use Illuminate\Foundation\Http\FormRequest; +use Illuminate\Http\Request; +use Illuminate\Support\Facades\Log; +use Illuminate\Validation\Validator; + +abstract class AggregateFormRequest extends ApiRequest +{ + /** + * @var ApiRequest[] + */ + protected array $requests = []; + + /** @return class-string[] */ + abstract protected function getRequests(): array; + + public function initialize(array $query = [], array $request = [], array $attributes = [], array $cookies = [], array $files = [], array $server = [], $content = null): void + { + parent::initialize($query, $request, $attributes, $cookies, $files, $server, $content); + + // instantiate all subrequests and share current requests' bags with them + foreach ($this->getRequests() as $config) { + $requestClass = is_array($config) ? array_shift($config) : $config; + + if (!is_a($requestClass, Request::class, true)) { + throw new \RuntimeException('getRequests() must return class-strings of subclasses of Request'); + } + + $instance = $this->requests[] = new $requestClass(); + $instance->request = $this->request; + $instance->query = $this->query; + $instance->attributes = $this->attributes; + $instance->cookies = $this->cookies; + $instance->files = $this->files; + $instance->server = $this->server; + $instance->headers = $this->headers; + + if ($instance instanceof ApiRequest) { + $instance->handleConfig(is_array($config) ? $config : []); + } + } + } + + public function rules(): array + { + // check all subrequests for rules and combine them + return array_reduce( + $this->requests, + static fn (array $rules, FormRequest $request) => + $rules + + (method_exists($request, 'rules') + ? $request->rules() + : [] + ), + [], + ); + } + + public function withValidator(Validator $validator): void + { + // register all subrequests' validators + foreach ($this->requests as $request) { + if (method_exists($request, 'withValidator')) { + $request->withValidator($validator); + } + } + } +} diff --git a/app/Api/V1/Requests/ApiRequest.php b/app/Api/V1/Requests/ApiRequest.php new file mode 100644 index 0000000000..cb9a7b6046 --- /dev/null +++ b/app/Api/V1/Requests/ApiRequest.php @@ -0,0 +1,45 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Api\V1\Requests; + +use FireflyIII\Exceptions\ValidationException; +use FireflyIII\Support\Request\ChecksLogin; +use FireflyIII\Support\Request\ConvertsDataTypes; +use Illuminate\Foundation\Http\FormRequest; +use Illuminate\Validation\Validator; + +class ApiRequest extends FormRequest +{ + use ChecksLogin; + use ConvertsDataTypes; + + protected string $required = ''; + + public function handleConfig(array $config): void + { + if (in_array('required', $config)) { + $this->required = 'required'; + } + } +} diff --git a/app/Api/V1/Requests/Data/DateRequest.php b/app/Api/V1/Requests/Data/DateRequest.php deleted file mode 100644 index c112495d46..0000000000 --- a/app/Api/V1/Requests/Data/DateRequest.php +++ /dev/null @@ -1,82 +0,0 @@ -. - */ - -declare(strict_types=1); - -namespace FireflyIII\Api\V1\Requests\Data; - -use FireflyIII\Exceptions\ValidationException; -use FireflyIII\Support\Request\ChecksLogin; -use FireflyIII\Support\Request\ConvertsDataTypes; -use Illuminate\Foundation\Http\FormRequest; - -/** - * Request class for end points that require date parameters. - * - * Class DateRequest - */ -class DateRequest extends FormRequest -{ - use ChecksLogin; - use ConvertsDataTypes; - - /** - * Get all data from the request. - */ - public function getAll(): array - { - $start = $this->getCarbonDate('start'); - $end = $this->getCarbonDate('end'); - if (null === $start) { - $start = now()->startOfMonth(); - } - if (null === $end) { - $end = now()->endOfMonth(); - } - // sanity check on dates: - [$start, $end] = $end < $start ? [$end, $start] : [$start, $end]; - - $start->startOfDay(); - $end->endOfDay(); - if ($start->diffInYears($end, true) > 5) { - throw new ValidationException('Date range out of range.'); - } - - return [ - 'start' => $start, - 'end' => $end, - 'date' => $this->getCarbonDate('date'), - ]; - } - - /** - * The rules that the incoming request must be matched against. - */ - public function rules(): array - { - return [ - 'date' => 'date|after:1970-01-02|before:2038-01-17', - 'start' => 'date|after:1970-01-02|before:2038-01-17|before:end|required_with:end', - 'end' => 'date|after:1970-01-02|before:2038-01-17|after:start|required_with:start', - ]; - } -} diff --git a/app/Api/V1/Requests/DateRangeRequest.php b/app/Api/V1/Requests/DateRangeRequest.php new file mode 100644 index 0000000000..c4e2dc87ec --- /dev/null +++ b/app/Api/V1/Requests/DateRangeRequest.php @@ -0,0 +1,56 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Api\V1\Requests; + +use FireflyIII\Exceptions\ValidationException; +use FireflyIII\Support\Request\ChecksLogin; +use FireflyIII\Support\Request\ConvertsDataTypes; +use Illuminate\Foundation\Http\FormRequest; + +class DateRangeRequest extends ApiRequest +{ + public function rules(): array + { + return [ + 'start' => 'date|after:1970-01-02|before:2038-01-17|before:end|required_with:end|' . $this->required, + 'end' => 'date|after:1970-01-02|before:2038-01-17|after:start|required_with:start|' . $this->required, + ]; + } + + public function withValidator(Validator $validator): void + { + $validator->after( + function (Validator $validator): void { + if (!$validator->valid()) { + return; + } + $start = $this->getCarbonDate('start')?->startOfDay(); + $end = $this->getCarbonDate('end')?->endOfDay(); + + $this->attributes->set('start', $start); + $this->attributes->set('end', $end); + } + ); + } +} diff --git a/app/Api/V1/Requests/DateRequest.php b/app/Api/V1/Requests/DateRequest.php new file mode 100644 index 0000000000..c59dd65b13 --- /dev/null +++ b/app/Api/V1/Requests/DateRequest.php @@ -0,0 +1,57 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Api\V1\Requests; + +use Illuminate\Validation\Validator; + +class DateRequest extends ApiRequest +{ + public function rules(): array + { + return [ + 'date' => 'date|after:1970-01-02|before:2038-01-17|' . $this->required, + ]; + } + + public function withValidator(Validator $validator): void + { + $validator->after( + function (Validator $validator): void { + if (!$validator->valid()) { + return; + } + $date = $this->getCarbonDate('date')?->endOfDay(); + + // if we also have a range, date must be in that range + $start = $this->attributes->get('start'); + $end = $this->attributes->get('end'); + if ($date && $start && $end && !$date->between($start, $end)) { + $validator->errors()->add('date', (string)trans('validation.between_date')); + } + + $this->attributes->set('date', $date); + } + ); + } +} diff --git a/app/Api/V1/Requests/Models/Account/AccountTypeApiRequest.php b/app/Api/V1/Requests/Models/Account/AccountTypeApiRequest.php new file mode 100644 index 0000000000..10576e5717 --- /dev/null +++ b/app/Api/V1/Requests/Models/Account/AccountTypeApiRequest.php @@ -0,0 +1,57 @@ +. + */ + +namespace FireflyIII\Api\V1\Requests\Models\Account; + +use FireflyIII\Api\V1\Requests\ApiRequest; +use FireflyIII\Support\Http\Api\AccountFilter; +use Illuminate\Validation\Validator; + +class AccountTypeApiRequest extends ApiRequest +{ + use AccountFilter; + + public function rules(): array + { + return [ + 'type' => sprintf('in:%s', implode(',', array_keys($this->types))), + ]; + } + + public function withValidator(Validator $validator): void + { + $validator->after( + function (Validator $validator): void { + if (!$validator->valid()) { + return; + } + + $type = $this->convertString('type', 'all'); + $this->attributes->add([ + 'type' => $type, + 'types' => $this->mapAccountTypes($type), + ]); + } + ); + } +} diff --git a/app/Api/V1/Requests/Models/Account/ShowRequest.php b/app/Api/V1/Requests/Models/Account/ShowRequest.php index f4f7d73332..ed0a7dafe1 100644 --- a/app/Api/V1/Requests/Models/Account/ShowRequest.php +++ b/app/Api/V1/Requests/Models/Account/ShowRequest.php @@ -23,77 +23,21 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests\Models\Account; -use Carbon\Carbon; +use FireflyIII\Api\V1\Requests\AggregateFormRequest; +use FireflyIII\Api\V1\Requests\DateRangeRequest; +use FireflyIII\Api\V1\Requests\DateRequest; +use FireflyIII\Api\V1\Requests\PaginationRequest; use FireflyIII\Models\Account; -use FireflyIII\Rules\IsValidSortInstruction; -use FireflyIII\Support\Facades\Preferences; -use FireflyIII\Support\Http\Api\AccountFilter; -use FireflyIII\Support\Request\ConvertsDataTypes; -use FireflyIII\User; -use Illuminate\Foundation\Http\FormRequest; -use Illuminate\Validation\Validator; -class ShowRequest extends FormRequest +class ShowRequest extends AggregateFormRequest { - use AccountFilter; - use ConvertsDataTypes; - - public function getParameters(): array + protected function getRequests(): array { - $limit = $this->convertInteger('limit'); - if (0 === $limit) { - // get default for user: - /** @var User $user */ - $user = auth()->user(); - $limit = (int)Preferences::getForUser($user, 'listPageSize', 50)->data; - } - - $page = $this->convertInteger('page'); - $page = min(max(1, $page), 2 ** 16); - return [ - 'type' => $this->convertString('type', 'all'), - 'limit' => $limit, - 'sort' => $this->convertSortParameters('sort', Account::class), - 'page' => $page, + [PaginationRequest::class, 'sort_class' => Account::class], + DateRangeRequest::class, + DateRequest::class, + AccountTypeApiRequest::class, ]; } - - public function rules(): array - { - $keys = implode(',', array_keys($this->types)); - - return [ - 'date' => 'date', - 'start' => 'date|present_with:end|before_or_equal:end|before:2038-01-17|after:1970-01-02', - 'end' => 'date|present_with:start|after_or_equal:start|before:2038-01-17|after:1970-01-02', - 'sort' => ['nullable', new IsValidSortInstruction(Account::class)], - 'type' => sprintf('in:%s', $keys), - 'limit' => 'numeric|min:1|max:131337', - 'page' => 'numeric|min:1|max:131337', - ]; - } - - public function withValidator(Validator $validator): void - { - $validator->after( - function (Validator $validator): void { - if (count($validator->failed()) > 0) { - return; - } - $data = $validator->getData(); - - - if (array_key_exists('date', $data) && array_key_exists('start', $data) && array_key_exists('end', $data)) { - // assume valid dates, before we got here. - $start = Carbon::parse($data['start'], config('app.timezone'))->startOfDay(); - $end = Carbon::parse($data['end'], config('app.timezone'))->endOfDay(); - $date = Carbon::parse($data['date'], config('app.timezone')); - if (!$date->between($start, $end)) { - $validator->errors()->add('date', (string)trans('validation.between_date')); - } - } - } - ); - } } diff --git a/app/Api/V1/Requests/PaginationRequest.php b/app/Api/V1/Requests/PaginationRequest.php new file mode 100644 index 0000000000..0dc9719929 --- /dev/null +++ b/app/Api/V1/Requests/PaginationRequest.php @@ -0,0 +1,85 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Api\V1\Requests; + +use FireflyIII\Models\Account; +use FireflyIII\Rules\IsValidSortInstruction; +use FireflyIII\Support\Facades\Preferences; +use FireflyIII\User; +use Illuminate\Validation\Validator; + +class PaginationRequest extends ApiRequest +{ + private ?string $sortClass = null; + + public function handleConfig(array $config): void + { + parent::handleConfig($config); + + $this->sortClass = $config['sort_class'] ?? null; + + if (!$this->sortClass) { + throw new \RuntimeException('PaginationRequest requires a sort_class config'); + } + } + + + public function rules(): array + { + return [ + 'sort' => ['nullable', new IsValidSortInstruction($this->sortClass)], + 'limit' => 'numeric|min:1|max:131337', + 'page' => 'numeric|min:1|max:131337', + ]; + } + + public function withValidator(Validator $validator): void + { + $validator->after( + function (Validator $validator): void { + if (!$validator->valid()) { + return; + } + + $limit = $this->convertInteger('limit'); + if (0 === $limit) { + // get default for user: + /** @var User $user */ + $user = auth()->user(); + $limit = (int)Preferences::getForUser($user, 'listPageSize', 50)->data; + } + + $page = $this->convertInteger('page'); + $page = min(max(1, $page), 2 ** 16); + $offset = ($page - 1) * $limit; + $sort = $this->sortClass ? $this->convertSortParameters('sort', $this->sortClass) : $this->get('sort'); + + $this->attributes->set('limit', $limit); + $this->attributes->set('sort', $sort); + $this->attributes->set('page', $page); + $this->attributes->set('offset', $offset); + } + ); + } +}