feat: downloaded files now use the document description where availab…

…le (#177)
dvsa · Jun 3, 2024 · e336116 · e336116
1 parent 48067a3
commit e336116
Show file tree

Hide file tree

Showing 11 changed files with 168 additions and 49 deletions.
diff --git a/module/Api/src/Domain/QueryHandler/Document/AbstractDownload.php b/module/Api/src/Domain/QueryHandler/Document/AbstractDownload.php
@@ -8,6 +8,8 @@
 use Dvsa\Olcs\Api\Domain\UploaderAwareTrait;
 use Dvsa\Olcs\DocumentShare\Data\Object\File as ContentStoreFile;
 use Dvsa\Olcs\Utils\Helper\FileHelper;
+use Laminas\Http\Response\Stream;
+use Olcs\Logging\Log\Logger;
 use Psr\Container\ContainerInterface;
 use Laminas\Http\Response;
 use Psr\Container\ContainerExceptionInterface;
@@ -31,24 +33,28 @@ abstract class AbstractDownload extends AbstractQueryHandler implements Uploader
     /**
      * Process downloading file
      *
-     * @param string      $identifier File name
-     * @param string|null $path       Path to file
-     *
-     * @return Response\Stream
      * @throws NotFoundException
      */
-    protected function download($identifier, $path = null)
+    protected function download(string $identifier, ?string $path = null, ?string $chosenFileName = null): Stream
     {
         if ($path === null) {
             $path = $identifier;
         }
 
         $file = $this->getUploader()->download($path);
-        if ($file === null) {
+
+        if ($file === false) {
+            $logInfo = [
+                'identifier' => $identifier,
+                'path' => $path,
+                'filename' => $chosenFileName,
+            ];
+
+            Logger::info('File could not be downloaded', $logInfo);
             throw new NotFoundException();
         }
 
-        $response = new \Laminas\Http\Response\Stream();
+        $response = new Stream();
         $response->setStatusCode(Response::STATUS_CODE_200);
 
         $fileName = $file->getResource();
@@ -59,15 +65,26 @@ protected function download($identifier, $path = null)
         $response->setContentLength($fileSize);
         $response->setCleanup(true);
 
+        $extension = FileHelper::getExtension($identifier);
+
         $isInline = (
             $this->isInline === true
-            || 'html' === FileHelper::getExtension($identifier)
+            || 'html' === $extension
         );
 
-        // OLCS-14910 If file doesn't have an extension then add a '.txt' extension
         $downloadFileName = basename($identifier);
-        if (empty(FileHelper::getExtension($downloadFileName))) {
+
+        // OLCS-14910 If file doesn't have an extension then add a '.txt' extension
+        if (empty($extension)) {
+            //used in case of the original identifier being used
             $downloadFileName .= '.txt';
+
+            //used in the case of a user chosen filename
+            $extension = 'txt';
+        }
+
+        if ($chosenFileName !== null) {
+            $downloadFileName = $chosenFileName . '.' . $extension;
         }
 
         $headers = $response->getHeaders();

diff --git a/module/Api/src/Domain/QueryHandler/Document/Download.php b/module/Api/src/Domain/QueryHandler/Document/Download.php
@@ -30,8 +30,20 @@ public function handleQuery(QueryInterface $query)
         /* @var \Dvsa\Olcs\Api\Entity\Doc\Document $document */
         $document = $this->getRepo()->fetchById($query->getIdentifier());
 
+        $chosenFileName = null;
+
+        if ($this->isInternalUser()) {
+            $description = $document->getDescription();
+
+            if (!empty($description)) {
+                $chosenFileName = $description;
+            }
+        }
+
         return $this->download(
-            $document->getIdentifier()
+            $document->getIdentifier(),
+            null,
+            $chosenFileName
         );
     }
 }
diff --git a/module/Api/src/Domain/QueryHandler/Document/DownloadGuide.php b/module/Api/src/Domain/QueryHandler/Document/DownloadGuide.php
@@ -4,6 +4,7 @@
 
 use Dvsa\Olcs\Api\Domain\Exception\NotFoundException;
 use Dvsa\Olcs\Transfer\Query\QueryInterface;
+use Laminas\Http\Response\Stream;
 
 /**
  * Download a guide document, these are located in a "/guides/" directory in the content store
@@ -13,14 +14,11 @@ class DownloadGuide extends AbstractDownload
     protected $extraRepos = ['DocTemplate'];
 
     /**
-     * Handle query
-     *
      * @param \Dvsa\Olcs\Transfer\Query\Document\DownloadGuide $query DTO
      *
-     * @return array
      * @throws NotFoundException
      */
-    public function handleQuery(QueryInterface $query)
+    public function handleQuery(QueryInterface $query): Stream
     {
         $this->setIsInline($query->isInline());
 

diff --git a/module/DocumentShare/src/Service/DocManClient.php b/module/DocumentShare/src/Service/DocManClient.php
@@ -111,10 +111,8 @@ public function getWorkspace()
      * Read content from document store
      *
      * @param string $path Path
-     *
-     * @return File|null
      */
-    public function read($path): ?File
+    public function read($path): File | false
     {
         $tmpFileName = tempnam(sys_get_temp_dir(), self::DS_DOWNLOAD_FILE_PREFIX);
 
@@ -132,7 +130,7 @@ public function read($path): ?File
             if (!$response->isSuccess()) {
                 $message = json_encode(["error" => self::ERR_RESP_FAIL, "reason" => $response->getStatusCode(), "path" => $path]);
                 Logger::logResponse($response->getStatusCode(), $message);
-                return null;
+                return false;
             }
 
             $file = new File();
@@ -158,7 +156,7 @@ public function read($path): ?File
             Logger::logResponse(Response::STATUS_CODE_404, $errMssg);
         }
 
-        return null;
+        return false;
     }
 
     /**

diff --git a/module/DocumentShare/src/Service/WebDavClient.php b/module/DocumentShare/src/Service/WebDavClient.php
@@ -7,9 +7,6 @@
 use League\Flysystem\FileNotFoundException;
 use League\Flysystem\FilesystemInterface;
 
-/**
- * Class Client
- */
 class WebDavClient implements DocumentStoreInterface
 {
     public const DS_DOWNLOAD_FILE_PREFIX = 'ds_dwnld_';
@@ -43,10 +40,8 @@ public function __construct(FilesystemInterface $filesystem)
      * Read content from document store
      *
      * @param string $path Path
-     *
-     * @return File|bool
      */
-    public function read($path)
+    public function read($path): File | false
     {
         $tmpFileName = tempnam(sys_get_temp_dir(), self::DS_DOWNLOAD_FILE_PREFIX);
 

diff --git a/test/module/Api/src/Domain/QueryHandler/Document/AbstractDownloadStub.php b/test/module/Api/src/Domain/QueryHandler/Document/AbstractDownloadStub.php
@@ -4,15 +4,16 @@
 
 use Dvsa\Olcs\Api\Domain\QueryHandler\Document\AbstractDownload;
 use Dvsa\Olcs\Transfer\Query\QueryInterface;
+use Laminas\Http\Response\Stream;
 
 /**
  * Stub class of AbstractDownload handler for testing
  */
 class AbstractDownloadStub extends AbstractDownload
 {
-    public function download($identifier, $path = null)
+    public function download(string $identifier, ?string $path = null, ?string $fileName = null): Stream
     {
-        return parent::download($identifier, $path);
+        return parent::download($identifier, $path, $fileName);
     }
 
     public function setIsInline($inline)

diff --git a/test/module/Api/src/Domain/QueryHandler/Document/AbstractDownloadTest.php b/test/module/Api/src/Domain/QueryHandler/Document/AbstractDownloadTest.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Dvsa\OlcsTest\Api\Domain\QueryHandler\Document;
 
 use Dvsa\Olcs\Api\Domain\Exception\NotFoundException;
@@ -46,15 +48,15 @@ public function testDownloadFailExcNotFound()
             ->shouldReceive('download')
             ->once()
             ->with($path)
-            ->andReturn(null);
+            ->andReturnFalse();
 
         $this->sut->download($path);
     }
 
     /**
      * @dataProvider dpTestDownload
      */
-    public function testDownload($identifier, $path, $isInline, $expect)
+    public function testDownload($identifier, $path, $isInline, $chosenFileName, $expect)
     {
         $this->sut->setIsInline($isInline);
 
@@ -79,7 +81,7 @@ public function testDownload($identifier, $path, $isInline, $expect)
             ->andReturn($mockFile);
 
         //  call & check
-        $actual = $this->sut->download($identifier, $path);
+        $actual = $this->sut->download($identifier, $path, $chosenFileName);
 
         static::assertInstanceOf(\Laminas\Http\Response\Stream::class, $actual);
         static::assertEquals($tmpFilePath, $actual->getStreamName());
@@ -101,6 +103,7 @@ public function dpTestDownload()
                 'identifier' => 'unit_file.ext',
                 'path' => '/unit_dir/unit_file1.pdf',
                 'isInline' => false,
+                null,
                 'expect' => [
                     'mime' => self::MIME_TYPE,
                     'isDownload' => true,
@@ -112,6 +115,7 @@ public function dpTestDownload()
                 'identifier' => 'unit_file.html',
                 'path' => null,
                 'isInline' => false,
+                null,
                 'expect' => [
                     'mime' => self::MIME_TYPE,
                     'isDownload' => false,
@@ -123,6 +127,7 @@ public function dpTestDownload()
                 'identifier' => 'dir/dir/unit_file.unit_excl_ext',
                 'path' => null,
                 'isInline' => false,
+                null,
                 'expect' => [
                     'mime' => self::MIME_TYPE_EXCLUDE,
                     'isDownload' => true,
@@ -134,6 +139,7 @@ public function dpTestDownload()
                 'identifier' => 'unit_file.ext',
                 'path' => 'unti_path',
                 'isInline' => true,
+                null,
                 'expect' => [
                     'mime' => self::MIME_TYPE,
                     'isDownload' => false,
@@ -145,13 +151,74 @@ public function dpTestDownload()
                 'identifier' => '/foo/bar',
                 'path' => null,
                 'isInline' => false,
+                null,
                 'expect' => [
                     'mime' => self::MIME_TYPE,
                     'isDownload' => true,
                     'path' => '/foo/bar',
                     'filename' => 'bar.txt',
                 ],
             ],
+            [
+                'identifier' => 'unit_file.ext',
+                'path' => '/unit_dir/unit_file1.pdf',
+                'isInline' => false,
+                'chosen_filename',
+                'expect' => [
+                    'mime' => self::MIME_TYPE,
+                    'isDownload' => true,
+                    'path' => '/unit_dir/unit_file1.pdf',
+                    'filename' => 'chosen_filename.ext',
+                ],
+            ],
+            [
+                'identifier' => 'unit_file.html',
+                'path' => null,
+                'isInline' => false,
+                'chosen_filename',
+                'expect' => [
+                    'mime' => self::MIME_TYPE,
+                    'isDownload' => false,
+                    'path' => 'unit_file.html',
+                    'filename' => 'chosen_filename.html',
+                ],
+            ],
+            [
+                'identifier' => 'dir/dir/unit_file.unit_excl_ext',
+                'path' => null,
+                'isInline' => false,
+                'chosen_filename',
+                'expect' => [
+                    'mime' => self::MIME_TYPE_EXCLUDE,
+                    'isDownload' => true,
+                    'path' => 'dir/dir/unit_file.unit_excl_ext',
+                    'filename' => 'chosen_filename.unit_excl_ext',
+                ],
+            ],
+            [
+                'identifier' => 'unit_file.ext',
+                'path' => 'unti_path',
+                'isInline' => true,
+                'chosen_filename.txt',
+                'expect' => [
+                    'mime' => self::MIME_TYPE,
+                    'isDownload' => false,
+                    'path' => 'unti_path',
+                    'filename' => 'chosen_filename.txt.ext',
+                ],
+            ],
+            [
+                'identifier' => '/foo/bar',
+                'path' => null,
+                'isInline' => false,
+                'chosen_filename',
+                'expect' => [
+                    'mime' => self::MIME_TYPE,
+                    'isDownload' => true,
+                    'path' => '/foo/bar',
+                    'filename' => 'chosen_filename.txt',
+                ],
+            ],
         ];
     }
 }
diff --git a/test/module/Api/src/Domain/QueryHandler/Document/DownloadGuideTest.php b/test/module/Api/src/Domain/QueryHandler/Document/DownloadGuideTest.php
@@ -9,6 +9,7 @@
 use Dvsa\Olcs\Api\Service\File\ContentStoreFileUploader;
 use Dvsa\OlcsTest\Api\Domain\QueryHandler\QueryHandlerTestCase;
 use Dvsa\Olcs\Api\Domain\Repository\DocTemplate as DocTemplateRepo;
+use Laminas\Http\Response\Stream;
 use Mockery as m;
 
 /**
@@ -49,13 +50,14 @@ public function testHandleQueryTryingToGetIntoParent()
     public function testHandleQuery()
     {
         $fileName = 'unit_file1.pdf';
+        $file = m::mock(Stream::class);
 
         $this->sut
             ->shouldReceive('setIsInline')->once()->with(false)
             ->shouldReceive('download')
             ->once()
             ->with($fileName, '/guides/' . $fileName)
-            ->andReturn('EXPECTED');
+            ->andReturn($file);
 
         $query = TransferQry\Document\DownloadGuide::create(
             [
@@ -65,13 +67,14 @@ public function testHandleQuery()
         );
         $actual = $this->sut->handleQuery($query);
 
-        static::assertEquals('EXPECTED', $actual);
+        static::assertEquals($file, $actual);
     }
 
     public function testHandleQueryIsSlug()
     {
         $templateSlug = 'some-template-slug';
         $fileName = 'someFile.txt';
+        $file = m::mock(Stream::class);
 
         $docTemplate = m::mock(DocTemplate::class);
 
@@ -88,7 +91,7 @@ public function testHandleQueryIsSlug()
             ->shouldReceive('download')
             ->once()
             ->with($fileName, '/guides/' . $fileName)
-            ->andReturn('EXPECTED');
+            ->andReturn($file);
 
         $query = TransferQry\Document\DownloadGuide::create(
             [
@@ -99,6 +102,6 @@ public function testHandleQueryIsSlug()
         );
         $actual = $this->sut->handleQuery($query);
 
-        static::assertEquals('EXPECTED', $actual);
+        static::assertEquals($file, $actual);
     }
 }