refactor: update logic and add tests

src/App/Action/Backend/Debug/DebugDownloadLogsAction.php

@@ -6,44 +6,25 @@
 
 use MyParcelNL\Pdk\App\Action\Contract\ActionInterface;
 use MyParcelNL\Pdk\Base\Contract\ZipServiceInterface;
-use MyParcelNL\Pdk\Base\FileSystemInterface;
+use MyParcelNL\Pdk\Facade\Logger;
 use MyParcelNL\Pdk\Facade\Pdk;
-use MyParcelNL\Pdk\Logger\Contract\PdkLoggerInterface;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\HeaderUtils;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
-use ZipArchive;
 
 class DebugDownloadLogsAction implements ActionInterface
 {
-    /**
-     * @var \MyParcelNL\Pdk\Base\FileSystemInterface
-     */
-    protected $fileSystem;
-
-    /**
-     * @var \MyParcelNL\Pdk\Logger\Contract\PdkLoggerInterface
-     */
-    protected $logger;
-
     /**
      * @var \MyParcelNL\Pdk\Base\Contract\ZipServiceInterface
      */
     private $zipService;
 
     /**
-     * @param  \MyParcelNL\Pdk\Logger\Contract\PdkLoggerInterface $logger
-     * @param  \MyParcelNL\Pdk\Base\FileSystemInterface           $fileSystem
-     * @param  \MyParcelNL\Pdk\Base\Contract\ZipServiceInterface  $zipService
+     * @param  \MyParcelNL\Pdk\Base\Contract\ZipServiceInterface $zipService
      */
-    public function __construct(
-        PdkLoggerInterface  $logger,
-        FileSystemInterface $fileSystem,
-        ZipServiceInterface $zipService
-    ) {
-        $this->logger     = $logger;
-        $this->fileSystem = $fileSystem;
+    public function __construct(ZipServiceInterface $zipService)
+    {
         $this->zipService = $zipService;
     }
 
@@ -61,6 +42,7 @@ public function handle(Request $request): Response
         $response = new BinaryFileResponse($path);
 
         $disposition = HeaderUtils::makeDisposition(HeaderUtils::DISPOSITION_ATTACHMENT, basename($path));
+
         $response->headers->set('Content-Disposition', $disposition);
         $response->deleteFileAfterSend();
 
@@ -74,16 +56,12 @@ public function handle(Request $request): Response
      */
     protected function createLogsZip(string $path): void
     {
-        $logs = $this->logger->getLogs();
+        $logFiles = Logger::getLogFiles();
 
         $this->zipService->create($path);
 
-        foreach ($logs as $level => $log) {
-            if (empty($log)) {
-                continue;
-            }
-
-            $this->zipService->addFromString("$level.log", $log);
+        foreach ($logFiles as $filePath) {
+            $this->zipService->addFile($filePath);
         }
 
         $this->zipService->close();
@@ -99,6 +77,6 @@ protected function createZipPath(): string
         $timestamp = date('Y-m-d_H-i-s');
         $filename  = "{$timestamp}_{$appInfo->name}_logs.zip";
 
-        return $appInfo->path . $filename;
+        return $appInfo->createPath($filename);
     }
 }

src/Base/Contract/ZipServiceInterface.php

@@ -9,7 +9,7 @@ interface ZipServiceInterface
     /**
      * Add a file to the zip archive. Optionally specify a target filename.
      */
-    public function addFile(string $filename, ?string $targetFilename): void;
+    public function addFile(string $filename, ?string $targetFilename = null): void;
 
     /**
      * Add a file to the zip archive from a string.

src/Base/Model/AppInfo.php

@@ -28,4 +28,16 @@ class AppInfo extends Model
         'path'    => 'string',
         'url'     => 'string',
     ];
+
+    /**
+     * @param  string $path
+     *
+     * @return string
+     */
+    public function createPath(string $path): string
+    {
+        $pattern = sprintf('/\%s+/', DIRECTORY_SEPARATOR);
+
+        return preg_replace($pattern, DIRECTORY_SEPARATOR, "$this->path/$path");
+    }
 }

src/Base/Service/ZipService.php

@@ -5,12 +5,14 @@
 namespace MyParcelNL\Pdk\Base\Service;
 
 use MyParcelNL\Pdk\Base\Contract\ZipServiceInterface;
+use RuntimeException;
 use ZipArchive;
+use function basename;
 
 class ZipService implements ZipServiceInterface
 {
     /**
-     * @var \ZipArchive
+     * @var null|\ZipArchive
      */
     private $currentFile;
 
@@ -19,10 +21,16 @@ class ZipService implements ZipServiceInterface
      * @param  null|string $targetFilename
      *
      * @return void
+     * @throws \RuntimeException
      */
-    public function addFile(string $filename, ?string $targetFilename): void
+    public function addFile(string $filename, ?string $targetFilename = null): void
     {
-        $this->currentFile->addFile($filename, $targetFilename ?? $filename);
+        $this->validateHasFile();
+        $success = $this->currentFile->addFile($filename, $targetFilename ?? basename($filename));
+
+        if (! $success) {
+            throw new RuntimeException('Failed to add file to zip');
+        }
     }
 
     /**
@@ -33,6 +41,7 @@ public function addFile(string $filename, ?string $targetFilename): void
      */
     public function addFromString(string $string, string $targetFilename): void
     {
+        $this->validateHasFile();
         $this->currentFile->addFromString($targetFilename, $string);
     }
 
@@ -41,6 +50,7 @@ public function addFromString(string $string, string $targetFilename): void
      */
     public function close(): void
     {
+        $this->validateHasFile();
         $this->currentFile->close();
         $this->currentFile = null;
     }
@@ -58,4 +68,16 @@ public function create(string $filename): void
 
         $this->currentFile = $zip;
     }
+
+    /**
+     * @return void
+     */
+    private function validateHasFile(): void
+    {
+        if (null !== $this->currentFile) {
+            return;
+        }
+
+        throw new RuntimeException('No zip file is open');
+    }
 }

