BC-Break in 2.4.1? Really annoying NotEmpty injection by default

[manuakasam]

Hey guys, I've run into troubles at work with our InputFilters behaving very weird when updating from 2.4.0 to 2.4.2. The change that gave us trouble actually happened in 2.4.1 and it's been this one right here:

https://github.com/zendframework/zf2/pull/7474 https://github.com/zendframework/zf2/commit/ffad7925a62627f9a28a077824db1dfc724d7321#diff-eba196bb0648a9366fa5c59a0edb701b

     public function setRequired($required)
     {
         $this->required = (bool) $required;
-        $this->setAllowEmpty(!$required);

         return $this;
     }

The removal of this little snippet has led to the injection of the NotEmpty validator by default. I have created a little test to outline my problem:

public function testWtf()
{
    $data = [
        'foo' => '0'
    ];

    $inputFilter = new InputFilter();
    $inputFilter->add([
        'name'       => 'foo',
        'required'   => false,
        'filters'    => [
            ['name' => 'Boolean']
        ],
        'validators' => []
    ]);

    $inputFilter->setData($data);

    $this->assertTrue($inputFilter->isValid());
}

This is basically how I wrote the InputFilter. Now knowing the internals of the InputFilter (after doing some digging), i now know that I can simply workaround this by adding allow_empty => true to the inputFilter configuration, but I'm asking myself "why?".

2.4.1 had only Bugfixes but in my opinion this is clearly a BC Break as it is breaking existing stuff.

I'm trying to figure out the reasoning for having the NotEmpty validator be added by default. So if anyone could elaborate on this, that'd probably be a huge help to me.

Aside from this, but this may be offtopic to this component: why is NotEmpty behaving the way it is? Why is 0 or false be considered empty be default?

Those two things combined are really tricky to get around and I'm using this framework pretty regularly ^^

[gianarb]

mm i don't know why this change.. but it seems a BC Break o.O

EDIT: the commit is into the main post sorry

[weierophinney]

I can tell you exactly what happened. First, the issues/patches that led to it:

• zendframework/zf2#4567
• zendframework/zf2#7445
• zendframework/zf2#7448
• zendframework/zf2#7474

Essentially, the workflow for required + allow_empty was tricky, and zendframework/zf2#4567 attempted to make it simpler. However, in doing so, it made the flags order dependent. If you wanted to omit the "not-empty" validator, you were required to call the setters for the required and allow_empty flags in the following order:

$input->setRequired(true);
$input->allowEmpty(true);

This broke down if you used configuration-driven input filters, as there was no way to enforce the order in which the flags would be set, and if you set the required flag after the allow_empty flag, you would override the value you set for allow_empty! So, we shipped 2.4.0 with a BC break, which was reported in zendframework/zf2#7445.

zendframework/zf2#7448 reverted the flag toggling that happened inside setRequired(). Unfortunately, this didn't take into consideration the continue_if_empty flag's default value, which led to a situation where the combination of required === true && allow_empty === true && continue_if_empty === true would not pass the value on to the composed validator chain.

As such, I submitted zendframework/zf2#7474, which codified the expected interactions between the three flags. There are 8 interactions in total, and that issue has a list of them. That issue itself is also a BC break, but was necessary to ensure (a) that the expected behaviors are properly codified (i.e., we have tests for each), (b) that no setter changes the state of a different property (which was what led to the initial break), and (c) that original behavior pre-2.4 is mostly intact. With regards to (c), the tests for the various behaviors essentially showed that we had one or two cases where the expected behavior was not implemented correctly.

Based on what you described, you definitely want the combination of:

• required === true
• allow_empty === true

That combination does exactly what you're trying to accomplish: it allows empty values to skip validation entirely, and will not result in injection of the not-empty validator into the validator chain. That was the original intent for the allow_empty flag.

[manuakasam]

Thanks for the detailed description of what was going on @weierophinney I knew about some of these things going on, but since 2.4.1 specifically didn't state any BCBreak I got a bit confused ;)

Seems a bit counter-intuitive at first to make something required (but allow_empty) when a field (going by logic) simply isn't required. However knowing this it is fine, as I have outlined in my first post. I just wanted to make sure that nothing slipped through the cracks ;)

Thanks again