From c2c55bc8b2620915c3523ea2eb32b9a5672a4ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Wed, 30 Sep 2026 12:09:57 +0200 Subject: [PATCH] fix: answer null instead of 500 when the export marker is missing --- docs/api-consumer-guide.md | 4 +- src/BusProNet/Model/XmlExportInfo.php | 23 +++++++++ src/Controller/Api/LastUpdateController.php | 21 +++----- .../Api/LastUpdateControllerTest.php | 50 +++++++++++++++++++ 4 files changed, 83 insertions(+), 15 deletions(-) create mode 100644 tests/Controller/Api/LastUpdateControllerTest.php diff --git a/docs/api-consumer-guide.md b/docs/api-consumer-guide.md index 070ad898..f361fe47 100644 --- a/docs/api-consumer-guide.md +++ b/docs/api-consumer-guide.md @@ -92,7 +92,9 @@ Product master data from the upstream BusProNet system, cached server-side for 1 #### `GET /api/last-update` — scope `api` -Timestamp of the last upstream data export: `{"timestamp": "YYYY-MM-DD HH:MM:SS"}`. Note: if no export marker file exists the current implementation errors (500) rather than returning null — treat a non-JSON/500 response as "unknown". +Timestamp of the last upstream data export: `{"timestamp": "YYYY-MM-DD HH:MM:SS"}` in `Europe/Berlin` local time. +If the export marker is missing or unreadable (for instance mid-sync), the answer is `{"timestamp": null}` with +status 200 — treat null as "unknown", not as "changed". --- diff --git a/src/BusProNet/Model/XmlExportInfo.php b/src/BusProNet/Model/XmlExportInfo.php index b8975ead..aa710054 100644 --- a/src/BusProNet/Model/XmlExportInfo.php +++ b/src/BusProNet/Model/XmlExportInfo.php @@ -4,6 +4,9 @@ declare(strict_types=1); namespace App\BusProNet\Model; +use League\Flysystem\FilesystemException; +use League\Flysystem\FilesystemOperator; + final readonly class XmlExportInfo { private const INFO_FILENAME = 'uebertragung.info'; @@ -20,6 +23,26 @@ final readonly class XmlExportInfo return self::INFO_FILENAME; } + /** + * Reads the marker from a storage, or null when it is missing or unreadable. + * + * A marker caught mid-sync or truncated is treated as absent rather than as an error: + * callers only ever want to know which export they are looking at, and "unknown" is + * an answer they can act on. + */ + public static function fromStorage(FilesystemOperator $storage): ?self + { + try { + if (false === $storage->fileExists(self::INFO_FILENAME)) { + return null; + } + + return self::fromString($storage->read(self::INFO_FILENAME)); + } catch (FilesystemException|\InvalidArgumentException) { + return null; + } + } + public static function fromString(string $content): self { $content = mb_convert_encoding($content, 'UTF-8', 'Windows-1252'); diff --git a/src/Controller/Api/LastUpdateController.php b/src/Controller/Api/LastUpdateController.php index 83652246..597d20c6 100644 --- a/src/Controller/Api/LastUpdateController.php +++ b/src/Controller/Api/LastUpdateController.php @@ -2,6 +2,7 @@ namespace App\Controller\Api; +use App\BusProNet\Model\XmlExportInfo; use League\Flysystem\FilesystemOperator; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\JsonResponse; @@ -16,23 +17,15 @@ class LastUpdateController extends AbstractController { } + /** + * A missing or unreadable marker answers `{"timestamp": null}`, never a 500: consumers + * poll this for change detection and treat null as "unknown". + */ #[Route(path: '/last-update', name: 'api_last_update', methods: ['GET'])] public function index(): JsonResponse { - if (false === $this->xmlExport->has('uebertragung.info')) { - $lastUploadAt = null; - } else { - $file = $this->xmlExport->read('uebertragung.info'); - $lines = explode("\n", $file); - $line = trim($lines[0]); + $info = XmlExportInfo::fromStorage($this->xmlExport); - if (true === empty($line)) { - $lastUploadAt = null; - } else { - $lastUploadAt = \DateTime::createFromFormat('d.m.Y H:i:s', $line); - } - } - - return $this->json(['timestamp' => $lastUploadAt->format('Y-m-d H:i:s')]); + return $this->json(['timestamp' => $info?->lastTransfer->format('Y-m-d H:i:s')]); } } diff --git a/tests/Controller/Api/LastUpdateControllerTest.php b/tests/Controller/Api/LastUpdateControllerTest.php new file mode 100644 index 00000000..f75a78ba --- /dev/null +++ b/tests/Controller/Api/LastUpdateControllerTest.php @@ -0,0 +1,50 @@ +timestampFor( + "22.09.2026 11:10:15\r\nLetzte \xDCbertragung\r\nAnzahl \xDCbertragene Dateien: 177\r\nVollexport\r\n" + )); + } + + public function testAMissingMarkerIsNullRatherThanAServerError(): void + { + self::assertNull($this->timestampFor(null)); + } + + public function testAnEmptyMarkerIsNull(): void + { + self::assertNull($this->timestampFor('')); + } + + public function testAnUnparseableMarkerIsNull(): void + { + self::assertNull($this->timestampFor("gestern\r\nLetzte \xDCbertragung\r\nAnzahl \xDCbertragene Dateien: 177\r\n")); + } + + private function timestampFor(?string $marker): ?string + { + $storage = $this->createStub(FilesystemOperator::class); + $storage->method('fileExists')->willReturn(null !== $marker); + $storage->method('read')->willReturn((string) $marker); + + $controller = new LastUpdateController($storage); + $controller->setContainer(new Container()); + + $body = json_decode((string) $controller->index()->getContent(), true); + self::assertArrayHasKey('timestamp', $body); + + return $body['timestamp']; + } +}