fix(api): handle errors better in WebDavClient.php

app/api/module/DocumentShare/src/Service/ClientFactory.php

@@ -101,6 +101,8 @@ public function __invoke(ContainerInterface $container, $requestedName, array $o
         $adapter = new WebDAVAdapter($sabreClient, $clientOptions['workspace']);
         $fileSystem = new Filesystem($adapter);
 
-        return new WebDavClient($fileSystem);
+        $logger = $container->get('Logger');
+
+        return new WebDavClient($fileSystem, $logger);
     }
 }

app/api/module/DocumentShare/src/Service/WebDavClient.php

@@ -3,6 +3,7 @@
 namespace Dvsa\Olcs\DocumentShare\Service;
 
 use Dvsa\Olcs\DocumentShare\Data\Object\File;
+use Laminas\Log\Logger;
 use League\Flysystem\FileExistsException;
 use League\Flysystem\FileNotFoundException;
 use League\Flysystem\FilesystemInterface;
@@ -11,11 +12,6 @@ class WebDavClient implements DocumentStoreInterface
 {
     public const DS_DOWNLOAD_FILE_PREFIX = 'ds_dwnld_';
 
-    /**
-     * @var FilesystemInterface
-     */
-    protected $filesystem;
-
     /**
      * @var string
      */
@@ -31,23 +27,36 @@ class WebDavClient implements DocumentStoreInterface
      *
      * @param FilesystemInterface $filesystem File System
      */
-    public function __construct(FilesystemInterface $filesystem)
-    {
-        $this->filesystem = $filesystem;
-    }
+    public function __construct(
+        protected FilesystemInterface $filesystem,
+        protected Logger $logger
+    ) {}
 
     /**
      * Read content from document store
      *
      * @param string $path Path
+     * @throws \Exception
      */
     public function read($path): File | false
     {
         $tmpFileName = tempnam(sys_get_temp_dir(), self::DS_DOWNLOAD_FILE_PREFIX);
 
+        if ($tmpFileName === false) {
+            $this->logger->err('Failed to create temp file', ['path' => $path, 'tmpDir' => sys_get_temp_dir()]);
+            return false;
+        }
+
+        $this->logger->debug('Temp file created', ['tmpFileName' => $tmpFileName, 'is_file' => is_file($tmpFileName), 'is_readable' => is_readable($tmpFileName), 'is_writable' => is_writable($tmpFileName)]);
+
         try {
             $readStream = $this->filesystem->readStream($path);
-            file_put_contents($tmpFileName, $readStream);
+            $fpc = file_put_contents($tmpFileName, $readStream);
+
+            if ($fpc === false) {
+                $this->logger->err('Failed to write file to temp location', ['path' => $path, 'tmpFileName' => $tmpFileName]);
+                return false;
+            }
 
             $file = new File();
             $file->setContentFromStream($tmpFileName);
@@ -97,8 +106,19 @@ public function write($path, File $file)
     {
         $response = new WebDavResponse();
         try {
+            $this->logger->debug('Opening file for reading', ['file' => $file->getResource(), 'path' => $path]);
+
+            $this->logger->debug('File contents', ['contents' => file_get_contents($file->getResource())]);
+
             $fh = fopen($file->getResource(), 'rb');
-            $response->setResponse($this->filesystem->writeStream($path, $fh));
+
+            if ($fh === false) {
+                $this->logger->err('Failed to open file for reading', ['file' => $file->getResource(), 'path' => $path]);
+
+                $response->setResponse(false);
+            } else {
+                $response->setResponse($this->filesystem->writeStream($path, $fh));
+            }
         } catch (FileExistsException) {
             $response->setResponse(false);
         } finally {

app/api/test/module/DocumentShare/src/Service/ClientFactoryTest.php

@@ -6,6 +6,7 @@
 
 use Dvsa\Olcs\DocumentShare\Service\ClientFactory;
 use Dvsa\Olcs\DocumentShare\Service\WebDavClient;
+use Laminas\Log\Logger;
 use Psr\Container\ContainerInterface;
 use Mockery as m;
 use Mockery\Adapter\Phpunit\MockeryTestCase;
@@ -30,6 +31,7 @@ public function testInvoke(): void
 
         $mockContainer = m::mock(ContainerInterface::class);
         $mockContainer->expects('get')->with('Configuration')->andReturn($config);
+        $mockContainer->expects('get')->with('Logger')->andReturn($this->createMock(Logger::class));
 
         $service = $sut->__invoke($mockContainer, WebDavClient::class);
         $this->assertInstanceOf(WebDavClient::class, $service);

app/api/test/module/DocumentShare/src/Service/WebDavClientTest.php

@@ -37,7 +37,7 @@ public function setUp(): void
     {
         $this->mockFileSystem = m::mock(FilesystemInterface::class);
 
-        $this->sut = new Client($this->mockFileSystem);
+        $this->sut = new Client($this->mockFileSystem, $this->createMock(\Laminas\Log\Logger::class));
 
         $this->mockFile = m::mock(DsFile::class);
 
@@ -97,7 +97,7 @@ public function testWriteSuccess()
 
         /** @var DsFile $mockFile */
         $mockFile = m::mock(DsFile::class)
-            ->shouldReceive('getResource')->once()->andReturn($res)
+            ->shouldReceive('getResource')->andReturn($res)
             ->getMock();
 
         $this->mockFileSystem->expects('writeStream')->with($expectPath, new IsTypeOf('resource'))->andReturn(true);
@@ -119,7 +119,7 @@ public function testWriteFail()
 
         /** @var DsFile $mockFile */
         $mockFile = m::mock(DsFile::class)
-            ->shouldReceive('getResource')->once()->andReturn($res)
+            ->shouldReceive('getResource')->andReturn($res)
             ->getMock();
 
         $this->mockFileSystem->expects('writeStream')->with($expectPath, new IsTypeOf('resource'))->andReturn(false);
@@ -141,7 +141,7 @@ public function testWriteFileAlreadyExists()
 
         /** @var DsFile $mockFile */
         $mockFile = m::mock(DsFile::class)
-            ->shouldReceive('getResource')->once()->andReturn($res)
+            ->shouldReceive('getResource')->andReturn($res)
             ->getMock();
 
         $this->mockFileSystem->expects('writeStream')->with($expectPath, new IsTypeOf('resource'))->andThrow(