Set the content-type and -disposition headers from constructor arguments

After thinking about this at some length, it makes sense to me (at least
at this stage) to set the content-type and content-disposition headers
from values supplied ($contentType and $filename values respectively) to
the constructor. This, I think, will be cleaner and more maintainable.

Author: settermjd

src/Response/DownloadResponse.php

@@ -26,6 +26,9 @@
  */
 class DownloadResponse extends Response
 {
+    const DEFAULT_CONTENT_TYPE = 'application/octet-stream';
+    const DEFAULT_DOWNLOAD_FILENAME = 'download';
+
     /**
      * A list of header keys required to be sent with a download response
      *
@@ -40,36 +43,52 @@ class DownloadResponse extends Response
         'pragma'
     ];
 
+    /**
+     * @var string The filename to be sent with the response
+     */
+    private $filename;
+
+    /**
+     * @var string The content type to be sent with the response
+     */
+    private $contentType;
+
     /**
      * DownloadResponse constructor.
+     *
      * @param string|StreamInterface $body String or stream for the message body.
      * @param int $status Integer status code for the response; 200 by default.
-     * @param string $filename The name of the file to be downloaded
-     * @param array $headers Array of headers to use at initialization.
-     * @throws InvalidArgumentException if $text is neither a string or stream.
-     * @param array $headers
-     */
-    public function __construct($body, int $status = 200, string $filename = 'download', array $headers = [])
-    {
+     * @param string $filename The file name to be sent with the response
+     * @param string $contentType The content type to be sent with the response
+     * @param array $headers An array of optional headers. These cannot override those set in getDownloadHeaders       */
+    public function __construct(
+        $body,
+        int $status = 200,
+        string $filename = self::DEFAULT_DOWNLOAD_FILENAME,
+        string $contentType = self::DEFAULT_CONTENT_TYPE,
+        array $headers = []
+    ) {
+        $this->filename = $filename;
+        $this->contentType = $contentType;
+
         parent::__construct(
             $this->createBody($body),
             $status,
-            $this->prepareDownloadHeaders($filename, $headers)
+            $this->prepareDownloadHeaders($headers)
         );
     }
 
     /**
      * Get download headers
      *
-     * @param string $filename
      * @return array
      */
-    private function getDownloadHeaders(string $filename): array
+    private function getDownloadHeaders(): array
     {
         $headers = [];
         $headers['cache-control'] = 'must-revalidate';
         $headers['content-description'] = 'File Transfer';
-        $headers['content-disposition'] = sprintf('attachment; filename=%s', $filename);
+        $headers['content-disposition'] = sprintf('attachment; filename=%s', self::DEFAULT_DOWNLOAD_FILENAME);
         $headers['content-transfer-encoding'] = 'Binary';
         $headers['content-type'] = 'application/octet-stream';
         $headers['expires'] = '0';
@@ -104,11 +123,16 @@ public function overridesDownloadHeaders(array $downloadHeaders, array $headers
     /**
      * Prepare download response headers
      *
-     * @param string $filename
+     * This function prepares the download response headers. It does so by:
+     * - Merging the optional with over the default ones (the default ones cannot be overridden)
+     * - Set the content-type and content-disposition headers from $filename and $contentType passed
+     *   to the constructor.
+     *
      * @param array $headers
      * @return array
+     * @throws InvalidArgumentException if an attempt is made to override a default header
      */
-    private function prepareDownloadHeaders(string $filename, array $headers = []) : array
+    private function prepareDownloadHeaders(array $headers = []) : array
     {
         if ($this->overridesDownloadHeaders($this->downloadResponseHeaders, $headers)) {
             throw new InvalidArgumentException(
@@ -119,7 +143,14 @@ private function prepareDownloadHeaders(string $filename, array $headers = []) :
             );
         }
 
-        return array_merge($headers, $this->getDownloadHeaders($filename));
+        return array_merge(
+            $headers,
+            $this->getDownloadHeaders(),
+            [
+                'content-disposition' => sprintf('attachment; filename=%s', $this->filename),
+                'content-type' => $this->contentType,
+            ]
+        );
     }
 
     /**

test/Response/DownloadResponseTest.php

@@ -83,18 +83,35 @@ public function testCanSendResponseWithCustomFilename()
         $this->assertSame('attachment; filename=valid.csv', $response->getHeaderLine('content-disposition'));
     }
 
+    public function testCanSendResponseWithCustomContentType()
+    {
+        $body = new Stream($this->root->url() . '/files/valid.csv');
+        $response = new DownloadResponse($body, 200, 'valid.csv', 'text/csv');
+        $this->assertInstanceOf(Response::class, $response);
+        $this->assertEquals(
+            file_get_contents($this->root->url() . '/files/valid.csv'),
+            (string) $response->getBody()
+        );
+        $this->assertHasValidResponseHeaders($response, 'valid.csv', 'text/csv');
+        $this->assertSame('attachment; filename=valid.csv', $response->getHeaderLine('content-disposition'));
+    }
+
     /**
      * @param DownloadResponse $response
      * @param string $filename
+     * @param string $contentType
      */
-    private function assertHasValidResponseHeaders(DownloadResponse $response, $filename = 'download'): void
-    {
+    private function assertHasValidResponseHeaders(
+        DownloadResponse $response,
+        $filename = 'download',
+        $contentType = 'application/octet-stream'
+    ) : void {
         $requiredHeaders = [
             'cache-control' => 'must-revalidate',
             'content-description' => 'File Transfer',
             'content-disposition' => sprintf('attachment; filename=%s', $filename),
             'content-transfer-encoding' => 'Binary',
-            'content-type' => 'application/octet-stream',
+            'content-type' => $contentType,
             'expires' => '0',
             'pragma' => 'Public'
         ];