From 52f221ec2a416716af2d26ed9b31a596e11cf9ac Mon Sep 17 00:00:00 2001 From: Volha Pivavarchyk Date: Wed, 3 Aug 2022 10:56:48 +0300 Subject: [PATCH] Assets are streamed by default (#11043) * Assets are streamed by default * Add a functional test * Add functional tests * Modify tests * Fix tests * Fix tests Co-authored-by: Ruth Cheesley --- .../Controller/AssetController.php | 4 +- .../Controller/PublicController.php | 3 +- .../Tests/Asset/AbstractAssetTest.php | 107 ++++++++++++++++++ .../AssetControllerFunctionalTest.php | 84 +++++++++++--- .../PublicControllerFunctionalTest.php | 48 ++++++++ 5 files changed, 231 insertions(+), 15 deletions(-) create mode 100644 app/bundles/AssetBundle/Tests/Asset/AbstractAssetTest.php create mode 100644 app/bundles/AssetBundle/Tests/Controller/PublicControllerFunctionalTest.php diff --git a/app/bundles/AssetBundle/Controller/AssetController.php b/app/bundles/AssetBundle/Controller/AssetController.php index d95b4b9e1a5..8621c5650b9 100644 --- a/app/bundles/AssetBundle/Controller/AssetController.php +++ b/app/bundles/AssetBundle/Controller/AssetController.php @@ -229,7 +229,9 @@ public function previewAction($objectId) } $download = $this->request->query->get('download', 0); - $stream = $this->request->query->get('stream', 0); + + // Display the file directly in the browser by default + $stream = $this->request->query->get('stream', '1'); if ('1' === $download || '1' === $stream) { try { diff --git a/app/bundles/AssetBundle/Controller/PublicController.php b/app/bundles/AssetBundle/Controller/PublicController.php index 35d62eb5b2a..31acce3640d 100644 --- a/app/bundles/AssetBundle/Controller/PublicController.php +++ b/app/bundles/AssetBundle/Controller/PublicController.php @@ -76,7 +76,8 @@ public function downloadAction($slug) $response->headers->set('Content-Type', $entity->getFileMimeType()); - $stream = $this->request->get('stream', 0); + // Display the file directly in the browser by default + $stream = $this->request->get('stream', '1'); if (!$stream) { $response->headers->set('Content-Disposition', 'attachment;filename="'.$entity->getOriginalFileName()); } diff --git a/app/bundles/AssetBundle/Tests/Asset/AbstractAssetTest.php b/app/bundles/AssetBundle/Tests/Asset/AbstractAssetTest.php new file mode 100644 index 00000000000..24d1697793b --- /dev/null +++ b/app/bundles/AssetBundle/Tests/Asset/AbstractAssetTest.php @@ -0,0 +1,107 @@ +generateCsv(); + + $assetData = [ + 'title' => 'Asset controller test. Preview action', + 'alias' => 'Test', + 'createdAt' => new \DateTime('2021-05-05 22:30:00'), + 'updatedAt' => new \DateTime('2022-05-05 22:30:00'), + 'createdBy' => 'User', + 'storage' => 'local', + 'path' => basename($this->csvPath), + 'extension' => 'png', + ]; + $this->asset = $this->createAsset($assetData); + + $this->expectedMimeType = 'text/plain; charset=UTF-8'; + $this->expectedContentDisposition = 'attachment;filename="'; + $this->expectedPngContent = file_get_contents($this->csvPath); + } + + protected function tearDown(): void + { + parent::tearDown(); + + if (file_exists($this->csvPath)) { + unlink($this->csvPath); + } + } + + /** + * Create an asset entity in the DB. + * + * @param array $assetData + * + * @throws ORMException + * @throws MappingException + */ + protected function createAsset(array $assetData): Asset + { + $asset = new Asset(); + $asset->setTitle($assetData['title']); + $asset->setAlias($assetData['alias']); + $asset->setDateAdded($assetData['createdAt'] ?? new \DateTime()); + $asset->setDateModified($assetData['updatedAt'] ?? new \DateTime()); + $asset->setCreatedByUser($assetData['createdBy'] ?? 'User'); + $asset->setStorageLocation($assetData['storage'] ?? 'local'); + $asset->setPath($assetData['path'] ?? ''); + $asset->setExtension($assetData['extension'] ?? ''); + + $this->em->persist($asset); + $this->em->flush(); + $this->em->clear(); + + return $asset; + } + + /** + * Generate the csv asset and return the path of the asset. + */ + protected function generateCsv(): void + { + $uploadDir = self::$container->get('mautic.helper.core_parameters')->get('upload_dir') ?? sys_get_temp_dir(); + $tmpFile = tempnam($uploadDir, 'mautic_asset_test_'); + $file = fopen($tmpFile, 'w'); + + $initialList = [ + ['email', 'firstname', 'lastname'], + ['john.doe@his-site.com.email', 'John', 'Doe'], + ['john.smith@his-site.com.email', 'John', 'Smith'], + ['jim.doe@his-site.com.email', 'Jim', 'Doe'], + [''], + ['jim.smith@his-site.com.email', 'Jim', 'Smith'], + ]; + + foreach ($initialList as $line) { + fputcsv($file, $line); + } + + fclose($file); + + $this->csvPath = $tmpFile; + } +} diff --git a/app/bundles/AssetBundle/Tests/Controller/AssetControllerFunctionalTest.php b/app/bundles/AssetBundle/Tests/Controller/AssetControllerFunctionalTest.php index 93e7e6281ce..eab4dc1a98d 100644 --- a/app/bundles/AssetBundle/Tests/Controller/AssetControllerFunctionalTest.php +++ b/app/bundles/AssetBundle/Tests/Controller/AssetControllerFunctionalTest.php @@ -2,11 +2,12 @@ namespace Mautic\AssetBundle\Tests\Controller; -use Mautic\AssetBundle\Entity\Asset; -use Mautic\CoreBundle\Test\MauticMysqlTestCase; +use Mautic\AssetBundle\Tests\Asset\AbstractAssetTest; use Mautic\CoreBundle\Tests\Traits\ControllerTrait; +use Mautic\PageBundle\Tests\Controller\PageControllerTest; +use Symfony\Component\HttpFoundation\Response; -class AssetControllerFunctionalTest extends MauticMysqlTestCase +class AssetControllerFunctionalTest extends AbstractAssetTest { use ControllerTrait; @@ -15,16 +16,14 @@ class AssetControllerFunctionalTest extends MauticMysqlTestCase */ public function testIndexAction(): void { - $asset = new Asset(); - $asset->setTitle('test'); - $asset->setAlias('test'); - $asset->setDateAdded(new \DateTime('2020-02-07 20:29:02')); - $asset->setDateModified(new \DateTime('2020-03-21 20:29:02')); - $asset->setCreatedByUser('Test User'); - - $this->em->persist($asset); - $this->em->flush(); - $this->em->clear(); + $assetData = [ + 'title' => 'Asset controller test. Index action', + 'alias' => 'Test', + 'createdAt' => new \DateTime('2020-02-07 20:29:02'), + 'updatedAt' => new \DateTime('2020-03-21 20:29:02'), + 'createdBy' => 'Test User', + ]; + $this->createAsset($assetData); $urlAlias = 'assets'; $routeAlias = 'asset'; @@ -34,4 +33,63 @@ public function testIndexAction(): void $this->getControllerColumnTests($urlAlias, $routeAlias, $column, $tableAlias, $column2); } + + /** + * Preview action should return the file content. + */ + public function testPreviewActionStreamByDefault(): void + { + $this->client->request('GET', '/s/assets/preview/'.$this->asset->getId()); + ob_start(); + $response = $this->client->getResponse(); + $response->sendContent(); + $content = ob_get_contents(); + ob_end_clean(); + + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + $this->assertSame($this->expectedMimeType, $response->headers->get('Content-Type')); + $this->assertNotSame($this->expectedContentDisposition.$this->asset->getOriginalFileName(), $response->headers->get('Content-Disposition')); + $this->assertEquals($this->expectedPngContent, $content); + } + + /** + * Preview action should return the file content. + */ + public function testPreviewActionStreamIsZero(): void + { + $this->client->request('GET', '/s/assets/preview/'.$this->asset->getId().'?stream=0&download=1'); + ob_start(); + $response = $this->client->getResponse(); + $response->sendContent(); + $content = ob_get_contents(); + ob_end_clean(); + + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + $this->assertSame($this->expectedContentDisposition.$this->asset->getOriginalFileName(), $response->headers->get('Content-Disposition')); + $this->assertEquals($this->expectedPngContent, $content); + } + + /** + * Preview action should return the html code. + */ + public function testPreviewActionStreamDownloadAreZero(): void + { + $this->client->request('GET', '/s/assets/preview/'.$this->asset->getId().'?stream=0&download=0'); + ob_start(); + $response = $this->client->getResponse(); + $response->sendContent(); + $content = ob_get_contents(); + ob_end_clean(); + + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + $this->assertNotEquals($this->expectedPngContent, $content); + PageControllerTest::assertTrue($response->isOk()); + + $assetSlug = $this->asset->getId().':'.$this->asset->getAlias(); + PageControllerTest::assertStringContainsString( + '/asset/'.$assetSlug, + $content, + 'The return must contain the assert slug' + ); + } } diff --git a/app/bundles/AssetBundle/Tests/Controller/PublicControllerFunctionalTest.php b/app/bundles/AssetBundle/Tests/Controller/PublicControllerFunctionalTest.php new file mode 100644 index 00000000000..29adb23f780 --- /dev/null +++ b/app/bundles/AssetBundle/Tests/Controller/PublicControllerFunctionalTest.php @@ -0,0 +1,48 @@ +asset->getId().':'.$this->asset->getAlias(); + + $this->client->request('GET', '/asset/'.$assetSlug); + ob_start(); + $response = $this->client->getResponse(); + $response->sendContent(); + $content = ob_get_contents(); + ob_end_clean(); + + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + $this->assertSame($this->expectedMimeType, $response->headers->get('Content-Type')); + $this->assertNotSame($this->expectedContentDisposition.$this->asset->getOriginalFileName(), $response->headers->get('Content-Disposition')); + $this->assertEquals($this->expectedPngContent, $content); + } + + /** + * Download action should return the file content. + */ + public function testDownloadActionStreamIsZero(): void + { + $assetSlug = $this->asset->getId().':'.$this->asset->getAlias(); + + $this->client->request('GET', '/asset/'.$assetSlug.'?stream=0'); + ob_start(); + $response = $this->client->getResponse(); + $response->sendContent(); + $content = ob_get_contents(); + ob_end_clean(); + + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + $this->assertSame($this->expectedContentDisposition.$this->asset->getOriginalFileName(), $response->headers->get('Content-Disposition')); + $this->assertEquals($this->expectedPngContent, $content); + } +}