ENGCOM-2240: Remove PDF files after generation #16401

Stanislav Idolov · web-flow · commit 190d0ae64ebc · 2018-07-27T16:31:03.000+03:00
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Creditmemo/AbstractCreditmemo/Pdfcreditmemos.php b/app/code/Magento/Sales/Controller/Adminhtml/Creditmemo/AbstractCreditmemo/Pdfcreditmemos.php
@@ -74,9 +74,12 @@ public function __construct(
      */
     public function massAction(AbstractCollection $collection)
     {
+        $pdf = $this->pdfCreditmemo->getPdf($collection);
+        $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
         return $this->fileFactory->create(
             sprintf('creditmemo%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),
-            $this->pdfCreditmemo->getPdf($collection)->render(),
+            $fileContent,
             DirectoryList::VAR_DIR,
             'application/pdf'
         );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Creditmemo/AbstractCreditmemo/PrintAction.php b/app/code/Magento/Sales/Controller/Adminhtml/Creditmemo/AbstractCreditmemo/PrintAction.php
@@ -53,6 +53,7 @@ public function __construct(
 
     /**
      * @return ResponseInterface|\Magento\Backend\Model\View\Result\Forward
+     * @throws \Exception
      */
     public function execute()
     {
@@ -69,9 +70,11 @@ public function execute()
                 $date = $this->_objectManager->get(
                     \Magento\Framework\Stdlib\DateTime\DateTime::class
                 )->date('Y-m-d_H-i-s');
+                $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
                 return $this->_fileFactory->create(
                     \creditmemo::class . $date . '.pdf',
-                    $pdf->render(),
+                    $fileContent,
                     DirectoryList::VAR_DIR,
                     'application/pdf'
                 );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Invoice/AbstractInvoice/Pdfinvoices.php b/app/code/Magento/Sales/Controller/Adminhtml/Invoice/AbstractInvoice/Pdfinvoices.php
@@ -75,9 +75,12 @@ public function __construct(
      */
     public function massAction(AbstractCollection $collection)
     {
+        $pdf = $this->pdfInvoice->getPdf($collection);
+        $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
         return $this->fileFactory->create(
             sprintf('invoice%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),
-            $this->pdfInvoice->getPdf($collection)->render(),
+            $fileContent,
             DirectoryList::VAR_DIR,
             'application/pdf'
         );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Invoice/AbstractInvoice/PrintAction.php b/app/code/Magento/Sales/Controller/Adminhtml/Invoice/AbstractInvoice/PrintAction.php
@@ -45,6 +45,7 @@ public function __construct(
 
     /**
      * @return ResponseInterface|void
+     * @throws \Exception
      */
     public function execute()
     {
@@ -58,9 +59,11 @@ public function execute()
                 $date = $this->_objectManager->get(
                     \Magento\Framework\Stdlib\DateTime\DateTime::class
                 )->date('Y-m-d_H-i-s');
+                $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
                 return $this->_fileFactory->create(
                     'invoice' . $date . '.pdf',
-                    $pdf->render(),
+                    $fileContent,
                     DirectoryList::VAR_DIR,
                     'application/pdf'
                 );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfcreditmemos.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfcreditmemos.php
@@ -76,6 +76,7 @@ public function __construct(
      *
      * @param AbstractCollection $collection
      * @return ResponseInterface|ResultInterface
+     * @throws \Exception
      */
     protected function massAction(AbstractCollection $collection)
     {
@@ -85,9 +86,12 @@ protected function massAction(AbstractCollection $collection)
             $this->messageManager->addError(__('There are no printable documents related to selected orders.'));
             return $this->resultRedirectFactory->create()->setPath($this->getComponentRefererUrl());
         }
+        $pdf = $this->pdfCreditmemo->getPdf($creditmemoCollection->getItems());
+        $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
         return $this->fileFactory->create(
             sprintf('creditmemo%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),
-            $this->pdfCreditmemo->getPdf($creditmemoCollection->getItems())->render(),
+            $fileContent,
             DirectoryList::VAR_DIR,
             'application/pdf'
         );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfdocs.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfdocs.php
@@ -113,6 +113,7 @@ public function __construct(
      * @return ResponseInterface|\Magento\Backend\Model\View\Result\Redirect
      *
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     * @throws \Exception
      */
     protected function massAction(AbstractCollection $collection)
     {
@@ -142,10 +143,11 @@ protected function massAction(AbstractCollection $collection)
         foreach ($documents as $document) {
             $pdf->pages = array_merge($pdf->pages, $document->pages);
         }
+        $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
 
         return $this->fileFactory->create(
             sprintf('docs%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),
-            $pdf->render(),
+            $fileContent,
             DirectoryList::VAR_DIR,
             'application/pdf'
         );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfinvoices.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfinvoices.php
@@ -75,6 +75,7 @@ public function __construct(
      *
      * @param AbstractCollection $collection
      * @return ResponseInterface|ResultInterface
+     * @throws \Exception
      */
     protected function massAction(AbstractCollection $collection)
     {
@@ -83,9 +84,12 @@ protected function massAction(AbstractCollection $collection)
             $this->messageManager->addError(__('There are no printable documents related to selected orders.'));
             return $this->resultRedirectFactory->create()->setPath($this->getComponentRefererUrl());
         }
+        $pdf = $this->pdfInvoice->getPdf($invoicesCollection->getItems());
+        $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
         return $this->fileFactory->create(
             sprintf('invoice%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),
-            $this->pdfInvoice->getPdf($invoicesCollection->getItems())->render(),
+            $fileContent,
             DirectoryList::VAR_DIR,
             'application/pdf'
         );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfshipments.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Pdfshipments.php
@@ -77,6 +77,7 @@ public function __construct(
      *
      * @param AbstractCollection $collection
      * @return ResponseInterface|\Magento\Backend\Model\View\Result\Redirect
+     * @throws \Exception
      */
     protected function massAction(AbstractCollection $collection)
     {
@@ -87,9 +88,13 @@ protected function massAction(AbstractCollection $collection)
             $this->messageManager->addError(__('There are no printable documents related to selected orders.'));
             return $this->resultRedirectFactory->create()->setPath($this->getComponentRefererUrl());
         }
+
+        $pdf = $this->pdfShipment->getPdf($shipmentsCollection->getItems());
+        $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
         return $this->fileFactory->create(
             sprintf('packingslip%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),
-            $this->pdfShipment->getPdf($shipmentsCollection->getItems())->render(),
+            $fileContent,
             DirectoryList::VAR_DIR,
             'application/pdf'
         );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Shipment/AbstractShipment/Pdfshipments.php b/app/code/Magento/Sales/Controller/Adminhtml/Shipment/AbstractShipment/Pdfshipments.php
@@ -70,9 +70,12 @@ public function __construct(
      */
     public function massAction(AbstractCollection $collection)
     {
+        $pdf = $this->pdfShipment->getPdf($collection);
+        $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
         return $this->fileFactory->create(
             sprintf('packingslip%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),
-            $this->pdfShipment->getPdf($collection)->render(),
+            $fileContent,
             DirectoryList::VAR_DIR,
             'application/pdf'
         );
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Shipment/AbstractShipment/PrintAction.php b/app/code/Magento/Sales/Controller/Adminhtml/Shipment/AbstractShipment/PrintAction.php
@@ -48,6 +48,7 @@ public function __construct(
 
     /**
      * @return ResponseInterface|\Magento\Backend\Model\View\Result\Forward
+     * @throws \Exception
      */
     public function execute()
     {
@@ -63,9 +64,11 @@ public function execute()
                 $date = $this->_objectManager->get(
                     \Magento\Framework\Stdlib\DateTime\DateTime::class
                 )->date('Y-m-d_H-i-s');
+                $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];
+
                 return $this->_fileFactory->create(
                     'packingslip' . $date . '.pdf',
-                    $pdf->render(),
+                    $fileContent,
                     DirectoryList::VAR_DIR,
                     'application/pdf'
                 );
diff --git a/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Creditmemo/PrintActionTest.php b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Creditmemo/PrintActionTest.php
@@ -155,7 +155,8 @@ public function testExecute()
         $creditmemoId = 2;
         $date = '2015-01-19_13-03-45';
         $fileName = 'creditmemo2015-01-19_13-03-45.pdf';
-        $fileContents = 'pdf0123456789';
+        $pdfContent = 'pdf0123456789';
+        $fileData = ['type' => 'string', 'value' => $pdfContent, 'rm' => true];
         $this->prepareTestExecute($creditmemoId);
 
         $this->objectManagerMock->expects($this->any())
@@ -184,12 +185,12 @@ public function testExecute()
             ->willReturn($date);
         $this->pdfMock->expects($this->once())
             ->method('render')
-            ->willReturn($fileContents);
+            ->willReturn($pdfContent);
         $this->fileFactoryMock->expects($this->once())
             ->method('create')
             ->with(
                 $fileName,
-                $fileContents,
+                $fileData,
                 \Magento\Framework\App\Filesystem\DirectoryList::VAR_DIR,
                 'application/pdf'
             )
diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/Filesystem/CreatePdfFileTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/Filesystem/CreatePdfFileTest.php
@@ -0,0 +1,53 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ *
+ */
+declare(strict_types=1);
+
+namespace Magento\Framework\App\Filesystem;
+
+use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\App\Response\Http\FileFactory;
+use Magento\Framework\Filesystem;
+use Magento\TestFramework\Helper\Bootstrap;
+use Zend\Http\Header\ContentType;
+
+/**
+ * Class CreatePdfFileTest
+ *
+ * Integration test for testing a file creation from string
+ */
+class CreatePdfFileTest extends \PHPUnit\Framework\TestCase
+{
+    public function testGenerateFileFromString()
+    {
+        $objectManager = Bootstrap::getObjectManager();
+        /** @var FileFactory $fileFactory */
+        $fileFactory =  $objectManager->get(FileFactory::class);
+        /** @var Filesystem $filesystem */
+        $filesystem = $objectManager->get(Filesystem::class);
+        $filename = 'test.pdf';
+        $contentType = 'application/pdf';
+        $fileContent = ['type' => 'string', 'value' => ''];
+        $response = $fileFactory->create($filename, $fileContent, DirectoryList::VAR_DIR, $contentType);
+        /** @var ContentType $contentTypeHeader */
+        $contentTypeHeader = $response->getHeader('Content-type');
+
+        /* Check the system returns the correct type */
+        self::assertEquals("Content-Type: $contentType", $contentTypeHeader->toString());
+
+        $varDirectory = $filesystem->getDirectoryRead(DirectoryList::VAR_DIR);
+        $varDirectory->isFile($filename);
+
+        /* Check the file is generated */
+        self::assertTrue($varDirectory->isFile($filename));
+
+        /* Check the file is removed after generation if the corresponding option is set */
+        $fileContent = ['type' => 'string', 'value' => '', 'rm' => true];
+        $fileFactory->create($filename, $fileContent, DirectoryList::VAR_DIR, $contentType);
+
+        self::assertFalse($varDirectory->isFile($filename));
+    }
+}
diff --git a/lib/internal/Magento/Framework/App/Response/Http/FileFactory.php b/lib/internal/Magento/Framework/App/Response/Http/FileFactory.php
@@ -59,6 +59,7 @@ public function create(
         $dir = $this->_filesystem->getDirectoryWrite($baseDir);
         $isFile = false;
         $file = null;
+        $fileContent = $this->getFileContent($content);
         if (is_array($content)) {
             if (!isset($content['type']) || !isset($content['value'])) {
                 throw new \InvalidArgumentException("Invalid arguments. Keys 'type' and 'value' are required.");
@@ -76,7 +77,7 @@ public function create(
             ->setHeader('Pragma', 'public', true)
             ->setHeader('Cache-Control', 'must-revalidate, post-check=0, pre-check=0', true)
             ->setHeader('Content-type', $contentType, true)
-            ->setHeader('Content-Length', $contentLength === null ? strlen($content) : $contentLength, true)
+            ->setHeader('Content-Length', $contentLength === null ? strlen($fileContent) : $contentLength, true)
             ->setHeader('Content-Disposition', 'attachment; filename="' . $fileName . '"', true)
             ->setHeader('Last-Modified', date('r'), true);
 
@@ -88,7 +89,8 @@ public function create(
                     echo $stream->read(1024);
                 }
             } else {
-                $dir->writeFile($fileName, $content);
+                $dir->writeFile($fileName, $fileContent);
+                $file = $fileName;
                 $stream = $dir->openFile($fileName, 'r');
                 while (!$stream->eof()) {
                     echo $stream->read(1024);
@@ -102,4 +104,19 @@ public function create(
         }
         return $this->_response;
     }
+
+    /**
+     * Returns file content for writing.
+     *
+     * @param string|array $content
+     * @return string|array
+     */
+    private function getFileContent($content)
+    {
+        if (isset($content['type']) && $content['type'] === 'string') {
+            return $content['value'];
+        }
+
+        return $content;
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -113,6 +113,7 @@ public function __construct(`
`113`	`113`	`* @return ResponseInterface\|\Magento\Backend\Model\View\Result\Redirect`
`114`	`114`	`*`
`115`	`115`	`* @SuppressWarnings(PHPMD.CyclomaticComplexity)`
	`116`	`+ * @throws \Exception`
`116`	`117`	`*/`
`117`	`118`	`protected function massAction(AbstractCollection $collection)`
`118`	`119`	`{`
`@@ -142,10 +143,11 @@ protected function massAction(AbstractCollection $collection)`
`142`	`143`	`foreach ($documents as $document) {`
`143`	`144`	`$pdf->pages = array_merge($pdf->pages, $document->pages);`
`144`	`145`	`}`
	`146`	`+ $fileContent = ['type' => 'string', 'value' => $pdf->render(), 'rm' => true];`
`145`	`147`
`146`	`148`	`return $this->fileFactory->create(`
`147`	`149`	`sprintf('docs%s.pdf', $this->dateTime->date('Y-m-d_H-i-s')),`
`148`		`- $pdf->render(),`
	`150`	`+ $fileContent,`
`149`	`151`	`DirectoryList::VAR_DIR,`
`150`	`152`	`'application/pdf'`
`151`	`153`	`);`