sroze / SRIORestUploadBundle

A symfony bundle to handle multiple upload ways on your REST API.
46 stars 17 forks source link

Removed Content-Length requirement and unused use statement #12

Closed inverse closed 9 years ago

inverse commented 9 years ago

When I tried to do a composer install it said that it was out of date. Is it worth commiting that too or just removing the lock file?

I am running on Windows 8.1 and ran phpunit but get the following: http://pastebin.com/bJRB2dq1

Related to #11

sroze commented 9 years ago

You shouldn't have to edit the composer.json... And 2 tests are failing now, update them or fix the problem besides please :)

inverse commented 9 years ago

Sorry I should have been more clear - I meant the compose.lock was stating it was out of date. As for the failing tests I will look into this for you :) I'm relatively new to Symfony and seeing your bundle has helped me understand stuff a little better.

inverse commented 9 years ago

Alternatively I could init Content-Type to '' and modify the checkHeaders method to check for length too?

protected function checkHeaders (Request $request, array $headers)
{
    foreach ($headers as $header) {
        if (!$request->headers->has($header)) {
            throw new UploadException(sprintf('%s header is needed', $header));
        } else {
            $requestHeader = $request->headers->get($header, null);
            if ($requestHeader === null || strlen($requestHeader) === 0) {
                throw new UploadException(sprintf('%s header must not be empty', $header));
            }
        }
    }
}
inverse commented 9 years ago

@sroze Any suggestion?

sroze commented 9 years ago

You're right, a workaround is to set the Content-Type to an empty string in test and then check the length of string in checkHeaders.

By the way, there's a simple way than your purposed snippet:

protected function checkHeaders (Request $request, array $headers)
{
    foreach ($headers as $header) {
        $value = $request->headers->get($header, null);

        if ($value === null) {
            throw new UploadException(sprintf('%s header is needed', $header));
        } else if (empty($value)) {
            throw new UploadException(sprintf('%s header must not be empty', $header));
        }
    }
}
inverse commented 9 years ago

I like your proposed snippit better although doing the check for empty makes another test fail due to CONTENT_LENGTH being set to 0.

ResumableUploadTest->testChunkedUpload()

sroze commented 9 years ago
else if (!is_int($value) && empty($value))

?

inverse commented 9 years ago

That makes more sense ;D having a slow day :(

sroze commented 9 years ago

Great, thanks for your work :)