diff --git a/lib/Http/Client/Middleware/CountingStream.php b/lib/Http/Client/Middleware/CountingStream.php index c8a6367..dd1c100 100644 --- a/lib/Http/Client/Middleware/CountingStream.php +++ b/lib/Http/Client/Middleware/CountingStream.php @@ -195,12 +195,7 @@ private function writeLog(bool $complete): void { [$jsonFile, $plainFile] = LogPathHelper::getPathsFromMeta($this->meta, $this->logBaseDir, $this->reqId); - if (in_array($this->logFormat, ['json', 'both'], true)) { - @file_put_contents($jsonFile, json_encode($merged, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . PHP_EOL, FILE_APPEND | LOCK_EX); - } - if (in_array($this->logFormat, ['plain', 'both'], true)) { - @file_put_contents($plainFile, $plain, FILE_APPEND | LOCK_EX); - } + LogWriter::write($this->logFormat, $jsonFile, $merged, $plainFile, $plain, $this->logger); TransferStatsStore::clear($this->reqId); } catch (\Throwable $e) { diff --git a/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php b/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php index ccdfb1a..114b186 100644 --- a/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php +++ b/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php @@ -195,7 +195,7 @@ function (ResponseInterface $response) use ($request, $reqHeaders, $reqId) { if ($immediate) { if ($this->shouldLog($intStatus)) { - $this->writeImmediate($reqId, $meta, $respHeaders, $handlerStats); + $this->writeImmediate($reqId, $meta, $handlerStats); } TransferStatsStore::clear($reqId); } else { @@ -250,12 +250,7 @@ function ($reason) use ($request, $reqHeaders, $reqId) { ]; [$jsonFile, $plainFile] = LogPathHelper::getPathsFromMeta($metaForPaths, $this->logBaseDir, $reqId); - if (in_array($this->logFormat, ['json', 'both'], true)) { - @file_put_contents($jsonFile, json_encode($entry, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . PHP_EOL, FILE_APPEND | LOCK_EX); - } - if (in_array($this->logFormat, ['plain', 'both'], true)) { - @file_put_contents($plainFile, $plain, FILE_APPEND | LOCK_EX); - } + LogWriter::write($this->logFormat, $jsonFile, $entry, $plainFile, $plain, $this->logger); TransferStatsStore::clear($reqId); } catch (\Throwable $e) { @@ -271,7 +266,7 @@ function ($reason) use ($request, $reqHeaders, $reqId) { }; } - private function writeImmediate(string $reqId, array $meta, array $respHeaders, ?array $handlerStats): void { + private function writeImmediate(string $reqId, array $meta, ?array $handlerStats): void { try { $compressed = null; $encoding = 'none'; @@ -280,7 +275,7 @@ private function writeImmediate(string $reqId, array $meta, array $respHeaders, $compressed = (int)round($handlerStats['size_download']); } - $compact = $this->compactHeaders($respHeaders); + $compact = $meta['responseHeaders']; foreach ($compact as $k => $v) { $lk = strtolower($k); @@ -358,12 +353,7 @@ private function writeImmediate(string $reqId, array $meta, array $respHeaders, [$jsonFile, $plainFile] = LogPathHelper::getPathsFromMeta($meta, $this->logBaseDir, $reqId); - if (in_array($this->logFormat, ['json', 'both'], true)) { - @file_put_contents($jsonFile, json_encode($merged, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . PHP_EOL, FILE_APPEND | LOCK_EX); - } - if (in_array($this->logFormat, ['plain', 'both'], true)) { - @file_put_contents($plainFile, $plain, FILE_APPEND | LOCK_EX); - } + LogWriter::write($this->logFormat, $jsonFile, $merged, $plainFile, $plain, $this->logger); } catch (\Throwable $e) { $this->logger->debug('HttpClientLoggerMiddleware: writeImmediate failed: ' . $e->getMessage()); } diff --git a/lib/Http/Client/Middleware/LogWriter.php b/lib/Http/Client/Middleware/LogWriter.php new file mode 100644 index 0000000..fe5a7b5 --- /dev/null +++ b/lib/Http/Client/Middleware/LogWriter.php @@ -0,0 +1,49 @@ + + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +declare(strict_types=1); + +namespace OCA\AdminAuditHttpClient\Http\Client\Middleware; + +use Psr\Log\LoggerInterface; + +/** + * Single write path for all log sinks. Appends with an exclusive lock and + * reports the first failed write per process as a warning — without that, a + * full or unwritable log directory would go entirely unnoticed. + */ +class LogWriter { + private static bool $failureLogged = false; + + public static function write( + string $format, + string $jsonFile, + array $entry, + string $plainFile, + string $plainLine, + LoggerInterface $logger, + ): void { + if (in_array($format, ['json', 'both'], true)) { + self::append($jsonFile, json_encode($entry, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . PHP_EOL, $logger); + } + if (in_array($format, ['plain', 'both'], true)) { + self::append($plainFile, $plainLine, $logger); + } + } + + private static function append(string $file, string $line, LoggerInterface $logger): void { + if (@file_put_contents($file, $line, FILE_APPEND | LOCK_EX) !== false) { + return; + } + if (!self::$failureLogged) { + self::$failureLogged = true; + $logger->warning( + 'admin_audit_http_client: could not write to ' . $file . ' - further write failures are not reported' + ); + } + } +} diff --git a/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php b/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php index fe0d6e3..ce9019b 100644 --- a/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php +++ b/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php @@ -171,7 +171,7 @@ public function testWriteImmediateOmitsDecompressedBytesAndRatio(): void { ]; try { - $this->invokePrivate($mw, 'writeImmediate', ['req-wi-1', $meta, [], ['size_download' => 10]]); + $this->invokePrivate($mw, 'writeImmediate', ['req-wi-1', $meta, ['size_download' => 10]]); $lines = file($dir . '/example.com.json', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); $this->assertNotFalse($lines); diff --git a/tests/unit/Http/Client/Middleware/LogWriterTest.php b/tests/unit/Http/Client/Middleware/LogWriterTest.php new file mode 100644 index 0000000..714a843 --- /dev/null +++ b/tests/unit/Http/Client/Middleware/LogWriterTest.php @@ -0,0 +1,65 @@ + + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace OCA\AdminAuditHttpClient\Tests\Unit\Http\Client\Middleware; + +use OCA\AdminAuditHttpClient\Http\Client\Middleware\LogWriter; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; + +class LogWriterTest extends TestCase { + private string $baseDir; + + protected function setUp(): void { + parent::setUp(); + $this->baseDir = sys_get_temp_dir() . '/aahc-writer-' . bin2hex(random_bytes(4)); + mkdir($this->baseDir); + } + + protected function tearDown(): void { + foreach (glob($this->baseDir . '/*') ?: [] as $file) { + @unlink($file); + } + @rmdir($this->baseDir); + parent::tearDown(); + } + + public function testWritesJsonAndPlainForFormatBoth(): void { + $json = $this->baseDir . '/host.json'; + $plain = $this->baseDir . '/host.log'; + + LogWriter::write('both', $json, ['a' => 1], $plain, "plain line\n", new NullLogger()); + + $this->assertSame('{"a":1}' . PHP_EOL, file_get_contents($json)); + $this->assertSame("plain line\n", file_get_contents($plain)); + } + + public function testRespectsFormatSelection(): void { + $json = $this->baseDir . '/host.json'; + $plain = $this->baseDir . '/host.log'; + + LogWriter::write('json', $json, ['a' => 1], $plain, "plain line\n", new NullLogger()); + + $this->assertFileExists($json); + $this->assertFileDoesNotExist($plain); + } + + public function testWarnsExactlyOnceOnWriteFailure(): void { + $blocker = $this->baseDir . '/blocker'; + touch($blocker); + + $logger = $this->createMock(LoggerInterface::class); + // Both appends fail (paths below a regular file); once() proves the + // second failure is suppressed. + $logger->expects($this->once())->method('warning'); + + LogWriter::write('both', $blocker . '/x.json', ['a' => 1], $blocker . '/x.log', "line\n", $logger); + } +}