slimphp / Slim

Slim is a PHP micro framework that helps you quickly write simple yet powerful web applications and APIs.
http://slimframework.com
MIT License
11.98k stars 1.95k forks source link

Slim3: Apparent duplicated code "isEmptyResponse" #1614

Closed rbairwell closed 8 years ago

rbairwell commented 8 years ago

Just glancing at the code shows that in Slim\App->finalize it receives a ResponseInterface an then calls "$this->isEmptyResponse" to filter out the body/content-type and content-length. However, Slim\Http\Response also has an identical isEmptyResponse method (which is not used).

I feel that Slim\App should be seeing if the Response has a isEmptyResponse method and be calling that and only falling back to its internal method if the Response does not support it (i.e. it is a "strictly PSR Response Interface" class). This will allow other parties to drop in their own ResponseInterface supporting objects which may indicate other status codes (etc) could be "empty".

geggleto commented 8 years ago

Related Code Highlights: https://github.com/slimphp/Slim/blob/3.x/Slim/App.php#L538-L540

https://github.com/slimphp/Slim/blob/3.x/Slim/App.php#L390

https://github.com/slimphp/Slim/blob/3.x/Slim/App.php#L561

Probably needs to be switched too...

if ($response->getBody()->getSize() > 0) 
codeguy commented 8 years ago

@geggleto We are determining if the response should have an empty body based on the status, and if so, we remove the appropriate headers. I'm fine having the duplicate isEmptyResponse() method in Slim\App.php to avoid conflicts with alternative PSR 7 implementations.

akrabat commented 8 years ago

Fixed in #1630.