diff --git a/UPGRADE.md b/UPGRADE.md index 074f7922..10b1cd99 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -22,6 +22,78 @@ vendor/bin/rector process src Le set est **cumulatif** : le passer une fois rattrape toutes les versions franchies d'un coup. Il ne contient que ce que Rector sait faire sans deviner ; tout le reste est écrit à la main ci-dessous. +## Non publié + +### `WorkflowHistorySourceInterface` gagne `hasSideEffectForSlot()` + +**Qui est concerné** : uniquement qui **implémente** `WorkflowHistorySourceInterface` — c'est-à-dire +qui écrit un backend. Une application qui appelle `sideEffect()` n'a rien à changer ; elle gagne le +correctif sans rien faire. + +**Ce qui était cassé.** `findSideEffectForSlot()` rend `mixed` et signalait « rien d'enregistré » par +`null`. Une closure qui rend légitimement `null` était donc indistinguable d'un slot vide : elle +était **ré-exécutée à chaque passe de rejeu**, et le journal grossissait d'un `SideEffectRecorded` +par passe. C'est la garantie même que `sideEffect()` existe pour offrir. Les valeurs `false`, `0`, +`''` et `[]` n'étaient pas touchées — la comparaison était un `!==` strict. + +**Ce qu'il faut écrire.** Une méthode qui répond *le slot existe-t-il*, sans regarder ce qu'il porte. +Rector ne peut rien ici : la réponse dépend de la façon dont votre backend range ses slots, et lui +en faire deviner une produirait un adaptateur qui compile et ment. Les deux implémentations livrées +donnent les deux formes attendues. + +Sur un journal parcouru : + +```php +public function hasSideEffectForSlot(int $slot): bool +{ + $index = 0; + foreach ($this->eventStore->readStream($this->executionId) as $event) { + if ($event instanceof SideEffectRecorded) { + if ($index === $slot) { + return true; + } + ++$index; + } + } + + return false; +} +``` + +Sur un tableau indexé par slot — et c'est `array_key_exists()`, jamais `isset()`, qui rouvrirait +exactement le trou que ce correctif ferme : + +```php +public function hasSideEffectForSlot(int $slot): bool +{ + return \array_key_exists($slot, $this->sideEffects); +} +``` + +`findSideEffectForSlot()` ne change pas de signature et garde son comportement : elle rend la valeur, +et rend `null` aussi bien pour un slot absent que pour un slot portant `null`. C'est désormais écrit +dans son contrat, et c'est `hasSideEffectForSlot()` qui décide s'il faut exécuter la closure. + + +### `version()` cesse de basculer une exécution en vol + +**Qui est concerné** : toute application qui appelle `version()`. Rien à écrire ; le comportement +change, en mieux, et il faut savoir en quoi. + +`version()` décide de rendre l'ancien comportement quand l'exécution est encore en train de +rejouer. Ce signal se déduisait des quatre types de slot qui savent dire leur présence — activité, +minuteur, workflow enfant, opération Nexus — et laissait les effets de bord de côté, pour la raison +même que le correctif ci-dessus vient de lever : leur présence ne se lisait pas sans lire leur +valeur. + +Conséquence : une exécution dont le travail restant devant elle n'était fait que d'effets de bord +était vue comme arrivée au bout de son historique. Elle prenait la branche **neuve** au milieu d'un +rejeu et y écrivait son marqueur de version — dans une histoire écrite avant que le point de +changement existe. `hasSideEffectForSlot()` étant désormais au port, ce cas rejoint les autres. + +Une exécution qui a déjà écrit un marqueur de version garde le sien : `versionForChangeId()` est +consulté en premier, et rien de ce commit ne le touche. + ## 0.1.0-alpha8 ### Laravel refuse au démarrage un workflow dont les noms de paramètres divergent du contrat diff --git a/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php b/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php index 44fc1390..492dcb14 100644 --- a/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php +++ b/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php @@ -562,6 +562,11 @@ public function findScheduledTimerId(int $slot): ?string return $this->scheduledTimerIds[$slot] ?? null; } + public function hasSideEffectForSlot(int $slot): bool + { + return \array_key_exists($slot, $this->sideEffects); + } + public function findSideEffectForSlot(int $slot): mixed { return $this->sideEffects[$slot] ?? null; diff --git a/src/Durable/ExecutionContext.php b/src/Durable/ExecutionContext.php index 25cfadd0..4255a7d2 100644 --- a/src/Durable/ExecutionContext.php +++ b/src/Durable/ExecutionContext.php @@ -239,17 +239,20 @@ public function version(string $changeId, int $minSupported, int $maxSupported): * Déduit, donc déterministe : deux replays de la même histoire répondent pareil, ce qui est la * seule propriété dont le versioning a besoin. * - * Les effets de bord ne sont pas consultés : `findSideEffectForSlot()` rend `mixed`, et une - * valeur enregistrée peut légitimement être `null` — on ne peut pas distinguer « rien ici » de - * « ici, la valeur null ». Un workflow dont le seul travail avant un point de changement est - * un effet de bord sera donc traité comme neuf. C'est le trou, il est étroit, et il est écrit. + * Les effets de bord comptent comme les autres depuis que le port sait dire leur présence + * sans passer par leur valeur. Ils ne le pouvaient pas tant que `findSideEffectForSlot()` + * rendait `mixed` : une valeur enregistrée peut légitimement être `null`, et « rien ici » ne + * s'y distinguait pas de « ici, la valeur null ». Un workflow dont le seul travail avant un + * point de changement était un effet de bord basculait alors sur la branche neuve, en plein + * rejeu — le trou est fermé avec celui de `sideEffect()`, dont il était la même cause. */ private function hasRecordedWorkAhead(): bool { return null !== $this->historySource->findScheduledActivityId($this->activitySlotIndex) || null !== $this->historySource->findScheduledTimerId($this->timerSlotIndex) || null !== $this->historySource->findScheduledChildExecutionId($this->childWorkflowSlotIndex) - || null !== $this->historySource->findScheduledNexusOperation($this->nexusOperationSlotIndex); + || null !== $this->historySource->findScheduledNexusOperation($this->nexusOperationSlotIndex) + || $this->historySource->hasSideEffectForSlot($this->sideEffectSlotIndex); } /** @@ -311,10 +314,12 @@ private function refuseDivergence(string $slotKind, int $slotIndex, ?string $rec public function sideEffect(\Closure $closure): Awaitable { $slotIndex = $this->sideEffectSlotIndex++; - $replayResult = $this->historySource->findSideEffectForSlot($slotIndex); $deferred = new \Gplanchat\Durable\Awaitable\Deferred(); - if (null !== $replayResult) { - $deferred->resolve($replayResult); + + // La présence du slot, jamais la valeur qu'il porte : une closure qui rend `null` a bel et + // bien été exécutée, et la relire est exactement ce que `sideEffect()` promet. + if ($this->historySource->hasSideEffectForSlot($slotIndex)) { + $deferred->resolve($this->historySource->findSideEffectForSlot($slotIndex)); return $deferred->awaitable(); } diff --git a/src/Durable/Port/WorkflowHistorySourceInterface.php b/src/Durable/Port/WorkflowHistorySourceInterface.php index 5f1d292d..192faf79 100644 --- a/src/Durable/Port/WorkflowHistorySourceInterface.php +++ b/src/Durable/Port/WorkflowHistorySourceInterface.php @@ -81,7 +81,25 @@ public function findTimerSlotResult(int $slot): ?array; public function findScheduledTimerId(int $slot): ?string; /** - * Returns the recorded side effect result at slot N, or null if not yet recorded. + * Whether slot N holds a recorded side effect. + * + * Presence is a fact about the history; the recorded value is data. They must be asked + * separately, because a side effect legitimately records `null` — and inferring "not recorded" + * from a `null` result re-runs a non-deterministic closure on every replay and appends a + * `SideEffectRecorded` per pass, which is the one guarantee `sideEffect()` exists to give. + * + * The same separation already exists on this port for timers, where + * {@see findScheduledTimerId()} answers the state and {@see findTimerSlotResult()} the value, + * and for activities, child workflows and Nexus operations, whose three sibling methods wrap + * their result in an `array{result: mixed, ...}` for exactly this reason. + */ + public function hasSideEffectForSlot(int $slot): bool; + + /** + * Returns the recorded side effect result at slot N. + * + * Returns `null` both for a slot that recorded `null` and for a slot that recorded nothing; + * callers deciding whether to run a closure MUST ask {@see hasSideEffectForSlot()} first. */ public function findSideEffectForSlot(int $slot): mixed; diff --git a/src/Durable/Store/EventStoreHistorySource.php b/src/Durable/Store/EventStoreHistorySource.php index b13c566c..5aefc249 100644 --- a/src/Durable/Store/EventStoreHistorySource.php +++ b/src/Durable/Store/EventStoreHistorySource.php @@ -220,6 +220,21 @@ public function findScheduledTimerId(int $slot): ?string return null; } + public function hasSideEffectForSlot(int $slot): bool + { + $index = 0; + foreach ($this->eventStore->readStream($this->executionId) as $event) { + if ($event instanceof SideEffectRecorded) { + if ($index === $slot) { + return true; + } + ++$index; + } + } + + return false; + } + public function findSideEffectForSlot(int $slot): mixed { $index = 0; diff --git a/tests/unit/Durable/SideEffectSlotPresenceTest.php b/tests/unit/Durable/SideEffectSlotPresenceTest.php new file mode 100644 index 00000000..5c523079 --- /dev/null +++ b/tests/unit/Durable/SideEffectSlotPresenceTest.php @@ -0,0 +1,167 @@ + + */ + public static function valeursQuiSeConfondentAvecLAbsence(): iterable + { + yield 'null' => [null]; + yield 'false' => [false]; + yield 'zéro' => [0]; + yield 'chaîne vide' => ['']; + yield 'liste vide' => [[]]; + } + + #[DataProvider('valeursQuiSeConfondentAvecLAbsence')] + public function testUneClosureQuiRendUneValeurFausseNeTourneQuUneFois(mixed $valeur): void + { + $store = new InMemoryEventStore(); + $appels = 0; + + $workflow = static function (WorkflowEnvironment $wf) use ($valeur, &$appels): mixed { + return $wf->sideEffect(static function () use ($valeur, &$appels): mixed { + ++$appels; + + return $valeur; + }); + }; + + self::executer($store, 'exec-1', $workflow); + self::assertSame(1, $appels, 'la première passe exécute la closure'); + + self::executer($store, 'exec-1', $workflow); + self::assertSame(1, $appels, 'la passe de rejeu doit relire le résultat, pas le recalculer'); + } + + #[DataProvider('valeursQuiSeConfondentAvecLAbsence')] + public function testLeJournalNeGrossitPasDUnEvenementParPasse(mixed $valeur): void + { + $store = new InMemoryEventStore(); + + $workflow = static fn(WorkflowEnvironment $wf): mixed => $wf->sideEffect(static fn(): mixed => $valeur); + + self::executer($store, 'exec-1', $workflow); + self::executer($store, 'exec-1', $workflow); + self::executer($store, 'exec-1', $workflow); + + self::assertSame( + 1, + self::compterEffetsDeBord($store, 'exec-1'), + 'trois passes sur une seule instruction sideEffect() doivent laisser un seul événement', + ); + } + + #[DataProvider('valeursQuiSeConfondentAvecLAbsence')] + public function testLaValeurRelueEstLaValeurEnregistree(mixed $valeur): void + { + $store = new InMemoryEventStore(); + + $workflow = static fn(WorkflowEnvironment $wf): mixed => $wf->sideEffect(static fn(): mixed => $valeur); + + self::assertSame($valeur, self::executer($store, 'exec-1', $workflow)); + self::assertSame($valeur, self::executer($store, 'exec-1', $workflow), 'le rejeu rend la même valeur'); + } + + /** + * Les slots restent alignés : un effet de bord « faux » ne doit pas décaler celui d'après. + */ + public function testUnEffetDeBordFauxNeDecalePasLeSlotSuivant(): void + { + $store = new InMemoryEventStore(); + + $workflow = static fn(WorkflowEnvironment $wf): array => [ + 'premier' => $wf->sideEffect(static fn(): mixed => null), + 'second' => $wf->sideEffect(static fn(): string => 'après'), + ]; + + self::assertSame(['premier' => null, 'second' => 'après'], self::executer($store, 'exec-1', $workflow)); + self::assertSame(['premier' => null, 'second' => 'après'], self::executer($store, 'exec-1', $workflow)); + self::assertSame(2, self::compterEffetsDeBord($store, 'exec-1')); + } + + private static function executer(InMemoryEventStore $store, string $executionId, \Closure $workflow): mixed + { + $runner = new InMemoryWorkflowRunner( + $store, + new InMemoryActivityTransport(), + new RegistryActivityExecutor(), + 0, + new WorkflowRegistry(), + ); + + return $runner->run($executionId, $workflow); + } + + private static function compterEffetsDeBord(InMemoryEventStore $store, string $executionId): int + { + $total = 0; + foreach ($store->readStream($executionId) as $event) { + if ($event instanceof SideEffectRecorded) { + ++$total; + } + } + + return $total; + } + + /** + * L'appelant frère. `version()` demande « suis-je en train de rejouer ? » à + * `hasRecordedWorkAhead()`, qui interrogeait les quatre autres types de slot et pas les effets + * de bord, faute de pouvoir en lire la présence sans en lire la valeur. + * + * Une exécution dont le travail restant devant elle n'est fait que d'effets de bord était donc + * vue comme arrivée au bout de son historique. Elle prenait la branche neuve **en plein + * rejeu**, et écrivait son marqueur de version au milieu d'une histoire écrite avant que le + * point de changement existe — ce que `version()` est précisément là pour empêcher. + */ + public function testUnTravailRestantFaitDEffetsDeBordRetientLAncienneVersion(): void + { + $store = new InMemoryEventStore(); + + // Le code d'avant : deux effets de bord, aucun point de changement. + $avant = static fn(WorkflowEnvironment $wf): array => [ + 'premier' => $wf->sideEffect(static fn(): mixed => null), + 'second' => $wf->sideEffect(static fn(): string => 'après'), + ]; + self::executer($store, 'exec-1', $avant); + + // Le code d'après, sur la même exécution : un point de changement s'est glissé entre les + // deux effets de bord, et le second est encore devant. + $apres = static fn(WorkflowEnvironment $wf): array => [ + 'premier' => $wf->sideEffect(static fn(): mixed => null), + 'version' => $wf->version('changement-1', 1, 3), + 'second' => $wf->sideEffect(static fn(): string => 'après'), + ]; + + self::assertSame( + ChangePoint::DEFAULT_VERSION, + self::executer($store, 'exec-1', $apres)['version'], + 'une exécution en vol garde l\'ancien comportement tant qu\'il lui reste du journal à rejouer', + ); + } +}