feature #39128 [HttpFoundation] Deprecate BinaryFileResponse::create() (derrabus)

This PR was merged into the 5.2 branch.

Discussion
----------

[HttpFoundation] Deprecate BinaryFileResponse::create()

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | N/A
| License       | MIT
| Doc PR        | N/A

Follows #34771. We've deprecated the static `::create()` methods on all response classes but `BinaryFileResponse`. This PR proposes to fix this inconsistency.

Commits
-------

9ce2e86207 [HttpFoundation] Deprecate BinaryFileResponse::create().

Author: fabpot

BinaryFileResponse.php

@@ -65,9 +65,13 @@ public function __construct($file, int $status = 200, array $headers = [], bool
      * @param bool                $autoLastModified   Whether the Last-Modified header should be automatically set
      *
      * @return static
+     *
+     * @deprecated since Symfony 5.2, use __construct() instead.
      */
     public static function create($file = null, int $status = 200, array $headers = [], bool $public = true, string $contentDisposition = null, bool $autoEtag = false, bool $autoLastModified = true)
     {
+        trigger_deprecation('symfony/http-foundation', '5.2', 'The "%s()" method is deprecated, use "new %s()" instead.', __METHOD__, static::class);
+
         return new static($file, $status, $headers, $public, $contentDisposition, $autoEtag, $autoLastModified);
     }
 

CHANGELOG.md

@@ -12,6 +12,7 @@ CHANGELOG
  * added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter`
  * deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead.
  * Deprecated the `Request::HEADER_X_FORWARDED_ALL` constant, use either `HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO` or `HEADER_X_FORWARDED_AWS_ELB` or `HEADER_X_FORWARDED_TRAEFIK` constants instead.
+ * Deprecated `BinaryFileResponse::create()`, use `__construct()` instead
 
 
 5.1.0

Tests/BinaryFileResponseTest.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\HttpFoundation\Tests;
 
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\File\Stream;
 use Symfony\Component\HttpFoundation\Request;
@@ -19,6 +20,8 @@
 
 class BinaryFileResponseTest extends ResponseTestCase
 {
+    use ExpectDeprecationTrait;
+
     public function testConstruction()
     {
         $file = __DIR__.'/../README.md';
@@ -29,6 +32,26 @@ public function testConstruction()
         $this->assertTrue($response->headers->has('Last-Modified'));
         $this->assertFalse($response->headers->has('Content-Disposition'));
 
+        $response = new BinaryFileResponse($file, 404, [], true, ResponseHeaderBag::DISPOSITION_INLINE);
+        $this->assertEquals(404, $response->getStatusCode());
+        $this->assertFalse($response->headers->has('ETag'));
+        $this->assertEquals('inline; filename=README.md', $response->headers->get('Content-Disposition'));
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testConstructionLegacy()
+    {
+        $file = __DIR__.'/../README.md';
+        $this->expectDeprecation('Since symfony/http-foundation 5.2: The "Symfony\Component\HttpFoundation\BinaryFileResponse::create()" method is deprecated, use "new Symfony\Component\HttpFoundation\BinaryFileResponse()" instead.');
+        $response = BinaryFileResponse::create($file, 404, ['X-Header' => 'Foo'], true, null, true, true);
+        $this->assertEquals(404, $response->getStatusCode());
+        $this->assertEquals('Foo', $response->headers->get('X-Header'));
+        $this->assertTrue($response->headers->has('ETag'));
+        $this->assertTrue($response->headers->has('Last-Modified'));
+        $this->assertFalse($response->headers->has('Content-Disposition'));
+
         $response = BinaryFileResponse::create($file, 404, [], true, ResponseHeaderBag::DISPOSITION_INLINE);
         $this->assertEquals(404, $response->getStatusCode());
         $this->assertFalse($response->headers->has('ETag'));
@@ -83,7 +106,7 @@ public function testSetContentDispositionGeneratesSafeFallbackFilenameForWrongly
      */
     public function testRequests($requestRange, $offset, $length, $responseRange)
     {
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'])->setAutoEtag();
+        $response = (new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']))->setAutoEtag();
 
         // do a request to get the ETag
         $request = Request::create('/');
@@ -115,7 +138,7 @@ public function testRequests($requestRange, $offset, $length, $responseRange)
      */
     public function testRequestsWithoutEtag($requestRange, $offset, $length, $responseRange)
     {
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
+        $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
 
         // do a request to get the LastModified
         $request = Request::create('/');
@@ -156,7 +179,7 @@ public function provideRanges()
     public function testRangeRequestsWithoutLastModifiedDate()
     {
         // prevent auto last modified
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'], true, null, false, false);
+        $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'], true, null, false, false);
 
         // prepare a request for a range of the testing file
         $request = Request::create('/');
@@ -177,7 +200,7 @@ public function testRangeRequestsWithoutLastModifiedDate()
      */
     public function testFullFileRequests($requestRange)
     {
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'])->setAutoEtag();
+        $response = (new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']))->setAutoEtag();
 
         // prepare a request for a range of the testing file
         $request = Request::create('/');
@@ -213,7 +236,7 @@ public function testRangeOnPostMethod()
     {
         $request = Request::create('/', 'POST');
         $request->headers->set('Range', 'bytes=10-20');
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
+        $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
 
         $file = fopen(__DIR__.'/File/Fixtures/test.gif', 'r');
         $data = fread($file, 35);
@@ -231,7 +254,7 @@ public function testRangeOnPostMethod()
 
     public function testUnpreparedResponseSendsFullFile()
     {
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200);
+        $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200);
 
         $data = file_get_contents(__DIR__.'/File/Fixtures/test.gif');
 
@@ -247,7 +270,7 @@ public function testUnpreparedResponseSendsFullFile()
      */
     public function testInvalidRequests($requestRange)
     {
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'])->setAutoEtag();
+        $response = (new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']))->setAutoEtag();
 
         // prepare a request for a range of the testing file
         $request = Request::create('/');
@@ -278,7 +301,7 @@ public function testXSendfile($file)
         $request->headers->set('X-Sendfile-Type', 'X-Sendfile');
 
         BinaryFileResponse::trustXSendfileTypeHeader();
-        $response = BinaryFileResponse::create($file, 200, ['Content-Type' => 'application/octet-stream']);
+        $response = new BinaryFileResponse($file, 200, ['Content-Type' => 'application/octet-stream']);
         $response->prepare($request);
 
         $this->expectOutputString('');
@@ -338,7 +361,7 @@ public function testDeleteFileAfterSend()
     public function testAcceptRangeOnUnsafeMethods()
     {
         $request = Request::create('/', 'POST');
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
+        $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
         $response->prepare($request);
 
         $this->assertEquals('none', $response->headers->get('Accept-Ranges'));
@@ -347,7 +370,7 @@ public function testAcceptRangeOnUnsafeMethods()
     public function testAcceptRangeNotOverriden()
     {
         $request = Request::create('/', 'POST');
-        $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
+        $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']);
         $response->headers->set('Accept-Ranges', 'foo');
         $response->prepare($request);