From 3b6ee90a7e2f94c47ea687e1e196ec9e69fbeeee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Tue, 24 Mar 2026 10:02:46 +0100 Subject: [PATCH] feat: refresh existing snapshots from XML after sync --- config/packages/zenstruck_schedule.yaml | 4 +- src/BusProNet/XmlLoader/PickupLoader.php | 3 + src/BusProNet/XmlLoader/TravelLoader.php | 2 +- .../BpnRefreshTravelSnapshotCommand.php | 18 ++- src/Command/BpnXmlSyncCommand.php | 43 ++++- src/Service/TravelDataService.php | 50 ++++++ src/Service/TravelSnapshotService.php | 14 +- tests/Command/BpnXmlSyncCommandTest.php | 148 ++++++++++++++++++ tests/Service/TravelSnapshotServiceTest.php | 29 +++- 9 files changed, 289 insertions(+), 22 deletions(-) create mode 100644 tests/Command/BpnXmlSyncCommandTest.php diff --git a/config/packages/zenstruck_schedule.yaml b/config/packages/zenstruck_schedule.yaml index 02683a9..a02d482 100644 --- a/config/packages/zenstruck_schedule.yaml +++ b/config/packages/zenstruck_schedule.yaml @@ -33,11 +33,11 @@ zenstruck_schedule: description: "Removes expired oauth2 tokens hourly" - task: app:bpn:refresh-travel-snapshot --refresh-after 60 - frequency: "0 8-19 * * *" + frequency: "10 8-19 * * *" description: "Syncs travel data snapshots with BusPro XML data hourly during peak hours" - task: app:bpn:refresh-travel-snapshot --refresh-after 180 - frequency: "0 20-23,0-7 * * *" + frequency: "10 20-23,0-7 * * *" description: "Syncs travel data snapshots with BusPro XML data every 3 hours outside peak hours" when@staging: diff --git a/src/BusProNet/XmlLoader/PickupLoader.php b/src/BusProNet/XmlLoader/PickupLoader.php index ffbc253..1d0189f 100644 --- a/src/BusProNet/XmlLoader/PickupLoader.php +++ b/src/BusProNet/XmlLoader/PickupLoader.php @@ -137,6 +137,9 @@ class PickupLoader extends AbstractLoader { foreach ($pickups as $pickup) { $pickupData = $this->loadById($pickup->id); + if (null === $pickupData) { + continue; + } $pickup->code = $pickupData->code; $pickup->postalCode = $pickupData->postalCode; diff --git a/src/BusProNet/XmlLoader/TravelLoader.php b/src/BusProNet/XmlLoader/TravelLoader.php index 146c9e8..0098fd0 100644 --- a/src/BusProNet/XmlLoader/TravelLoader.php +++ b/src/BusProNet/XmlLoader/TravelLoader.php @@ -102,7 +102,7 @@ class TravelLoader extends AbstractLoader return $mapping; }); } catch (InvalidArgumentException $e) { - return []; + throw new \RuntimeException('Cache key error in generateFilesMap', 0, $e); } } diff --git a/src/Command/BpnRefreshTravelSnapshotCommand.php b/src/Command/BpnRefreshTravelSnapshotCommand.php index e72c58c..3d72b6e 100644 --- a/src/Command/BpnRefreshTravelSnapshotCommand.php +++ b/src/Command/BpnRefreshTravelSnapshotCommand.php @@ -9,6 +9,7 @@ use App\Service\TravelSnapshotService; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Command\LockableTrait; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -26,6 +27,8 @@ use Symfony\Component\Console\Style\SymfonyStyle; */ class BpnRefreshTravelSnapshotCommand extends Command { + use LockableTrait; + public function __construct( private readonly TravelSnapshotService $snapshotService, private readonly LoggerInterface $logger, @@ -52,19 +55,28 @@ class BpnRefreshTravelSnapshotCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); + + if (false === $this->lock('bpn_xml_sync')) { + $this->logger->info('Snapshot refresh skipped: xml-sync is running'); + + return Command::SUCCESS; + } + $limit = (int) $input->getOption('limit'); $force = true === $input->getOption('force'); $refreshAfterMinutes = (int) $input->getOption('refresh-after'); - $xmlDateIds = []; + $xmlFileMap = null; try { - $xmlDateIds = array_keys($this->travelLoader->generateFilesMap()); + $xmlFileMap = $this->travelLoader->generateFilesMap(); } catch (\Throwable $e) { - $this->logger->warning('Failed to load XML files map for snapshot refresh filtering; refreshing all snapshots', [ + $this->logger->warning('Failed to load XML files map for snapshot refresh filtering; skipping all candidates', [ 'error' => $e->getMessage(), ]); } + // Phase 2 — enrich non-XML-backed snapshots via extended-availability API + $xmlDateIds = null !== $xmlFileMap ? array_keys($xmlFileMap) : null; $result = $this->snapshotService->refreshExtendedSnapshots($limit, $force, $refreshAfterMinutes, $xmlDateIds); $io->success(sprintf( diff --git a/src/Command/BpnXmlSyncCommand.php b/src/Command/BpnXmlSyncCommand.php index e9daaa0..49a73c5 100644 --- a/src/Command/BpnXmlSyncCommand.php +++ b/src/Command/BpnXmlSyncCommand.php @@ -5,6 +5,8 @@ declare(strict_types=1); namespace App\Command; use App\BusProNet\Model\XmlExportInfo; +use App\BusProNet\XmlLoader\TravelLoader; +use App\Service\TravelDataService; use League\Flysystem\FilesystemException; use League\Flysystem\FilesystemOperator; use League\Flysystem\StorageAttributes; @@ -38,6 +40,8 @@ class BpnXmlSyncCommand extends Command private readonly FilesystemOperator $xmlExport, private readonly CacheInterface $cache, private readonly LoggerInterface $logger, + private readonly TravelLoader $travelLoader, + private readonly TravelDataService $travelDataService, ) { parent::__construct(); } @@ -53,7 +57,7 @@ class BpnXmlSyncCommand extends Command { $io = new SymfonyStyle($input, $output); - if (false === $this->lock()) { + if (false === $this->lock('bpn_xml_sync')) { $io->warning('Sync is already running in another process'); $this->logger->info('XML sync skipped: another instance is running'); @@ -115,13 +119,38 @@ class BpnXmlSyncCommand extends Command } $this->invalidateCaches(); - $io->text(sprintf('Invalidated %d cache keys', count(self::CACHE_KEYS_TO_INVALIDATE))); + $io->info(sprintf('Invalidated %d cache keys', count(self::CACHE_KEYS_TO_INVALIDATE))); - $io->success(sprintf( - 'Sync complete: %d files updated, %d files deleted', - $syncResult['updated'], - $syncResult['deleted'] - )); + $io->section('Refreshing snapshots'); + + $xmlFileMap = null; + $snapshotResult = null; + try { + $xmlFileMap = $this->travelLoader->generateFilesMap(); + } catch (\Throwable $e) { + $this->logger->warning('Failed to generate XML file map after sync; snapshot refresh skipped', [ + 'error' => $e->getMessage(), + ]); + } + + if (null !== $xmlFileMap) { + $total = array_sum(array_map(fn ($entry) => count($entry['hotels']), $xmlFileMap)); + $io->progressStart($total); + + $snapshotResult = $this->travelDataService->syncSnapshotsFromXml( + $xmlFileMap, + function () use ($io): void { $io->progressAdvance(); }, + ); + + $io->progressFinish(); + $this->logger->info('Travel snapshot XML sync triggered by xml-sync command', $snapshotResult); + } + + $successMessage = sprintf('Sync complete: %d files updated, %d files deleted', $syncResult['updated'], $syncResult['deleted']); + if (null !== $snapshotResult) { + $successMessage .= sprintf(', snapshots: %d processed, %d failed', $snapshotResult['processed'], $snapshotResult['failed']); + } + $io->success($successMessage); $this->logger->info('XML sync completed', [ 'files_updated' => $syncResult['updated'], 'files_deleted' => $syncResult['deleted'], diff --git a/src/Service/TravelDataService.php b/src/Service/TravelDataService.php index 48cb1f5..623bf0c 100644 --- a/src/Service/TravelDataService.php +++ b/src/Service/TravelDataService.php @@ -179,6 +179,56 @@ class TravelDataService return $travel; } + /** + * Syncs existing snapshots from fresh XML for all entries in the given file map. + * + * Iterates every date/hotel combination, calls getTravelDataFromXml() which + * loads, enriches, and upserts the snapshot (hash-guarded; no write if unchanged). + * + * @param array}> $xmlFileMap + * + * @return array{processed:int,failed:int} + */ + public function syncSnapshotsFromXml(array $xmlFileMap, ?callable $onProgress = null): array + { + $processed = 0; + $failed = 0; + + foreach ($xmlFileMap as $dateId => $entry) { + foreach (array_keys($entry['hotels']) as $hotelId) { + try { + $travel = $this->getTravelDataFromXml((int) $dateId, $hotelId); + } catch (\Throwable $e) { + $this->logger->warning('Failed to sync snapshot from XML', [ + 'dateId' => $dateId, + 'hotelId' => $hotelId, + 'error' => $e->getMessage(), + ]); + ++$failed; + if (null !== $onProgress) { + ($onProgress)(); + } + continue; + } + + if (null === $travel) { + ++$failed; + if (null !== $onProgress) { + ($onProgress)(); + } + continue; + } + + ++$processed; + if (null !== $onProgress) { + ($onProgress)(); + } + } + } + + return ['processed' => $processed, 'failed' => $failed]; + } + /** * Retrieve travel data specifically from remote API. * diff --git a/src/Service/TravelSnapshotService.php b/src/Service/TravelSnapshotService.php index 7a81b34..2e8bac2 100644 --- a/src/Service/TravelSnapshotService.php +++ b/src/Service/TravelSnapshotService.php @@ -176,9 +176,9 @@ class TravelSnapshotService * * @return array{processed:int,updated:int,failed:int} */ - public function refreshExtendedSnapshots(int $limit = 500, bool $force = false, int $refreshAfterMinutes = 360, array $xmlAvailableDateIds = []): array + public function refreshExtendedSnapshots(int $limit = 500, bool $force = false, int $refreshAfterMinutes = 360, ?array $xmlAvailableDateIds = null): array { - $dateToThreshold = new \DateTimeImmutable(sprintf('-%d days', $this->retentionBufferDays)); + $dateToThreshold = new \DateTimeImmutable('today'); $refreshBefore = new \DateTimeImmutable(sprintf('-%d minutes', $refreshAfterMinutes)); $candidates = true === $force @@ -192,10 +192,12 @@ class TravelSnapshotService $extendedResponseByDateId = []; foreach ($candidates as $snapshot) { - // Skip travels whose XML is still live; their snapshot is kept authoritative - // by the XML load path (getTravelDataFromXml → upsertFromTravel). An empty - // $xmlAvailableDateIds means the file map could not be loaded, so fall back - // to refreshing everything rather than skipping all candidates. + // null → file map unavailable; skip all candidates (conservative fallback) + // [] → no XML files exist; fall through and refresh everything + // [...] → skip travels whose XML is still live + if (null === $xmlAvailableDateIds) { + continue; + } if ([] !== $xmlAvailableDateIds && in_array($snapshot->getDateId(), $xmlAvailableDateIds, true)) { continue; } diff --git a/tests/Command/BpnXmlSyncCommandTest.php b/tests/Command/BpnXmlSyncCommandTest.php new file mode 100644 index 0000000..1035b19 --- /dev/null +++ b/tests/Command/BpnXmlSyncCommandTest.php @@ -0,0 +1,148 @@ +command, 'release'); + $release->setAccessible(true); + $release->invoke($this->command); + } + + protected function setUp(): void + { + $this->xmlSource = $this->createMock(FilesystemOperator::class); + $this->xmlExport = $this->createMock(FilesystemOperator::class); + $this->cache = $this->createMock(CacheInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->travelLoader = $this->createMock(TravelLoader::class); + $this->travelDataService = $this->createMock(TravelDataService::class); + + $this->command = new BpnXmlSyncCommand( + $this->xmlSource, + $this->xmlExport, + $this->cache, + $this->logger, + $this->travelLoader, + $this->travelDataService, + ); + } + + /** + * Remote newer than local → files downloaded → syncSnapshotsFromXml() called once. + */ + public function testSnapshotSyncIsTriggeredAfterSuccessfulFileDownload(): void + { + $this->xmlSource->method('read') + ->willReturn("24.03.2026 12:00:00\nExport\n3 Dateien\n"); + + $this->xmlExport->method('fileExists')->willReturn(true); + $this->xmlExport->method('read') + ->willReturn("24.03.2026 10:00:00\nExport\n3 Dateien\n"); + + // No remote files to copy (keeps syncFiles() trivial while still completing). + $this->xmlSource->method('listContents') + ->willReturn(new DirectoryListing([])); + $this->xmlExport->method('listContents') + ->willReturn(new DirectoryListing([])); + + $fileMap = [ + 101 => ['hotels' => ['H1' => null, 'H2' => null]], + 102 => ['hotels' => ['H3' => null]], + ]; + $this->travelLoader->expects($this->once()) + ->method('generateFilesMap') + ->willReturn($fileMap); + + $this->travelDataService->expects($this->once()) + ->method('syncSnapshotsFromXml') + ->with($fileMap, $this->isInstanceOf(\Closure::class)) + ->willReturn(['processed' => 2, 'failed' => 0]); + + $tester = new CommandTester($this->command); + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertStringContainsString('snapshots: 2 processed, 0 failed', $tester->getDisplay()); + } + + /** + * Local already up to date → early return before snapshot code is reached. + */ + public function testSnapshotSyncIsSkippedWhenLocalDataIsUpToDate(): void + { + $sameTimestamp = "24.03.2026 10:00:00\nExport\n3 Dateien\n"; + $this->xmlSource->method('read')->willReturn($sameTimestamp); + $this->xmlExport->method('fileExists')->willReturn(true); + $this->xmlExport->method('read')->willReturn($sameTimestamp); + + $this->travelLoader->expects($this->never())->method('generateFilesMap'); + $this->travelDataService->expects($this->never())->method('syncSnapshotsFromXml'); + + $tester = new CommandTester($this->command); + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + } + + /** + * generateFilesMap() throws → warning logged, syncSnapshotsFromXml() never called, + * command still returns SUCCESS. + */ + public function testSnapshotSyncIsSkippedAndWarningLoggedWhenFileMapGenerationFails(): void + { + $this->xmlSource->method('read') + ->willReturn("24.03.2026 12:00:00\nExport\n3 Dateien\n"); + + $this->xmlExport->method('fileExists')->willReturn(true); + $this->xmlExport->method('read') + ->willReturn("24.03.2026 10:00:00\nExport\n3 Dateien\n"); + + $this->xmlSource->method('listContents') + ->willReturn(new DirectoryListing([])); + $this->xmlExport->method('listContents') + ->willReturn(new DirectoryListing([])); + + $this->travelLoader->expects($this->once()) + ->method('generateFilesMap') + ->willThrowException(new \RuntimeException('Storage unavailable')); + + $this->logger->expects($this->atLeastOnce()) + ->method('warning') + ->with( + $this->stringContains('snapshot refresh skipped'), + $this->anything(), + ); + + $this->travelDataService->expects($this->never())->method('syncSnapshotsFromXml'); + + $tester = new CommandTester($this->command); + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + } +} diff --git a/tests/Service/TravelSnapshotServiceTest.php b/tests/Service/TravelSnapshotServiceTest.php index a3bd537..d365219 100644 --- a/tests/Service/TravelSnapshotServiceTest.php +++ b/tests/Service/TravelSnapshotServiceTest.php @@ -336,7 +336,7 @@ class TravelSnapshotServiceTest extends TestCase ->expects($this->once()) ->method('flush'); - $result = $this->service->refreshExtendedSnapshots(); + $result = $this->service->refreshExtendedSnapshots(500, false, 360, []); $this->assertSame(['processed' => 1, 'updated' => 1, 'failed' => 0], $result); } @@ -374,7 +374,7 @@ class TravelSnapshotServiceTest extends TestCase ->expects($this->never()) ->method('flush'); - $result = $this->service->refreshExtendedSnapshots(); + $result = $this->service->refreshExtendedSnapshots(500, false, 360, []); $this->assertSame(['processed' => 1, 'updated' => 0, 'failed' => 1], $result); } @@ -403,11 +403,34 @@ class TravelSnapshotServiceTest extends TestCase ->expects($this->never()) ->method('flush'); - $result = $this->service->refreshExtendedSnapshots(); + $result = $this->service->refreshExtendedSnapshots(500, false, 360, []); $this->assertSame(['processed' => 1, 'updated' => 0, 'failed' => 1], $result); } + public function testRefreshSkipsAllCandidatesWhenXmlDateIdsIsNull(): void + { + $payload = '{"id":100}'; + $snapshot = new TravelSnapshot(100, 200, $payload, hash('sha256', $payload)); + + $this->snapshotRepository + ->expects($this->once()) + ->method('findRefreshCandidates') + ->willReturn([$snapshot]); + + $this->apiClient + ->expects($this->never()) + ->method('getAvailabilitiesExtended'); + + $this->entityManager + ->expects($this->never()) + ->method('flush'); + + $result = $this->service->refreshExtendedSnapshots(500, false, 360, null); + + $this->assertSame(['processed' => 0, 'updated' => 0, 'failed' => 0], $result); + } + public function testRefreshSkipsSnapshotsWithXmlAvailable(): void { $payload = '{"id":100}';