src/Facade/Logger.php

@@ -19,6 +19,7 @@
  * @method static void notice($message, array $context = [])
  * @method static void warning($message, array $context = [])
  * @method static void deprecated(string $subject, string $replacement = null, array $context = [])
+ * @method static array getLogFiles()
  * @see \MyParcelNL\Pdk\Logger\Contract\PdkLoggerInterface
  */
 final class Logger extends Facade

src/Logger/AbstractLogger.php

@@ -96,10 +96,11 @@ public function error($message, array $context = []): void
     }
 
     /**
-     * @TODO: remove this default in v3.0.0, for now it's here to prevent breaking changes
-     * @return array
+     * @TODO   : remove this default in v3.0.0, for now it's here to prevent breaking changes
+     * @return array{notice?: string, warning?: string, error?: string, critical?: string, alert?: string, emergency?: string}
+     * @codeCoverageIgnore
      */
-    public function getLogs(): array
+    public function getLogFiles(): array
     {
         return [];
     }

src/Logger/Contract/PdkLoggerInterface.php

@@ -18,7 +18,9 @@ interface PdkLoggerInterface extends LoggerInterface
     public function deprecated(string $subject, ?string $replacement = null, array $context = []): void;
 
     /**
-     * @return array
+     * Get all logs as an associative array with log levels as keys and log file paths as values.
+     *
+     * @return array{notice?: string, warning?: string, error?: string, critical?: string, alert?: string, emergency?: string}
      */
-    public function getLogs(): array;
+    public function getLogFiles(): array;
 }

tests/Bootstrap/MockLogger.php

@@ -4,15 +4,59 @@
 
 namespace MyParcelNL\Pdk\Tests\Bootstrap;
 
+use MyParcelNL\Pdk\Base\FileSystemInterface;
+use MyParcelNL\Pdk\Facade\Pdk;
 use MyParcelNL\Pdk\Logger\AbstractLogger;
+use Psr\Log\LogLevel;
+use function array_filter;
+use function array_reduce;
+use function json_encode;
 
 class MockLogger extends AbstractLogger
 {
+    private const ALL_LOG_LEVELS = [
+        LogLevel::ALERT,
+        LogLevel::CRITICAL,
+        LogLevel::DEBUG,
+        LogLevel::EMERGENCY,
+        LogLevel::ERROR,
+        LogLevel::INFO,
+        LogLevel::NOTICE,
+        LogLevel::WARNING,
+    ];
+
+    /**
+     * @var \MyParcelNL\Pdk\Base\FileSystemInterface
+     */
+    private $fileSystem;
+
     /**
      * @var array
      */
     private $logs = [];
 
+    /**
+     * @var array<string, resource>
+     */
+    private $streams = [];
+
+    /**
+     * @param  \MyParcelNL\Pdk\Base\FileSystemInterface $fileSystem
+     */
+    public function __construct(FileSystemInterface $fileSystem)
+    {
+        $this->fileSystem = $fileSystem;
+
+        $appInfo = Pdk::getAppInfo();
+        $this->fileSystem->mkdir($appInfo->createPath('logs'), true);
+
+        foreach (self::ALL_LOG_LEVELS as $level) {
+            $filename = $appInfo->createPath("logs/test_$level.log");
+
+            $this->streams[$level] = $this->fileSystem->openStream($filename, 'w');
+        }
+    }
+
     /**
      * @return void
      */
@@ -21,6 +65,20 @@ public function clear(): void
         $this->logs = [];
     }
 
