diff --git a/documentation/user/configuration/_index.fr.md b/documentation/user/configuration/_index.fr.md index 6a792986..5c8d579e 100644 --- a/documentation/user/configuration/_index.fr.md +++ b/documentation/user/configuration/_index.fr.md @@ -30,6 +30,8 @@ durable: transport_name: durable_activities table_name: durable_activity_outbox max_activity_retries: 0 # réessais automatiques maximum avant de marquer une activité en échec + messenger: + buses: [] # [] = tous les bus (défaut) activity_contracts: cache: cache.app # pool de cache PSR-6 pour les métadonnées de contrat (défaut : null, pas de cache) contracts: @@ -146,6 +148,30 @@ sémantique de réessai que le transport apporte. --- +## `messenger` + +```yaml +durable: + messenger: + buses: + - messenger.bus.durable +``` + +Les bus Messenger sur lesquels le bundle installe ses middlewares — le verrou de reprise DBAL, et +le middleware de profil en debug. + +**Le défaut est tous les bus**, ce que les versions précédentes faisaient sans condition. Ce défaut +ne peut pas être plus fin : le bundle ne sait pas vers quel bus votre application route +`ResumeWorkflowMessage`, et deviner retirerait le verrou de reprise du bus qui porte réellement le +travail — une perte silencieuse de la garantie pour laquelle ce verrou existe. + +Nommer les bus vaut la peine dès que vous en avez plusieurs. Un bus de commandes métier ne +transporte aucun message durable, et y prendre un verrou par exécution est une contention que +personne n'a demandée. Un identifiant qui ne nomme aucun bus déclaré est refusé à la compilation, +plutôt que de ne rien faire en silence. + +--- + ## `max_activity_retries` ```yaml diff --git a/documentation/user/configuration/_index.md b/documentation/user/configuration/_index.md index 827acbf4..58da0888 100644 --- a/documentation/user/configuration/_index.md +++ b/documentation/user/configuration/_index.md @@ -30,6 +30,8 @@ durable: transport_name: durable_activities table_name: durable_activity_outbox max_activity_retries: 0 # maximum automatic retries before marking an activity as failed + messenger: + buses: [] # [] = every bus (default) activity_contracts: cache: cache.app # PSR-6 cache pool for contract metadata (default: null, no cache) contracts: @@ -141,6 +143,29 @@ semantics the transport provides. --- +## `messenger` + +```yaml +durable: + messenger: + buses: + - messenger.bus.durable +``` + +Which Messenger buses the bundle installs its middlewares on — the DBAL resume lock, and the +profiler middleware in debug. + +**The default is every bus**, which is what earlier versions did unconditionally. That default +cannot be narrower: the bundle does not know which bus your application routes +`ResumeWorkflowMessage` to, and guessing would take the resume lock off the bus that carries the +work — a silent loss of the guarantee the lock exists to give. + +Naming buses is worth doing once you have more than one. A business command bus carries no durable +message, and taking a per-execution lock on it is contention nobody asked for. An id that names no +declared bus is refused at compile time rather than silently doing nothing. + +--- + ## `max_activity_retries` ```yaml diff --git a/src/DurableBundle/DependencyInjection/Compiler/RegisterDurableMiddlewarePass.php b/src/DurableBundle/DependencyInjection/Compiler/RegisterDurableMiddlewarePass.php index d376676b..34e0f105 100644 --- a/src/DurableBundle/DependencyInjection/Compiler/RegisterDurableMiddlewarePass.php +++ b/src/DurableBundle/DependencyInjection/Compiler/RegisterDurableMiddlewarePass.php @@ -19,6 +19,11 @@ * D'où une balise qui appartient au bundle, `durable.messenger.middleware`, et cette passe pour la * consommer. Le prochain middleware du bundle s'installe en la posant, sans y penser. * + * **Sur quels bus.** Tous par défaut, et `durable.messenger.buses` permet de nommer les seuls qui + * portent des messages durables. Le défaut ne peut pas être plus fin : le bundle ne sait pas quel + * bus l'application a choisi pour router `ResumeWorkflowMessage`, et deviner retirerait le verrou + * là où il fait son travail. + * * L'ordre vient de l'attribut `priority`, décroissant : ce qui compte est que deux middlewares ne * dépendent pas de l'ordre d'itération du conteneur. Ils entrent en **tête** parce qu'un verrou * doit envelopper tout ce qui suit, y compris un `doctrine_transaction` — le relâcher avant le @@ -27,6 +32,7 @@ final class RegisterDurableMiddlewarePass implements CompilerPassInterface { public const TAG = 'durable.messenger.middleware'; + public const BUSES_PARAMETER = 'durable.messenger.buses'; public function process(ContainerBuilder $container): void { @@ -35,7 +41,7 @@ public function process(ContainerBuilder $container): void return; } - foreach (array_keys($container->findTaggedServiceIds('messenger.bus')) as $busId) { + foreach ($this->busesToServe($container) as $busId) { $param = $busId . '.middleware'; if (!$container->hasParameter($param)) { continue; @@ -57,6 +63,44 @@ public function process(ContainerBuilder $container): void } } + /** + * Les bus où installer, et rien qu'eux. + * + * Le défaut reste **tous les bus** : c'est le comportement historique, et le restreindre de + * notre propre chef retirerait le verrou de reprise du bus qui porte réellement les messages + * durables chez quelqu'un — une perte de durabilité silencieuse, exactement ce contre quoi le + * verrou existe. + * + * @return list + */ + private function busesToServe(ContainerBuilder $container): array + { + $declared = array_keys($container->findTaggedServiceIds('messenger.bus')); + + $chosen = $container->hasParameter(self::BUSES_PARAMETER) + ? $container->getParameter(self::BUSES_PARAMETER) + : []; + + if (!\is_array($chosen) || [] === $chosen) { + return $declared; + } + + // Un bus nommé qui n'existe pas est une faute de frappe, et la laisser passer produirait + // le silence qu'on cherche à supprimer : la configuration a l'air posée, rien ne s'installe. + $unknown = array_diff($chosen, $declared); + if ([] !== $unknown) { + throw new \LogicException(\sprintf( + 'durable.messenger.buses nomme %s, qui n\'est pas un bus Messenger de cette application. ' + . 'Bus déclarés : %s. Un identifiant de bus est un identifiant de service — ' + . '"messenger.bus.default" pour le bus par défaut de FrameworkBundle.', + implode(', ', array_map(static fn(string $id): string => '"' . $id . '"', $unknown)), + [] === $declared ? 'aucun' : implode(', ', $declared), + )); + } + + return array_values(array_intersect($declared, $chosen)); + } + /** * @return list */ diff --git a/src/DurableBundle/DependencyInjection/Configuration.php b/src/DurableBundle/DependencyInjection/Configuration.php index 58ed390b..29fffa88 100644 --- a/src/DurableBundle/DependencyInjection/Configuration.php +++ b/src/DurableBundle/DependencyInjection/Configuration.php @@ -51,6 +51,16 @@ public function getConfigTreeBuilder(): TreeBuilder ->scalarNode('transport_name')->defaultValue('durable_activities')->end() ->end() ->end() + ->arrayNode('messenger') + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('buses') + ->scalarPrototype()->end() + ->defaultValue([]) + ->info("Identifiants des bus Messenger où installer les middlewares du bundle (verrou de reprise, profil). Vide — le défaut — les installe sur tous les bus, ce qui est le comportement historique. Nommer des bus évite d'imposer un verrou par exécution au bus de commandes métier, qui ne transporte aucun message durable.") + ->end() + ->end() + ->end() ->integerNode('max_activity_retries')->defaultValue(0)->end() ->arrayNode('activity_contracts') ->addDefaultsIfNotSet() diff --git a/src/DurableBundle/DependencyInjection/DurableExtension.php b/src/DurableBundle/DependencyInjection/DurableExtension.php index f7aba215..a6cad655 100644 --- a/src/DurableBundle/DependencyInjection/DurableExtension.php +++ b/src/DurableBundle/DependencyInjection/DurableExtension.php @@ -102,6 +102,13 @@ public function load(array $configs, ContainerBuilder $container): void $this->registerRuntime($container, $config); $this->registerWorkflowMessengerServices($container, $config); $this->registerParentChildCoordinator($container); + // La passe qui installe les middlewares tourne bien après les extensions ; elle relit ce + // choix ici plutôt que de le redécouvrir. + $container->setParameter( + RegisterDurableMiddlewarePass::BUSES_PARAMETER, + $config['messenger']['buses'] ?? [], + ); + $this->registerActivityContractResolver($container, $config); $this->registerEngine($container, $config); $this->registerActivityContractCacheWarmer($container, $config); diff --git a/tests/unit/DurableBundle/DependencyInjection/Compiler/DurableMiddlewareBusScopeTest.php b/tests/unit/DurableBundle/DependencyInjection/Compiler/DurableMiddlewareBusScopeTest.php new file mode 100644 index 00000000..190f550e --- /dev/null +++ b/tests/unit/DurableBundle/DependencyInjection/Compiler/DurableMiddlewareBusScopeTest.php @@ -0,0 +1,103 @@ +compile([], ['messenger.bus.commandes', 'messenger.bus.durable']); + + self::assertContains('durable.dbal.single_resume_lock', $this->stackOf($container, 'messenger.bus.commandes')); + self::assertContains('durable.dbal.single_resume_lock', $this->stackOf($container, 'messenger.bus.durable')); + } + + public function testUneListeRestreintLesBusServis(): void + { + $container = $this->compile( + ['messenger' => ['buses' => ['messenger.bus.durable']]], + ['messenger.bus.commandes', 'messenger.bus.durable'], + ); + + self::assertNotContains( + 'durable.dbal.single_resume_lock', + $this->stackOf($container, 'messenger.bus.commandes'), + "le bus de commandes métier ne transporte aucun message durable : rien n'a à s'y installer", + ); + self::assertContains( + 'durable.dbal.single_resume_lock', + $this->stackOf($container, 'messenger.bus.durable'), + ); + } + + /** + * Une liste qui nomme un bus inexistant ne doit pas produire un silence : c'est exactement la + * faute qu'on croit avoir corrigée alors que rien ne s'est installé. + */ + public function testUnBusNommeQuiNExistePasEstRefuse(): void + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessageMatches('/messenger\.bus\.faute_de_frappe/'); + + $this->compile( + ['messenger' => ['buses' => ['messenger.bus.faute_de_frappe']]], + ['messenger.bus.durable'], + ); + } + + /** + * @param array $config + * @param list $buses + */ + private function compile(array $config, array $buses): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.debug', true); + (new DurableExtension())->load([$config + ['event_store' => ['type' => 'dbal']]], $container); + + // Ce que FrameworkExtension pose : un bus déclaré, et sa pile dans un paramètre. + foreach ($buses as $busId) { + $container->register($busId)->addTag('messenger.bus'); + $container->setParameter($busId . '.middleware', [['id' => 'send_message']]); + } + // Et ce que `framework.lock` pose, dont le backend DBAL a besoin. + $container->register('lock.factory', \stdClass::class); + + (new DurableBundle())->build($container); + foreach ($container->getCompilerPassConfig()->getBeforeOptimizationPasses() as $pass) { + $pass->process($container); + } + + return $container; + } + + /** + * @return list + */ + private function stackOf(ContainerBuilder $container, string $busId): array + { + /** @var list $stack */ + $stack = $container->getParameter($busId . '.middleware'); + + return array_values(array_filter(array_column($stack, 'id'))); + } +}