From 5f8a586385552f3060ff72f04770d02ec1f461b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Fri, 11 Sep 2026 19:33:51 +0200 Subject: [PATCH] fix: derive contingent change signal from the sync diff --- migrations/Version20260911170000.php | 26 ++++++++++++++ src/Command/BpnSyncContingentsCommand.php | 3 +- src/Entity/Groups/ContingentSyncState.php | 24 ++----------- src/Model/ContingentSyncResult.php | 11 ++++-- .../Groups/ContingentDayRepository.php | 24 ------------- src/Service/ContingentSnapshotManager.php | 31 +++++++++------- .../Command/BpnSyncContingentsCommandTest.php | 6 ++-- .../Service/ContingentSnapshotManagerTest.php | 35 ++++++++++++++----- 8 files changed, 87 insertions(+), 73 deletions(-) create mode 100644 migrations/Version20260911170000.php diff --git a/migrations/Version20260911170000.php b/migrations/Version20260911170000.php new file mode 100644 index 0000000..b0d8121 --- /dev/null +++ b/migrations/Version20260911170000.php @@ -0,0 +1,26 @@ +addSql('ALTER TABLE contingent_sync_state DROP content_hash'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE contingent_sync_state ADD content_hash VARCHAR(64) NOT NULL'); + } +} diff --git a/src/Command/BpnSyncContingentsCommand.php b/src/Command/BpnSyncContingentsCommand.php index f8733c5..e8c978c 100644 --- a/src/Command/BpnSyncContingentsCommand.php +++ b/src/Command/BpnSyncContingentsCommand.php @@ -114,12 +114,13 @@ class BpnSyncContingentsCommand extends Command } $io->writeln(sprintf( - '%s: %s (+%d ~%d -%d)', + '%s: %s (+%d ~%d -%d, %d beyond the previous horizon)', (string) $accommodation->getCalendarCode(), $result->changed ? 'changed' : 'unchanged', $result->added, $result->updated, $result->removed, + $result->extended, ), OutputInterface::VERBOSITY_VERBOSE); } diff --git a/src/Entity/Groups/ContingentSyncState.php b/src/Entity/Groups/ContingentSyncState.php index 5a34f00..97f4812 100644 --- a/src/Entity/Groups/ContingentSyncState.php +++ b/src/Entity/Groups/ContingentSyncState.php @@ -11,8 +11,8 @@ use Doctrine\ORM\Mapping as ORM; /** * Bookkeeping for the scheduled contingent sync of a single accommodation. * - * Holds the fingerprint of the stored contingent days, which is how the sync decides whether - * anything actually changed, and `changedAt` records when it last did. + * `changedAt` records when the sync last saw a day appear, vanish or switch status within the + * horizon it had already reached; `horizonTo` is how far that reach extends. */ #[ORM\Entity(repositoryClass: ContingentSyncStateRepository::class)] #[ORM\Table(name: 'contingent_sync_state')] @@ -28,13 +28,7 @@ class ContingentSyncState private ?Accommodation $accommodation = null; /** - * sha256 over the stored contingent days, contingent status only. - */ - #[ORM\Column(length: 64)] - private string $contentHash = ''; - - /** - * Last time the fingerprint actually changed. Kept for operators, not exposed via the API. + * Last time the stored snapshot actually changed. Kept for operators, not exposed via the API. */ #[ORM\Column(type: Types::DATETIME_IMMUTABLE, nullable: true)] private ?\DateTimeImmutable $changedAt = null; @@ -76,18 +70,6 @@ class ContingentSyncState return $this; } - public function getContentHash(): string - { - return $this->contentHash; - } - - public function setContentHash(string $contentHash): self - { - $this->contentHash = $contentHash; - - return $this; - } - public function getChangedAt(): ?\DateTimeImmutable { return $this->changedAt; diff --git a/src/Model/ContingentSyncResult.php b/src/Model/ContingentSyncResult.php index b2ec182..cf8d224 100644 --- a/src/Model/ContingentSyncResult.php +++ b/src/Model/ContingentSyncResult.php @@ -13,19 +13,24 @@ readonly class ContingentSyncResult public bool $successful, public bool $changed, public int $added, + public int $extended, public int $updated, public int $removed, public ?string $error = null, ) { } - public static function synced(bool $changed, int $added, int $updated, int $removed): self + /** + * $added counts days newly stored within the horizon the previous run already reached; + * $extended counts days beyond it, which is the rolling window growing rather than a change. + */ + public static function synced(bool $changed, int $added, int $extended, int $updated, int $removed): self { - return new self(true, $changed, $added, $updated, $removed); + return new self(true, $changed, $added, $extended, $updated, $removed); } public static function failed(string $error): self { - return new self(false, false, 0, 0, 0, $error); + return new self(false, false, 0, 0, 0, 0, $error); } } diff --git a/src/Repository/Groups/ContingentDayRepository.php b/src/Repository/Groups/ContingentDayRepository.php index 20a185f..9472dda 100644 --- a/src/Repository/Groups/ContingentDayRepository.php +++ b/src/Repository/Groups/ContingentDayRepository.php @@ -71,30 +71,6 @@ class ContingentDayRepository extends ServiceEntityRepository return $this->indexByDate($days); } - /** - * Returns the accommodation's complete stored snapshot as an ordered "Y-m-d:STATUS" list. - * - * Scalar hydration keeps the fingerprint cheap: the entities themselves are of no interest here. - * - * @return list - */ - public function findStatusFingerprintParts(Accommodation $accommodation): array - { - /** @var array $rows */ - $rows = $this->createQueryBuilder('cd') - ->select('cd.date', 'cd.status') - ->where('cd.accommodation = :accommodation') - ->setParameter('accommodation', $accommodation) - ->orderBy('cd.date', 'ASC') - ->getQuery() - ->getArrayResult(); - - return array_map( - static fn (array $row) => $row['date']->format('Y-m-d').':'.$row['status']->value, - $rows, - ); - } - /** * Retention: drops snapshot days that are in the past and can no longer be requested. */ diff --git a/src/Service/ContingentSnapshotManager.php b/src/Service/ContingentSnapshotManager.php index 995495d..1905ba8 100644 --- a/src/Service/ContingentSnapshotManager.php +++ b/src/Service/ContingentSnapshotManager.php @@ -53,6 +53,10 @@ class ContingentSnapshotManager $state = $this->syncStateRepository->findOneByAccommodation($accommodation) ?? new ContingentSyncState($accommodation); + // Read before recordSuccess() moves it: days beyond the horizon the previous run reached + // are the window growing, not the contingent situation changing. + $previousHorizonTo = $state->getHorizonTo(); + try { $calendar = $this->contingentsClient->getContingentCalendar( $hotelCode, @@ -72,6 +76,7 @@ class ContingentSnapshotManager } $added = 0; + $extended = 0; $updated = 0; $seen = []; @@ -102,7 +107,12 @@ class ContingentSnapshotManager ->setDate($date); $this->entityManager->persist($day); - ++$added; + + if (null !== $previousHorizonTo && $date > $previousHorizonTo) { + ++$extended; + } else { + ++$added; + } } elseif ($day->getStatus() === $entry->status) { continue; } else { @@ -124,26 +134,21 @@ class ContingentSnapshotManager $this->entityManager->flush(); $now = CarbonImmutable::now()->toDateTimeImmutable(); - $hash = $this->fingerprint($accommodation); - $changed = $hash !== $state->getContentHash(); + + // The diff is the change signal. A digest of the stored snapshot cannot be one: the window + // rolls forward a day at a time, so every digest would cover a different span than the one + // it is compared against and every run would report a change. + $changed = $added > 0 || $updated > 0 || $removed > 0; if ($changed) { - $state->setContentHash($hash)->setChangedAt($now); + $state->setChangedAt($now); } $state->recordSuccess($now, $dateTo); $this->entityManager->persist($state); $this->entityManager->flush(); - return ContingentSyncResult::synced($changed, $added, $updated, $removed); - } - - /** - * sha256 over the accommodation's complete stored snapshot, contingent status only. - */ - public function fingerprint(Accommodation $accommodation): string - { - return hash('sha256', implode('|', $this->dayRepository->findStatusFingerprintParts($accommodation))); + return ContingentSyncResult::synced($changed, $added, $extended, $updated, $removed); } private function recordFailure(ContingentSyncState $state, string $hotelCode, string $error): ContingentSyncResult diff --git a/tests/Command/BpnSyncContingentsCommandTest.php b/tests/Command/BpnSyncContingentsCommandTest.php index 0f92427..99c858d 100644 --- a/tests/Command/BpnSyncContingentsCommandTest.php +++ b/tests/Command/BpnSyncContingentsCommandTest.php @@ -36,9 +36,9 @@ class BpnSyncContingentsCommandTest extends TestCase public function testASingleFailingHotelDoesNotFailTheTask(): void { $tester = $this->runSync(['A', 'B', 'C'], [ - ContingentSyncResult::synced(true, 3, 0, 0), + ContingentSyncResult::synced(true, 3, 0, 0, 0), ContingentSyncResult::failed('unknown hotel code'), - ContingentSyncResult::synced(false, 0, 0, 0), + ContingentSyncResult::synced(false, 0, 0, 0, 0), ]); self::assertSame(Command::SUCCESS, $tester->getStatusCode()); @@ -48,7 +48,7 @@ class BpnSyncContingentsCommandTest extends TestCase public function testASuccessfulRunSucceeds(): void { - $tester = $this->runSync(['A'], [ContingentSyncResult::synced(true, 3, 1, 0)]); + $tester = $this->runSync(['A'], [ContingentSyncResult::synced(true, 3, 0, 1, 0)]); self::assertSame(Command::SUCCESS, $tester->getStatusCode()); } diff --git a/tests/Service/ContingentSnapshotManagerTest.php b/tests/Service/ContingentSnapshotManagerTest.php index 1bb1775..e9cdfd5 100644 --- a/tests/Service/ContingentSnapshotManagerTest.php +++ b/tests/Service/ContingentSnapshotManagerTest.php @@ -41,7 +41,6 @@ class ContingentSnapshotManagerTest extends TestCase $this->entityManager = $this->createMock(EntityManagerInterface::class); $this->dayRepository->method('findByAccommodationAndDateRange')->willReturn([]); - $this->dayRepository->method('findStatusFingerprintParts')->willReturn(['2026-07-01:OK', '2026-07-02:BLOCKED']); $this->client->method('getContingentCalendar')->willReturn($this->response([ new ContingentCalendarEntry('2026-07-01', ContingentStatus::Ok), new ContingentCalendarEntry('2026-07-02', ContingentStatus::Blocked), @@ -69,7 +68,6 @@ class ContingentSnapshotManagerTest extends TestCase ]; $this->dayRepository->method('findByAccommodationAndDateRange')->willReturn($existing); - $this->dayRepository->method('findStatusFingerprintParts')->willReturn(['2026-07-01:OK']); $this->client->method('getContingentCalendar')->willReturn($this->response([ new ContingentCalendarEntry('2026-07-01', ContingentStatus::Ok), new ContingentCalendarEntry('2026-07-02', ContingentStatus::OnRequest), @@ -85,16 +83,14 @@ class ContingentSnapshotManagerTest extends TestCase self::assertSame(ContingentStatus::OnRequest, $existing['2026-07-02']->getStatus()); } - public function testUnchangedFingerprintDoesNotMoveChangedAt(): void + public function testAnIdenticalUpstreamResponseDoesNotMoveChangedAt(): void { $state = new ContingentSyncState($this->accommodation()); - $state->setContentHash(hash('sha256', '2026-07-01:OK')); $this->syncStateRepository->method('findOneByAccommodation')->willReturn($state); $this->dayRepository->method('findByAccommodationAndDateRange')->willReturn([ '2026-07-01' => $this->day('2026-07-01', ContingentStatus::Ok), ]); - $this->dayRepository->method('findStatusFingerprintParts')->willReturn(['2026-07-01:OK']); $this->client->method('getContingentCalendar')->willReturn($this->response([ new ContingentCalendarEntry('2026-07-01', ContingentStatus::Ok), ])); @@ -148,7 +144,6 @@ class ContingentSnapshotManagerTest extends TestCase public function testDaysOutsideTheRequestedWindowAreIgnored(): void { $this->dayRepository->method('findByAccommodationAndDateRange')->willReturn([]); - $this->dayRepository->method('findStatusFingerprintParts')->willReturn([]); $this->client->method('getContingentCalendar')->willReturn($this->response([ new ContingentCalendarEntry('2026-06-30', ContingentStatus::Ok), new ContingentCalendarEntry('2026-07-01', ContingentStatus::Ok), @@ -167,7 +162,6 @@ class ContingentSnapshotManagerTest extends TestCase $this->syncStateRepository->method('findOneByAccommodation')->willReturn($state); $this->dayRepository->method('findByAccommodationAndDateRange')->willReturn([]); - $this->dayRepository->method('findStatusFingerprintParts')->willReturn([]); $this->client->method('getContingentCalendar')->willReturn($this->response([ new ContingentCalendarEntry('2026-07-01', ContingentStatus::Ok), ])); @@ -185,7 +179,6 @@ class ContingentSnapshotManagerTest extends TestCase $this->syncStateRepository->method('findOneByAccommodation')->willReturn($state); $this->dayRepository->method('findByAccommodationAndDateRange')->willReturn([]); - $this->dayRepository->method('findStatusFingerprintParts')->willReturn([]); $this->client->method('getContingentCalendar')->willReturn($this->response([ new ContingentCalendarEntry('2026-07-01', ContingentStatus::Ok), ])); @@ -195,6 +188,32 @@ class ContingentSnapshotManagerTest extends TestCase self::assertSame('2026-07-03', $state->getHorizonTo()?->format('Y-m-d')); } + public function testTheRollingWindowGrowingIsNotAChange(): void + { + $state = new ContingentSyncState($this->accommodation()); + $state->recordSuccess(new \DateTimeImmutable('2026-06-30'), new \DateTimeImmutable('2026-07-02')); + + $this->syncStateRepository->method('findOneByAccommodation')->willReturn($state); + $this->dayRepository->method('findByAccommodationAndDateRange')->willReturn([ + '2026-07-01' => $this->day('2026-07-01', ContingentStatus::Ok), + '2026-07-02' => $this->day('2026-07-02', ContingentStatus::Ok), + ]); + // Same two days as before plus the one the window just grew by, which is what every + // scheduled run sees after midnight. + $this->client->method('getContingentCalendar')->willReturn($this->response([ + new ContingentCalendarEntry('2026-07-01', ContingentStatus::Ok), + new ContingentCalendarEntry('2026-07-02', ContingentStatus::Ok), + new ContingentCalendarEntry('2026-07-03', ContingentStatus::Ok), + ])); + + $result = $this->sync(); + + self::assertFalse($result->changed); + self::assertSame(0, $result->added); + self::assertSame(1, $result->extended); + self::assertNull($state->getChangedAt()); + } + private function sync(): \App\Model\ContingentSyncResult { $manager = new ContingentSnapshotManager(