+    /**
+     * @inheritDoc
+     */
+    public function getLogFiles(): array
+    {
+        $appInfo = Pdk::getAppInfo();
+
+        return array_reduce(self::ALL_LOG_LEVELS, static function (array $acc, string $level) use ($appInfo) {
+            $acc[$level] = $appInfo->createPath("logs/test_$level.log");
+
+            return $acc;
+        }, []);
+    }
+
     /**
      * @return array
      */
@@ -43,5 +101,13 @@ public function log($level, $message, array $context = []): void
             'message' => $message,
             'context' => $context,
         ];
+
+        $formattedString = implode(' ', array_filter([
+            "!$level!",
+            $message,
+            empty($context) ? null : json_encode($context),
+        ]));
+
+        $this->fileSystem->writeToStream($this->streams[$level], $formattedString . PHP_EOL);
     }
 }

tests/Bootstrap/MockPdkConfig.php

@@ -69,7 +69,7 @@ private static function getDefaultConfig(): array
                     'name'    => 'pest',
                     'title'   => 'Pest',
                     'version' => '1.0.0',
-                    'path'    => 'APP_PATH',
+                    'path'    => '/app/.tmp/',
                     'url'     => 'APP_URL',
                 ]);
             }),

tests/Unit/App/Action/Backend/Debug/DebugDownloadLogsActionTest.php

@@ -0,0 +1,75 @@
+<?php
+/** @noinspection StaticClosureCanBeUsedInspection */
+
+declare(strict_types=1);
+
+namespace MyParcelNL\Pdk\App\Action\Backend\Debug;
+
+use MyParcelNL\Pdk\App\Api\Backend\PdkBackendActions;
+use MyParcelNL\Pdk\Base\FileSystem;
+use MyParcelNL\Pdk\Base\FileSystemInterface;
+use MyParcelNL\Pdk\Facade\Actions;
+use MyParcelNL\Pdk\Facade\Logger;
+use MyParcelNL\Pdk\Tests\Uses\UsesMockPdkInstance;
+use Symfony\Component\HttpFoundation\BinaryFileResponse;
+use Symfony\Component\HttpFoundation\Request;
+use function DI\get;
+use function MyParcelNL\Pdk\Tests\readZip;
+use function MyParcelNL\Pdk\Tests\usesShared;
+use const PHP_EOL;
+
+usesShared(new UsesMockPdkInstance([
+    // Using real file system because we are not mocking ZipArchive.
+    FileSystemInterface::class => get(FileSystem::class),
+]));
+
+test('it downloads logs', function () {
+    // Warning and notice are not called to check if they're omitted from the created zip for being empty.
+    Logger::emergency('emergency message');
+    Logger::alert('hi');
+    Logger::critical('some string');
+    Logger::error('error message');
+    Logger::info('info message with context', ['some' => 'context']);
+    Logger::debug('debug message');
+    Logger::debug('debug message 2');
+    Logger::debug('debug message 3');
+
+    $request = new Request(['action' => PdkBackendActions::DEBUG_DOWNLOAD_LOGS]);
+
+    /** @var \Symfony\Component\HttpFoundation\BinaryFileResponse $response */
+    $response = Actions::execute($request);
+
+    $file = $response->getFile();
+
+    expect($response)
+        ->toBeInstanceOf(BinaryFileResponse::class)
+        ->and($response->getStatusCode())
+        ->toBe(200)
+        ->and($file->isFile())
+        ->toBeTrue()
+        ->and($file->getFilename())
+        ->toMatch('/\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}_pest_logs.zip$/');
+
+    // Check if the returned zip file contains the logs
+    $logs = readZip($file->getPathname());
+
+    expect($logs)->toEqual([
+        'test_emergency.log' => '!emergency! [PDK]: emergency message' . PHP_EOL,
+        'test_alert.log'     => '!alert! [PDK]: hi' . PHP_EOL,
+        'test_critical.log'  => '!critical! [PDK]: some string' . PHP_EOL,
+        'test_error.log'     => '!error! [PDK]: error message' . PHP_EOL,
+        'test_info.log'      => '!info! [PDK]: info message with context {"some":"context"}' . PHP_EOL,
+        'test_debug.log'     => implode(PHP_EOL, [
+                '!debug! [PDK]: debug message',
+                '!debug! [PDK]: debug message 2',
+                '!debug! [PDK]: debug message 3',
+            ]) . PHP_EOL,
+        'test_notice.log'    => '',
+        'test_warning.log'   => '',
+    ]);
+
+    // Send the response to check if the created file is deleted after sending
+    $response->send();
+
+    expect($file->isFile())->toBeFalse();
+});