Description

Solution to ZF-8733 breaks a current unit test, namely {{Zend_Form_Element_FileTest::testFileSize}}, which relies on {{Zend_File_Transfer_Adapter_Abstract::getFileSize()}} to inspect the actual file size by means of the filesize() function, but since r20141 only returns the file's size attribute (which is falsely set in the Mock adapter). A function called _detectFileSize() is created, but only used in {{Zend_File_Transfer_Adapter_Http}}. Not quite sure what changing getFileSize() function had to do with the references issue though.

Comments

Posted by Thomas Weidner (thomas) on 2010-01-12T04:20:50.000+0000

Fixed with r20215

Thanks for the report
Note: User provided filesize is seen as unsecure/incorrect and therefor no longer be used

Posted by Menno Luiten (mluiten) on 2010-01-12T04:36:24.000+0000

Isn't that all the more reason to call _detectFileSize() (and _detectMimeType() for that matter) inside the Abstract class as well? In the current implementation, any adapter inherited from Abstract can now determine it's own filesize by setting the size attribute, instead of a check on the filesystem; or is that by design?

Posted by Thomas Weidner (thomas) on 2010-01-12T04:52:03.000+0000

Expect an adapter which uses a service.
The service sends the file afterwards and the informations before.

There would be no way to implement this without transferring the file BEFORE accessing the informations. In HTTP the file is send in combination with the informations so this does not matter for HTTP.

Conclusion: This is by design.

Posted by Menno Luiten (mluiten) on 2010-01-12T04:58:24.000+0000

In case the answer to the above question is "no", I would propose to add the attached patch; which used _detectFileSize inside the MockAdapter instead of 'relying' on metadata as given by the adapter as an additional unit test. To replicate the 'old' behaviour.

Posted by Menno Luiten (mluiten) on 2010-01-12T05:03:39.000+0000

In the service-case, that indeed makes sense... but file size will not be returned if the file is not physically found:

To your first question:
No... there is no need to test if an extended adapter works correct.
_detectFileSize() is a method of Zend_File_Transfer and not the File element.

Additionally there is no need to replicate old (for the file element broken) behaviour. The unittests work as they are intended to work.

To your second question:
Also wrong... getFileSize() does not always throw an exception when no file was given. When the option "ignoreNoFile" was set then there is a returned value (null/0). The wished behaviour can be set by the extending adapter.

Posted by Menno Luiten (mluiten) on 2010-01-12T06:07:16.000+0000

To the additional unit test: you are right... wrong place for the wrong task. thank you :)

To the second part: Not exactly what I meant. I was suggesting that in your Service Adapter analogy (where mime-type, file size, etc is sent before the actual transfer), requesting getFileSize() would always return either an Exception or 0/null, even if the service would set it to something useful. Same story with the mime-type. So while not calling _detectFileSize in Abstract, makes it possible to first send metadata, it will not return those values before the file transfer is complete. Right?

Thanks for your time, just coming to grasps with the inner workings of ZF, one step at a time ;)