Validation to only validate properties submitted if not a posting a new entity

[silverbackdan]

During a put/patch request - it doesn't seem applicable to validate fields that have not been submitted for change. In my case it was definitely not desired. When filling in a form, I'm looking to update each field as a user progresses through the form. Currently, this will send back violations for every field that failed validation and therefore not update the entity. Thoughts?

The Nelmio Behat test is now failing with a 500 error but I haven't traced the cause for this yet and am not sure whether it is related to this PR.
Edit: Must have just been on my local machine and an unrelated issue. Tests pass fine on the automated tools

I added a PHPUnit test for this functionality.

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	n/a

[silverbackdan]

SymfonyInsight is failing due to this error

[Composer\Downloader\TransportException]                                                               
  The "https://repo.packagist.org/packages.json" file could not be downloaded: SSL: Handshake timed out  
  failed to open stream: Cannot connect to HTTPS server through proxy   

I'm pretty sure this is just a SI error and not related to this PR. Sorry for the number of commits, I realised I hadn't injected the denormalizer into the Validator so tests passing were a false positive before. I think this would be OK now. I had a think about trying to make this option configurable, but I wasn't sure that it was needed. I couldn't think up a use case but I'm happy to improve and learn if this is a useful feature for others too. Thanks!

[antograssiot]

The default behavior looks the good one to me. Validation must take the entire entity state. When I have cases like yours I use validation groups.
You could even use https://api-platform.com/docs/core/validation/#dynamic-validation-groups

[silverbackdan]

@antograssiot I was really trying to use validation groups, but it'd involve making a specific group for every property. Without creating a custom metadata factory I couldn't work out a way to dynamically apply groups to every property. Then I'd have to detect the properties submitted so I can then dynamically change the validation groups being used. This is because the desired user interface isn't to submit a group of properties at a time but each individual property. I could be missing something incredibly obvious though and sorry if I am.

[antograssiot]

It should at least be opt-in from my perspective but core members may have a different opinion 😄

[silverbackdan]

I forgot to say in my first comment thank you very much for taking time to give feedback! I'm happy to make that change, but before dedicating more time to adding that I'll wait to hear from the core team. :) I'm not sure if this would improve performance at all by not validating the entire object (I haven't run any benchmarks) or if it'd be classed as a breaking change without an opt-in option. It'd be good to know a use-case when all fields would want to be validated even if they weren't all submitted but I haven't thought of one myself yet. Thanks again @antograssiot

[dunglas]

Instead of this patch, who looks very specific to me (too specific to be in core), cannot we add a request to attribute to skip validation (and so let the user does its custom one)? An attribute similar to the existing _api_receive/_api_respond/api_persist https://api-platform.com/docs/core/events/#the-event-system

It could be api_validate

[silverbackdan]

No problem - do you think this would be necessary or would it be best for me to just decorate/override the validator? I could always inject my own validator into ApiPlatform\Core\Bridge\Symfony\Validator\EventListener\ValidateListener which would mean I don't loose the validation groups.

The attribute to skip validation may just end up being a slight performance boost for users who don't need validation - but my initial thoughts on that may be that 99% of users will use some kind of validation and the performance boost would be negligible.

I'm happy to create a PR if you think it's worth it though.

[silverbackdan]

I've been thinking about this again. Probably for my use case the best thing is to inject my own validator into the ApiPlatform\Core\Bridge\Symfony\Validator\Validator class. Thanks for taking the time to look at this though. I'll close this PR as if you do think an api_validate attribute is useful it'd be a separate PR in any case.