Fix a validation issue on multiple file upload

[pjsde]

Fix a validation issue on multiple file upload mentionate in #2175

1. A function has been created in FileCollection where it will return null if it does not find the uploaded file or else return an array of multiple uploaded files so that a multiple upload can be validated.

2. Then the validation rules were changed so that it was checked first if the upload had been set with multiple files and if it was then validate all uploaded files returning error if any of them did not verify the rule.

Checklist:

• Securely signed commits
• Component(s) with PHPdocs
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[MGatner]

This looks like a nice expansion on uploads. One of the admins should look over it as well but in the meantime it will definitely need some tests and updates to the User Guide:

• user_guide_src/source/incoming/incomingrequest.rst
• user_guide_src/source/libraries/uploaded_files.rst
• user_guide_src/source/libraries/validation.rst

If you want help editing or reviewing those I would be glad to assist.

[lonnieezell]

I'm like this addition. I had one comment where I think you'll hit an exception if multiple files exist.

Definitely need tests and docs updates as @MGatner said.

[lonnieezell]

Pretty sure this needs an is_string() check before. An array would throw an exception under strpos.

[pjsde]

It doesnt need is_string() because $name is a string not an arrary.

[lonnieezell]

Doh! You're correct. Sorry, confused it when I saw the is_array check below. My bad for scanning too quickly.

[pjsde]

@MGatner it will be awesome if you could help with the editing/reviewing of the User Guide.

I will try to write some tests to validate the changes.

[pjsde]

By the way, I need some help with the following: I installed PHPUnit to be able to create and run the tests and detected that there are errors with the setUp() and tearDown() function definition. They should be as defined in the class TestCase:setUp():void / TestCase:tearDown():void.

I think there must have been an update in the TestCase class from PHPUnit where the return type was explicity declared as :void. I've already updated all classes that override these functions and added the return type: void to them.

Now my question is whether I should commit these changes to your framework or should I keep them local?

[lonnieezell]

I remember seeing that error previously, but I believe it had to do with the version of PHPUnit that was being used. It's been a while, though. For now - keep those locally.

Do you use a global PHPUnit install? If you use composer to install the dev dependencies locally it will provide the version that the framework is tested with in the vendor folder so vendor/phpunit/phpunit would be the command I believe. I've got a global alias setup that always expects to find it at that location so I just have to run phpu from the base of any of my apps. Long way of saying I may have that path wrong :)

[pjsde]

I'm not using a global installation. I did the installation with the composer but it must have gone wrong so I will have to see later then.

I will leave it locally and then see what happened and if i've installed a wrong version of PHPUnit.

Thanks

[pjsde]

I've just added now 5 tests to check getFileMultiple(). If you can see if its necessary more tests or even if this ones are ok, i'll apreciate.
Thanks

[MGatner]

I’m okay with this merge but let’s be sure the User Guide gets an update at some point.

[jim-parry]

Agreed. There are a couple of user guide updates in the works as housekeepig prep for rc.2 :)