CollectionInputFilter->getCount() gives wrong count on consecutive setData() calls

[halaxa]

If no count is explicitly set using setCount() on CollectionInputFilter, evaluated count($this->data) is saved and then served across different data set by setData() even if it it's valid no more. This patch makes it not to save temporary value across different data.

[danizord]

Ping @fabiocarneiro

[halaxa]

It therefore fails validating consecutive data sets of different count as isValid() calls getCount(). So it's an unpleasant bug. Shall i make an issue?

[Ocramius]

@halaxa who is using setCount? Can the property be completely removed?

[halaxa]

I'm not:) I understand that setCount() may be used to add a count check on the given data. It is not valid if actual count is lower than set count. There is also config key for it. But to me it seems redundant as one can easily use some kind of more variable count validator in validation chain if needed although I don't see any present right now.

[Ocramius]

@halaxa can you find its usages and eventually mark it as @deprecated? It should just do nothing.

[Ocramius]

Also, marking this as a BC break and delaying to 2.4.0

[halaxa]

What if we split it up and use this patch now and the other removing count property later in 2.4?

[halaxa]

Our project kind of needs this to be just fixed, nothing more:)

[danizord]

I'm not sure why the count property is not being used anymore, that's really needed:

$inputFilter = new \Zend\InputFilter\InputFilter();
$inputFilter->add([
    'name' => 'collection',
    'type' => 'Zend\InputFilter\CollectionInputFilter',
    'options' => [
        // Requires 3 elements
        'count' => 3,
    ],
]);
// Fill with 2 elements
$inputFilter->setData([
    'collection' => [
        ['name' => 'item1'],
        ['name' => 'item2']
    ]
]);
$this->assertFalse($inputFilter->isValid());

[Ocramius]

@danizord is it a documented behavior? Looks kinda silly to me.

@halaxa you could rely on either your internal branch or dev-develop until this is merged into master.

[fabiocarneiro]

#edit Sorry, this PR is right and should be merged. It does not break bc.

@Ocramius We can't remove the property because it will still be used by the factory. It will only be used if you set the required count in the spec.

[danizord]

@Ocramius Yes, that behavior is covered by testDataLessThanCountIsInvalid().

[halaxa]

What about just fixing current behaviour and create an issue about this?

[Ocramius]

Merged, thanks!