NotEmpty validator doesn't override the required attribute

[larsnystrom]

A NotEmpty validator should override the required check on an input, but in some cases it doesn't. I'm sorry I haven't been able to pin point the exact location of the bug, but here's the code to reproduce it. In test case 1 both NotEmpty validators overrides the required attribute, which can be seen by the error messages. In test case 2 however, field2 doesn't use it's NotEmpty validator but instead falls back to it's internal check to see if the value is empty.

<?php

require __DIR__ . '/vendor/autoload.php';

class MyFilter extends Zend\InputFilter\InputFilter
{
    public function init()
    {
        $this->add([
            'name' => 'field1',
            'required' => true,
            'validators' => [
                [
                    'name' => 'Zend\Validator\NotEmpty',
                    'break_chain_on_failure' => true,
                    'options' => [
                        'messages' => [
                            'isEmpty' => "Field1 cannot be empty",
                        ],
                    ],
                ],
            ],
        ]);

        $this->add([
            'name' => 'field2',
            'required' => true,
            'validators' => [
                [
                    'name' => 'Zend\Validator\NotEmpty',
                    'break_chain_on_failure' => true,
                    'options' => [
                        'messages' => [
                            'isEmpty' => "Field2 cannot be empty",
                        ],
                    ],
                ],
            ],
        ]);
    }
}

$app = Zend\Mvc\Application::init([
    'modules' => [
    ],

    'module_listener_options' => [
        'module_paths' => [
        ],

        'config_glob_paths' => [
        ],
    ],

    'input_filters' => [
        'invokables' => [
            'MyFilter' => 'MyFilter',
        ],
    ],
]);

$manager = $app->getServiceManager()->get('InputFilterManager');
$filter = $manager->get('MyFilter');

echo "\n";
echo "#------------------------------\n";
echo "# Test case 1\n";
echo "#------------------------------\n";
echo "\n";
$filter->setData([
]);

if (!$filter->isValid()) {
    var_dump($filter->getMessages());
}

echo "\n";
echo "#------------------------------\n";
echo "# Test case 2\n";
echo "#------------------------------\n";
echo "\n";
$filter->setData([
    'field1' => [
        'test',
    ],
]);

if (!$filter->isValid()) {
    var_dump($filter->getMessages());
}

The output I get is:

#------------------------------
# Test case 1
#------------------------------

array(2) {
  'field1' =>
  array(1) {
    'isEmpty' =>
    string(22) "Field1 cannot be empty"
  }
  'field2' =>
  array(1) {
    'isEmpty' =>
    string(22) "Field2 cannot be empty"
  }
}

#------------------------------
# Test case 2
#------------------------------

array(1) {
  'field2' =>
  array(1) {
    [0] =>
    string(17) "Value is required"
  }
}

[tflin]

We had the same issue that 'Value is required' message can not be localized to Chinese as it is hardcoded in the framework code. The custom-message NotEmpty validator cannot override this error message like this case. Please help to make the hardcoded message localizable.

[Maks3w]

@tflin please open a new issue

[Maks3w]

@larsnystrom I don't understand what do you refer about override required attribute. I can say the second commit of https://github.com/zendframework/zend-inputfilter/pull/25 will make the messages consistent using the message of test 2

[svycka]

@Maks3w I think he speaks about this https://github.com/zendframework/zend-inputfilter/blob/bded51bd893b828c0332035221e42793ec201ee1/src/BaseInputFilter.php#L270 and they speak about the same problem so no need for new issue.

[larsnystrom]

I've created a repo to make it easier to reproduce the bug.

git clone https://github.com/larsnystrom/zend-inputfilter-ticket14
composer install
php test.php

I haven't tested PR #25, I'll do that if I ever get the time.

[weierophinney]

@larsnystrom We've never had setting a NotEmpty validator toggle the required flag; the only correlation between the two is that in past versions, setting the required flag to true would implicitly add a NotEmpty validator. We've changed that logic recently, however (we now check for absence of a value when the required flag is true, and invalidate earlier when that condition arises). Is it possible that's the issue you're seeing?

[larsnystrom]

@weierophinney If you look at the MyFilter::init() method in the test code above, you'll see that I've set required to true and also added a NotEmpty validator to both input specifications in the InputFilter. Then, if a field is empty, the NotEmpty validator should run and fail, right? As you yourself wrote in #11

The solution to #11 is to keep the input as required, but to inject your own NotEmpty validator at the top of the chain yourself; Input will detect that it's present, and not inject its own NotEmpty validator.

So, the solution is:

• Keep the input required
• Inject your own NotEmpty validator with the custom message as the first validator in the chain.

That is exactly what I've done.

Now, if you look at test case 1, you'll see that this works when both fields are empty! Fantastic.

Moving on to test case 2, where only one of the fields are empty. I'd expect the corresponding NotEmpty validator to run and fail, right? That's what happened in the first test case. But (here comes the bug) the NotEmpty validator doesn't run and instead the required attribute and its associated logic takes precedence.

The behaviour is different when (1) all fields are empty and (2) only one field is empty. That's the bug.

[Maks3w]

Fields on test 1 are not empty. Are not set. Value is unknown so can't be determined if value is empty or not.

[Maks3w]

Please test against current master

[larsnystrom]

The current master solves the inconsistence, but makes it completely impossible to customize the "Value is required" message when there isn't a value. But that's discussed in other issues (#11 and #28), so I'll close this issue now.

Another issue is that the fallback value isn't validated. Personally I'd prefer if it was validated/filtered just like any other value. But that's outside the scope of this issue.