diff --git a/UPGRADE.md b/UPGRADE.md index 435ab653..4cd0aefb 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -75,6 +75,76 @@ Rector ne peut rien : réécrire un `$container->get('durable.event_store.dbal') demande de savoir où l'objet est utilisé, ce qu'aucune règle ne devine. Le tableau ci-dessus est la procédure. +### `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 ### La garde de divergence compare aussi la charge diff --git a/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php b/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php index 914f5620..1ff134ce 100644 --- a/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php +++ b/src/Bridge/Temporal/Worker/TemporalExecutionHistory.php @@ -53,10 +53,10 @@ final class TemporalExecutionHistory implements WorkflowHistorySourceInterface /** @var array activityId → activity name (to type the failures) */ private array $activityNames = []; - /** @var array> activityId → charge planifiée (garde DUR042) */ + /** @var array> activityId => scheduled payload (DUR042 guard) */ private array $activityPayloads = []; - /** @var array> slot → charge Nexus planifiée (garde DUR042) */ + /** @var array> slot => scheduled Nexus payload (DUR042 guard) */ private array $nexusOperationPayloads = []; /** @var array> slot → input du workflow enfant (garde DUR042) */ @@ -187,10 +187,10 @@ private function consumeEvent(HistoryEvent $event): void 'operation' => (string) $attr->getOperation(), ]; - // La charge de l'appelant, **nue** : une opération Nexus porte un - // `Payload` et non des `Payloads`, et l'enveloppe `{operationId, payload}` - // a été retirée du tampon (tâche 1.1). Décoder autre chose ici comparerait - // une forme que le fil ne porte plus. + // The caller's payload, **bare**: a Nexus operation carries a `Payload` + // and not `Payloads`, and the `{operationId, payload}` envelope was removed + // from the buffer (task 1.1). Decoding anything else here would compare a + // shape the wire no longer carries. $nexusInput = $attr->getInput(); if (null !== $nexusInput) { $decodedInput = JsonPlainPayload::decode($nexusInput); @@ -262,9 +262,9 @@ private function consumeEvent(HistoryEvent $event): void $this->activityNames[$activityId] = (string) ($attr->getActivityType()?->getName() ?? ''); $this->scheduledEventIdToActivityId[$eventId] = $activityId; - // L'entrée porte l'enveloppe écrite par TemporalActivityScheduleInput ; sa case - // `payload` tient les arguments. Absente ou illisible, on n'enregistre rien : - // la garde n'a alors rien à comparer, ce qui est son cas de repos. + // The input carries the envelope written by TemporalActivityScheduleInput; its + // `payload` slot holds the arguments. Missing or unreadable, nothing is recorded: + // the guard then has nothing to compare, which is its resting case. $input = $attr->getInput(); if (null !== $input) { $payloads = $input->getPayloads(); @@ -468,9 +468,9 @@ private function consumeEvent(HistoryEvent $event): void // the execution id being generated. $this->childWorkflowTypes[] = (string) ($attr->getWorkflowType()?->getName() ?? ''); - // `singlePayloads(encode($input))` côté tampon : une liste d'un élément, dont - // le premier est l'input nu. Une forme de plus que Nexus (Payload nu) et que - // l'activité (enveloppe) — les trois se ressemblent et ne se valent pas. + // `singlePayloads(encode($input))` on the buffer side: a one-element list whose + // first item is the bare input. One shape more than Nexus (a bare Payload) and + // than the activity (an envelope); the three look alike and are not equal. $childInput = $attr->getInput(); if (null !== $childInput) { $childPayloads = $childInput->getPayloads(); @@ -630,6 +630,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 803864cd..55f66d8f 100644 --- a/src/Durable/ExecutionContext.php +++ b/src/Durable/ExecutionContext.php @@ -256,17 +256,20 @@ public function version(string $changeId, int $minSupported, int $maxSupported): * Deduced, therefore deterministic: two replays of the same history answer alike, which is * the only property versioning needs. * - * Side effects are not consulted: `findSideEffectForSlot()` returns `mixed`, and a recorded - * value can legitimately be `null` — "nothing here" cannot be told apart from "here, the - * value null". A workflow whose only work before a change point is a side effect will - * therefore be treated as new. That is the hole, it is narrow, and it is written down. + * Side effects count like the rest, now that the port can state their presence without going + * through their value. They could not while `findSideEffectForSlot()` returned `mixed`: a + * recorded value can legitimately be `null`, and "nothing here" was indistinguishable from + * "here, the value null". A workflow whose only work before a change point was a side effect + * then switched to the new branch mid-replay. The hole is closed along with `sideEffect()`'s, + * of which it was the same 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); } /** @@ -476,10 +479,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); + + // The slot's presence, never the value it carries: a closure returning `null` did run, and + // reading it back is exactly what `sideEffect()` promises. + 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 8b1ebbba..59cf238e 100644 --- a/src/Durable/Port/WorkflowHistorySourceInterface.php +++ b/src/Durable/Port/WorkflowHistorySourceInterface.php @@ -97,7 +97,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 8696d1b5..938c24c3 100644 --- a/src/Durable/Store/EventStoreHistorySource.php +++ b/src/Durable/Store/EventStoreHistorySource.php @@ -133,8 +133,8 @@ public function activityPayloadForSlot(int $slot): ?array foreach ($this->eventStore->readStream($this->executionId) as $event) { if ($event instanceof ActivityScheduled) { if ($index === $slot) { - // `payload()` rend l'enveloppe de l'événement ; les arguments de l'activité en - // sont une case. Un non-tableau vaut « rien à comparer », pas « tableau vide ». + // `payload()` returns the event's envelope; the activity's arguments are one + // slot of it. A non-array means "nothing to compare", not "empty array". $arguments = $event->payload()['payload'] ?? null; return \is_array($arguments) ? $arguments : null; @@ -166,12 +166,12 @@ public function childWorkflowInputForSlot(int $slot): ?array /** * Toujours null, et ce n'est pas un oubli. * - * Ce backend refuse les opérations Nexus par construction (DUR036) : aucun de ses historiques - * n'en porte une que le workflow aurait planifiée. Le seul `NexusOperationScheduled` qui puisse - * traverser un flux vient du convertisseur du profileur, qui l'écrit pour l'affichage — et cet - * événement ne porte que le site d'appel, jamais la charge. Il n'y a donc rien à comparer. + * This backend refuses Nexus operations by design (DUR036): none of its histories carries one + * the workflow would have scheduled. The only `NexusOperationScheduled` that can cross a stream + * comes from the profiler's converter, which writes it for display, and that event carries only + * the call site, never the payload. So there is nothing to compare. * - * La garde s'exerce là où Nexus existe : {@see \Gplanchat\Bridge\Temporal\Worker\TemporalExecutionHistory}. + * The guard applies where Nexus exists: {@see \Gplanchat\Bridge\Temporal\Worker\TemporalExecutionHistory}. */ public function nexusOperationPayloadForSlot(int $slot): ?array { @@ -271,6 +271,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', + ); + } +}