Laravel Data two-step validation #470
Replies: 2 comments 2 replies
-
I'm not fully grasping all concepts you've shared, but I'm all in for better validation. Coincidentally I started working on an MVP today, where we're rewriting our API calls with other services. It is not uncommon for some services to require and return 50+ items in a request. To validate them with attributes makes for confusing code. Maybe there's an alternative for this that I don't know about, but if not and there will be a validation revamp, I would love to see a solution for this too. |
Beta Was this translation helpful? Give feedback.
-
I think one danger with the current implementation is that people not familiar with this library are not immediately aware of the differences between My initial assumption from the documentation was that the following would throw an exception: class FooDTO extends Data
{
public function __construct(
#[Max(2)]
public string $fooString,
) {}
}
new FooDTO(fooString: 'too long'); That kind of magic would be hard to achieve I guess, if there's nothing being called to process the attributes. My personal use case has very little to do with Laravel Form Requests, and more about passing DTOs between domains/contexts in a safe and easily testable/mockable way -- strict/self-validating DTOs can be very useful for that. I respect that everyone's use cases are different, and I don't really have a strong opinion on solutions. But I do think it could help to add some clear examples to the docs, to explain when validation is applied and when it isn't. Maybe also allow My current solution to force strictness is not super elegant, but it works fine: public function __construct(
#[Max(2)]
public string $fooString,
) {
$this->validate($this->all());
} |
Beta Was this translation helpful? Give feedback.
-
Lately, there needs to be more clarity on when and how laravel-data validates things. At the moment, this is how it works:
If you have a data object with custom-defined magic creation methods, nothing is validated, no property names are mapped, and the method is run to create the object. Unless you're passing in a Request, which always will be validated. The catch here is that this validation happens before creating the data object. And thus, things like this are complicated:
The solution is adding #[WithoutValidation] to the property, which stops the error but is weird. Another solution could be adding a hook before validating where the request would be cleaned up, but you probably don't want to mess with the Request object.
Almost the same thing is happening with
validateAndCreate
, which will validate everything passed to it.The reason why we're doing this? Putting data in PHP objects can cause exceptions. Putting
null
in apublic string $property
will fail hard with all kinds of doom Exceptions and Errors, and this is a scenario we want to avoid. Also, casts run at this point. We cannot put a string into a Carbon object; we must cast it before creating the data object.That being said, the setup we're having now is complicated, and it would be a lot easier to validate things if they are already in the data object.
Proposal
A two-step validation system. Let's look at the from method. If you haven't defined a magic creation method, the package will do its best to create the object by intelligent mapping and casting properties. In this case, we would still do this and generate validation rules. But these validation rules would be the bare minimum to build a valid PHP object.
For a type, this would mean
These rules would then be used to validate the payload created by the pipeline. If it is invalid, we return a ValidationException and don't create the PHP object. If it is valid, we create the data object and run a second validation step where we use all the custom rules, attributes rules, and so on defined on the data object.
This second validation step will only be executed when a request is passed. But there would be a few options to trigger this for other payloads:
always_validate_when_using_from
, would be false by defaultallwaysValidateWhenUsingFrom(): ?bool
would be null by default so that if not defined, it doesn't decide if we're validating or notfromWithValidation(mixed ...$payloads)
which always validateThe reason why we're not validating by default:
exist
in a collection of values is a no-go! But since each data object makes its own rules, we don't have this visibility which is kinda dangerous.So by default, only request validation but a manual opt-in for always enabling validation.
Trade-offs and inner working
Casts
Currently, casts run after validation, so they know their data is valid. This would not be the case after this change since casts would have to run after the first validation step but before the object creation and, thus second validation step, which usually validates the data before it is passed to the cast.
As a solution, we could let casts define rules that would run in the first validation step. Then, for example the
DateTimeInterfaceCast
would be able to check if the passed value is a string and a date.Custom rules per data object
These would run in the second validation step, which means an opportunity. Since the data object already exists, we can pass it through.
Validation attributes
This is a tricky one. Some validation attributes will break since they must work on more complex types. For example, a
#[Date]
rule will not work on a string but on a Carbon or DateTime object with the new implementation. Luckily the Laravel validator is smart enough, in this case, to still validate the object. But there will probably be other cases where we'll have some things breaking.Pipeline
It would be the same, but the validation pipe would not run anymore.
Mapping of property names + injecting of defaults
It would be the same and still run in the pipeline. Where would the mapping be used within the validation? The second validation step would always use the property name, that's out of question. But the first step would logically use the mapped names when generating validation rules.
Attributes and messages (in the validator)
It should be generated on the initial validation step since you may also want to overwrite these labels or messages. It could be reused further down the line in the second validation step.
ValidateAndCreate method
It would be removed since we now will have fromWithValidation.
GetValidationRules method
We must return rules from the initial + second validation step merged.
Two step tules
We would run validation twice, the first time with basic rules, the second time with more complicated ones. So for new users of the package this might seem odd that the validation could fail twice in a row with different rules.
That being said, the first step would be the most essential rules, just to make PHP not crash and being able to create the object, if you've written your code well this shouldn't fail normally.
Extra's besides two step validation
Laravel precognition
We want to start supporting Laravel Precognition, this should complete two validation steps before being valid.
Validating from non simple values
This is not working richt now:
We expect an array but get an
DocumentData
object, an an ideal world this would work. Implementing this also has a few quirks: #481Conclusion
This is a first draft of what could be done. I probably forget a lot of edge cases, so feel free to add a comment when you see one. This thread will only be about refactoring validation into this new form, new features can be added to the larvel-data v4 discussion.
I'm not entirely sure this is the way we want to fix this, if we want to fix this. It is a lot of work, and some things will be broken entirely for this change.
If we're doing this, then v4 will be the target.
Let me know what you think!
Beta Was this translation helpful? Give feedback.
All reactions