Bug #50585

Validation fails if nested properties of same type are present

Added by Martin Lipp almost 8 years ago. Updated about 7 years ago.

Status:
Resolved
Priority:
Must have
Category:
Validation
Target version:
-
Start date:
2013-07-31
Due date:
% Done:

100%

Estimated time:
PHP Version:
Has patch:
No
Complexity:

Description

The GenericObjectValidator dumps all prior calculated validation results, when validating a nested property which has the same class name as the validated object.

Quick example:

class Office {
    /**
     * @var string
     * @Flow\Validate(type="StringLength", options={ "minimum"=3, "maximum"=100 })
     * @Flow\Validate(type="NotEmpty")
     * @ORM\Column(length=100)
     */
    protected $title;

    /**
     * @var \Doctrine\Common\Collections\Collection<\Company\Project\Domain\Model\OpeningHours>
     * @ORM\OneToMany(mappedBy="office")
     */
    protected $openingHours;

    ...

class OpeningHours {
    /**
     * @var \Company\Project\Domain\Model\Office
     * @ORM\ManyToOne(inversedBy="openingHours")
     */
    protected $office;

    ...

When creating a new Office with a form including one ore more openingHours, all validation errors of properties defined prior to $openingHours are dumped and therefore not displayed as errors. If no other validation error after the $openingHours property occurs, the createAction is executed with no error messages (but not persisted, seems that the validation here works). In the above example an empty $title would pass the validation.

I've tracked it down to the GenericObjectValidator and the ValidatorResolver:
The ValidatorResolver only creates a new baseValidatorConjunction, if none exists for this class name. Otherwise it uses the existing one. Since the GenericObjectValidator's validate() method resets its member $this->result on every call, all results of prior Office properties are deleted as soon as it comes to the validation of the openingHours' $office property.
This does not happen in one of my older projects (with FLOW3 1.1), because there the GenericObjectValidator does not store its results in a member variable, but only calculates it inside the method.
Also it only seems to happen inside collections.


Related issues

Related to TYPO3.Flow - Task #46340: Improve validation to speed up FlowResolved2013-03-15

Actions
Related to TYPO3 Core - Bug #91029: Validation fails if nested properties of same type are presentNew2020-04-14

Actions
#1

Updated by Martin Lipp almost 8 years ago

I was able to reproduce the bug in the TYPO3.Blog package. Simply add the following form elements to the Post/New.html form:

<f:form.textfield id="comments.0.author" property="comments.0.author" />
<f:form.textfield id="comments.0.emailaddress" property="comments.0.emailAddress" />
<f:form.textarea id="comments.0.content" property="comments.0.content" rows="10" cols="50"/>

Now you are able to add one comment directly when creating a post. When submitting the empty form, only the validation errors for the comment are shown. No errors for the post's title, post's author, etc. are displayed anymore. This is because the models are of the same structure as in my above example (Office being Post, and OpeningHours being Comment). Filling the form elements for the comment only, the validation passes and a new Post is created (with an empty title, no content text, ...).

A possible solution would be, to skip the validation of the $post property in the Comment model, if and only if the Comment is created as a nested property of a collection. I'm guessing this could be achieved by checking the mappedBy annotation of the Post's $comments property and skipping the property in Comment which is defined by the mappedBy annotation. This should be safe, since $post is just a back reference to the currently created Post (where the validation happens anyway).

#2

Updated by Bastian Waidelich almost 8 years ago

  • Status changed from New to Accepted
  • Assignee set to Bastian Waidelich

This is a severe bug and I can reproduce it.

#3

Updated by Helmut Hummel almost 8 years ago

Martin Lipp wrote:

This does not happen in one of my older projects (with FLOW3 1.1), because there the GenericObjectValidator does not store its results in a member variable, but only calculates it inside the method.

Are you sure you mean FLOW3 1.1 and not 1.0?
The GenericObjectValidator looks quite similar in 1.1 and 2.0

#4

Updated by Martin Lipp almost 8 years ago

Helmut Hummel wrote:

Martin Lipp wrote:

This does not happen in one of my older projects (with FLOW3 1.1), because there the GenericObjectValidator does not store its results in a member variable, but only calculates it inside the method.

Are you sure you mean FLOW3 1.1 and not 1.0?
The GenericObjectValidator looks quite similar in 1.1 and 2.0

Yes. I checked again it works fine with FLOW3 1.1. I also quickly migrated the package to Flow 2.0, where the bug appears immediately. But you're right there is no difference in the GenericObjectValidator (don't know what I saw back then). The bug must be somewhere else, although it is true that - in many cases - $this->results is not empty when entering the validate() method, where it is overwritten with an empty result.

#5

Updated by Gerrit Code Review almost 8 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24218

#6

Updated by Gerrit Code Review almost 8 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24218

#7

Updated by Helmut Hummel almost 8 years ago

Martin Lipp wrote:

Yes. I checked again it works fine with FLOW3 1.1. I also quickly migrated the package to Flow 2.0, where the bug appears immediately. But you're right there is no difference in the GenericObjectValidator (don't know what I saw back then). The bug must be somewhere else, although it is true that - in many cases - $this->results is not empty when entering the validate() method, where it is overwritten with an empty result.

Can you check if the patch in Gerrit works for you? Thanks.

#8

Updated by Helmut Hummel almost 8 years ago

Breaking change was in #46340

#9

Updated by Gerrit Code Review almost 8 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24218

#10

Updated by Martin Lipp almost 8 years ago

Helmut Hummel wrote:

Martin Lipp wrote:

Yes. I checked again it works fine with FLOW3 1.1. I also quickly migrated the package to Flow 2.0, where the bug appears immediately. But you're right there is no difference in the GenericObjectValidator (don't know what I saw back then). The bug must be somewhere else, although it is true that - in many cases - $this->results is not empty when entering the validate() method, where it is overwritten with an empty result.

Can you check if the patch in Gerrit works for you? Thanks.

It works (at least patch 2 does).

#11

Updated by Gerrit Code Review almost 8 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24218

#12

Updated by Helmut Hummel almost 8 years ago

Martin Lipp wrote:

Helmut Hummel wrote:

Can you check if the patch in Gerrit works for you? Thanks.

It works (at least patch 2 does).

Thanks.

Patch 3 basically does the same (keeping the validation results of the top level object, when recursing through its properties).

#13

Updated by Helmut Hummel almost 8 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#14

Updated by Gerrit Code Review over 7 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch 2.0 of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/25875

#15

Updated by Gerrit Code Review over 7 years ago

Patch set 2 for branch 2.0 of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/25875

#16

Updated by Helmut Hummel about 7 years ago

  • Status changed from Under Review to Resolved
#17

Updated by Daniel Schultheis over 1 year ago

  • Related to Bug #91029: Validation fails if nested properties of same type are present added

Also available in: Atom PDF