#4996 broke File filters management

[Slamdunk]

I've found PR #4996 broke how custom file input filter specification is handled; both 2.2.3 and 2.2.4 releases are affected.

I've tried hard to figure out why, but I'm sorry I was not able to 😕

Here the tests: note that both assertions fail, not only the first.

[samsonasik]

remove @Package

[weierophinney]

There's a flaw in the test here. You need to specify an input type in order for the merge order to work correctly; otherwise, a normal Input will be created, and the FileInput settings merged to it, which will result in the wrong context.

That said, even with 'type' => 'Zend\InputFilter\FileInput' in your configuration, you still get an error -- just a different one (duplicated filters).

[Slamdunk]

@weierophinney so from release 2.2.3, the default behaviour is changed: till 2.2.2 https://github.com/zendframework/zf2/blob/release-2.2.2/library/Zend/Form/Element/File.php#L46-L53 was enough and it didn't need to be specified a second time in a custom getInputFilterSpecification.

This is a compatibility break:

1. If it was intentional, please document it on changelogs (I can't see it in http://framework.zend.com/blog/zend-framework-2-2-3-released.html)
2. If it was not intentional, I think the 2.2.2 default behaviour should be restored

[weierophinney]

The bug fix for 2.2.3 was intentional, and documented. You were relying on
behavior that was neither documented nor tested - in other words, you were
relying on the bug itself. As such, we did not cover it in the changelog,
as we did not know developers were utilizing it.

This fix I introduce here covers a new issue introduced with #4996, which
was the duplicate application of input filter specifications, and which was
uncovered by your test case once I updated it to include the input type.

When we release 2.2.5, I will note the change, and also point out the
changes users should make to their input filter specifications.

On Wednesday, September 4, 2013, Filippo Tessarotto wrote:

@weierophinney https://github.com/weierophinney so from release 2.2.3,
the default behaviour is changed: till 2.2.2
https://github.com/zendframework/zf2/blob/release-2.2.4/library/Zend/Form/Element/File.php#L46-L53was enough and it didn't need to be specified a second time in a custom
getInputFilterSpecification.

This is a compatibility break:

1. If it was intentional, please document it on changelogs (I can't
   see it in
   http://framework.zend.com/blog/zend-framework-2-2-3-released.html)
2. If it was not intentional, I think the 2.2.2 default behaviour
   should be restored

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5050#issuecomment-23768758
.

Matthew Weier O'Phinney
[email protected]
http://mwop.net/

[Slamdunk]

Ok, I understand.
I hope you'll realize that the following paragraphs are not polemic, but just a way to understand if ZF2 users can get it.

If type key is not mandatory, a new developer would think it is inherited.
Otherwise a weird situation could happens:

1. Write down a new My\Form, tests pass
2. Write a custom getInputFilterSpecification, tests fail
3. Why tests fail? Oh, I didn't write the type key
4. I need to look at each original Zend\Form\Element i'm using to see which type it needs

This is strange, isn't it?

If it sounds strange for you too, either type must be mandatory or must be inherited.

If this is ok for you, at least must be well documented on http://zf2.readthedocs.org/en/latest/modules/zend.form.quick-start.html#hinting-to-the-input-filter to avoid debugging headaches.

[weierophinney]

@Slamdunk We have a default type, Zend\InputFilter\Input. This is reasonable for 99% of use cases. The only time it is not is when you have specialized input types, such as the FileInput.

It does not make sense for the most common use case to require that people add the type. However, it's not unreasonable to require it for those cases where a specialized type is being used.

As such, the exceptional case needs to be documented. Would you be willing to do a PR against the documentation for that?

[Slamdunk]

@weierophinney what about something like this #5079?

I will also PR against the doc.