feat: add FormRequest for encapsulating validation and authorization#10087
feat: add FormRequest for encapsulating validation and authorization#10087michalsn wants to merge 12 commits intocodeigniter4:4.8from
FormRequest for encapsulating validation and authorization#10087Conversation
paulbalandan
left a comment
There was a problem hiding this comment.
I find the same named class in Laravel useful before for focused request validation. If the functionality is similar to that, then I think it would be also beneficial here.
Some initial thoughts:
|
Overall, this is a really nice feature to have. Most of the times, I have to create a final class which has static functions which return array of validation rules and call Validation service in controller. But looks like I have easier way to do that from 4.8.0. Thanks @michalsn |
|
You've started developing magic (autowire). It would be great not to limit yourself to one form and continue the idea. set the Autowire interface and any available class can be connected as in other frameworks. I think it can be done a long time ago. But the framework adheres to the rules from the 2000s =) Less obscure magic. Thus, we come to the need for DI/ServiceLocator, which is now replacing Services. You can view https://github.com/PHP-DI/PHP-DI as a separate package. |
|
I'm already making a similar object for validation myself, only directly |
|
@paulbalandan I took the idea for this directly from Laravel. |
|
@neznaika0 Thanks for the feedback. Full DI container support is a legitimate feature in frameworks that prioritize that approach, but I don't believe it's the direction CI4 is going. That's not a weakness - it's more a positioning decision. |
|
That's the philosophy I'm talking about. I'm worried that subsequent similar improvements will use the same principle, but duplicate the logic. Right now, I can't say what would be useful to dynamically add to the controllers.. Everything is possible - Models, Services, Configs.. in the end, we come to DI =). I think that's why many people don't look towards CI, because they're used to magic SF, Laravel. Wordpress is an exception - it has a large community and allows non-standard approaches. |
That's a real concern. In fact Services are CodeIgniter answer to dependency management - explicit, overridable, and testable. They're not a DI container in the classical sense, but they serve the same purpose without the implicit magic. |
0b455a9 to
0ae2d37
Compare
ac35c4a to
342e1df
Compare
Co-authored-by: John Paul E. Balandan, CPA <paulbalandan@gmail.com>
342e1df to
d676085
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new CodeIgniter\HTTP\FormRequest abstraction to encapsulate validation rules, custom messages, and authorization in dedicated request classes, and adds framework-level injection so these requests are authorized/validated before controller/closure execution.
Changes:
- Added
FormRequestbase class plus aFormRequestExceptionshort-circuit to return validation/authorization failure responses before the handler runs. - Updated dispatcher/controller invocation to resolve callable parameters via reflection (including
FormRequestinjection) and adjusted AutoRouterImproved’s URI-parameter counting. - Added
make:requestSpark generator scaffolding, tests, and user guide documentation/changelog entries.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/phpstan-baseline/missingType.callable.neon | Updated PHPStan baseline for newly introduced Closure-signature issues. |
| utils/phpstan-baseline/loader.neon | Updated PHPStan baseline totals. |
| utils/phpstan-baseline/argument.type.neon | Updated PHPStan baseline totals and removed now-unneeded ignores. |
| user_guide_src/source/libraries/validation.rst | Linked validation docs to the new Form Requests documentation. |
| user_guide_src/source/incoming/index.rst | Added Form Requests to Incoming Request docs index. |
| user_guide_src/source/incoming/form_requests.rst | Added full user guide page documenting Form Requests. |
| user_guide_src/source/incoming/form_requests/001.php | User guide example: basic FormRequest rules(). |
| user_guide_src/source/incoming/form_requests/002.php | User guide example: controller injection and validated(). |
| user_guide_src/source/incoming/form_requests/003.php | User guide example: route params + FormRequest ordering. |
| user_guide_src/source/incoming/form_requests/004.php | User guide example: custom messages(). |
| user_guide_src/source/incoming/form_requests/005.php | User guide example: isAuthorized(). |
| user_guide_src/source/incoming/form_requests/006.php | User guide example: prepareForValidation(). |
| user_guide_src/source/incoming/form_requests/007.php | User guide example: overriding validationData(). |
| user_guide_src/source/incoming/form_requests/008.php | User guide example: overriding failure responses. |
| user_guide_src/source/incoming/form_requests/009.php | User guide example: validated() output. |
| user_guide_src/source/incoming/form_requests/010.php | User guide example: accessing underlying IncomingRequest. |
| user_guide_src/source/incoming/form_requests/011.php | User guide example: closure-route injection. |
| user_guide_src/source/incoming/form_requests/012.php | User guide example: manual resolveRequest() in _remap(). |
| user_guide_src/source/incoming/form_requests/013.php | User guide example: flashing normalized values after failure. |
| user_guide_src/source/incoming/form_requests/014.php | User guide example: getValidated()/hasValidated() with dot syntax. |
| user_guide_src/source/changelogs/v4.8.0.rst | Changelog entries for FormRequest and make:request. |
| tests/system/Router/Controllers/Requests/MyFormRequest.php | Test fixture FormRequest for AutoRouterImproved routing tests. |
| tests/system/Router/Controllers/Mycontroller.php | Added controller methods that include FormRequest parameters. |
| tests/system/Router/AutoRouterImprovedTest.php | Tests ensuring FormRequest params don’t consume URI segments. |
| tests/system/HTTP/FormRequestTest.php | Comprehensive unit/integration coverage for FormRequest behavior. |
| tests/system/Commands/Utilities/Routes/ControllerFinderTest.php | Updated expected controller discovery to include new test controller. |
| tests/system/Commands/Utilities/Routes/AutoRouteCollectorTest.php | Updated expected autoroute collection output. |
| tests/system/Commands/Generators/FormRequestGeneratorTest.php | Added generator tests for make:request scaffolding. |
| tests/_support/HTTP/Requests/ValidPostFormRequest.php | Support FormRequest for tests requiring valid rules. |
| tests/_support/HTTP/Requests/UnauthorizedFormRequest.php | Support FormRequest for authorization-failure tests. |
| tests/_support/Controllers/FormRequestController.php | Support controller for FormRequest injection integration tests. |
| system/Router/RouterInterface.php | Adjusted PHPDoc return types related to closures/controller resolution. |
| system/Router/Router.php | Adjusted PHPDoc types for $controller and controllerName() docs. |
| system/Router/RouteCollectionInterface.php | Adjusted PHPDoc for route handler parameter types. |
| system/Router/RouteCollection.php | Adjusted PHPDoc for route handler parameter types. |
| system/Router/AutoRouterImproved.php | Excluded FormRequest params from URI segment count check. |
| system/Language/en/CLI.php | Added CLI language key for request generator prompt. |
| system/HTTP/FormRequest.php | New FormRequest base class (rules/messages/auth/validation/validated accessors). |
| system/HTTP/Exceptions/FormRequestException.php | New ResponsableInterface exception used to short-circuit responses. |
| system/Commands/Generators/Views/formrequest.tpl.php | New generator template for FormRequest scaffolding. |
| system/Commands/Generators/FormRequestGenerator.php | New make:request generator command implementation. |
| system/CodeIgniter.php | Added callable parameter resolution and FormRequest injection for controllers/closures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $uriParamCount = count(array_filter( | ||
| $refParams, | ||
| static fn ($p): bool => FormRequest::getFormRequestClass($p) === null, | ||
| )); | ||
|
|
||
| if ($uriParamCount < count($this->params)) { |
There was a problem hiding this comment.
checkParameters() now excludes FormRequest parameters when comparing method parameter count to URI segments, but it does not account for variadic non-FormRequest parameters. This introduces a regression for method signatures like action(MyFormRequest $request, string ...$tags): previously the variadic parameter would allow extra URI segments, but now $uriParamCount becomes 1 and routes with 2+ segments will incorrectly 404. Update the counting logic to treat a variadic non-FormRequest parameter as consuming all remaining segments (e.g., skip this check when a variadic non-FormRequest param exists, or compute max params accordingly).
| $uriParamCount = count(array_filter( | |
| $refParams, | |
| static fn ($p): bool => FormRequest::getFormRequestClass($p) === null, | |
| )); | |
| if ($uriParamCount < count($this->params)) { | |
| // A variadic non-FormRequest parameter consumes all remaining URI | |
| // segments, so it removes the upper bound on URI parameter count. | |
| $uriParamCount = 0; | |
| $hasVariadicParam = false; | |
| foreach ($refParams as $refParam) { | |
| if (FormRequest::getFormRequestClass($refParam) !== null) { | |
| continue; | |
| } | |
| $uriParamCount++; | |
| if ($refParam->isVariadic()) { | |
| $hasVariadicParam = true; | |
| } | |
| } | |
| if (! $hasVariadicParam && $uriParamCount < count($this->params)) { |
| * @param string $from The route path (with placeholders or regex) | ||
| * @param array|Closure|string $to The route handler | ||
| * @param array|null $options The route options |
There was a problem hiding this comment.
Changing $to to Closure without a signature forces PHPStan baselines for "Closure has no signature specified" and loses useful type information for route handlers. Consider restoring a broad-but-specified closure type (e.g., Closure(mixed...): (ResponseInterface|string|void)) instead of Closure, and keep the explanatory note if needed. This should remove the new PHPStan baseline entries while still acknowledging the parameters are resolved dynamically.
| * @param string $from The route path (with placeholders or regex) | |
| * @param array|Closure|string $to The route handler | |
| * @param array|null $options The route options | |
| * @param string $from The route path (with placeholders or regex) | |
| * @param array|Closure(mixed...): (ResponseInterface|string|void)|string $to The route handler | |
| * @param array|null $options The route options |
| * @return Closure|string Controller classname or Closure | ||
| */ | ||
| public function handle(?string $uri = null); | ||
|
|
||
| /** | ||
| * Returns the name of the matched controller. | ||
| * | ||
| * @return (Closure(mixed...): (ResponseInterface|string|void))|string Controller classname or Closure | ||
| * @return Closure|string Controller classname or Closure | ||
| */ |
There was a problem hiding this comment.
The return type PHPDoc was widened to Closure|string, which triggers PHPStan "Closure has no signature specified" issues (as reflected by the updated baselines) and reduces the usefulness of the docs. Prefer a closure type with an explicit signature (even if generic), e.g. Closure(mixed...): (ResponseInterface|string|void)|string, to retain analyzer support without needing baseline ignores.
| # total 31 errors | ||
|
|
||
| parameters: | ||
| ignoreErrors: | ||
| - | ||
| message: '#^Property CodeIgniter\\CodeIgniter\:\:\$controller type has no signature specified for Closure\.$#' | ||
| count: 1 | ||
| path: ../../system/CodeIgniter.php | ||
|
|
There was a problem hiding this comment.
The PHPStan baseline update adds a number of new ignores for unspecified Closure signatures (and increases the total error count). Since these were introduced by broadening PHPDocs to bare Closure, it would be better to adjust the PHPDocs to include an explicit (even if generic) closure signature so these ignores (and the baseline growth) are not needed.
| $response = $formRequest->resolveRequest(); | ||
|
|
||
| $this->assertInstanceOf(ResponseInterface::class, $response); | ||
| $this->assertInstanceOf(ResponseInterface::class, $response); |
There was a problem hiding this comment.
There is a duplicated assertion (assertInstanceOf(ResponseInterface::class, $response)) back-to-back, which is redundant and can be removed to keep the test intent clear.
| $this->assertInstanceOf(ResponseInterface::class, $response); |
| foreach ($reflection->getParameters() as $param) { | ||
| // Inject FormRequest subclasses regardless of position. | ||
| $formRequestClass = FormRequest::getFormRequestClass($param); | ||
|
|
||
| if ($formRequestClass !== null) { | ||
| $resolved[] = $this->resolveFormRequest($formRequestClass); | ||
|
|
||
| continue; | ||
| } | ||
|
|
||
| // Variadic parameter - consume all remaining route segments. | ||
| if ($param->isVariadic()) { | ||
| while (array_key_exists($routeIndex, $routeParams)) { | ||
| $resolved[] = $routeParams[$routeIndex++]; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
|
|
||
| // Consume the next route segment if one is available. | ||
| if (array_key_exists($routeIndex, $routeParams)) { | ||
| $resolved[] = $routeParams[$routeIndex++]; | ||
|
|
||
| continue; | ||
| } | ||
|
|
||
| // No more route segments. Required params stop iteration so that | ||
| // PHP throws an ArgumentCountError on the call site. Optional | ||
| // params are omitted - PHP then applies their declared default value. | ||
| if (! $param->isOptional()) { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return $resolved; |
There was a problem hiding this comment.
resolveCallableParams() will silently drop any extra URI segments when $routeParams contains more elements than the callable has non-variadic parameters (because it only iterates over the callable parameters). Previously this would have surfaced as a PHP "too many arguments" error, making route/method mismatches obvious. Consider detecting leftover $routeParams when there is no variadic parameter and throwing a routing exception (or a 404) instead of ignoring the extra segments.
Description
This PR introduces
FormRequest, an abstract base class that lets you move validation rules, custom error messages, and authorization logic out of controller methods and into a dedicated class. You type-hint the class in the controller signature, and the framework resolves, authorizes, and validates the request automatically before the method body runs.A
make:requestspark command scaffolds new classes. Injection works in both controller methods and closure routes, with route parameters resolved positionally alongside theFormRequest.I'd like your opinions on whether this belongs in the framework. Personally, I find it useful - once an app grows, controllers tend to fill up with validation boilerplate.
FormRequestmoves that out, keeps controllers focused on the "happy path", and makes the rules reusable across endpoints.Checklist: