diff --git a/documentation/user/configuration/_index.fr.md b/documentation/user/configuration/_index.fr.md index a022eb54..2dd01946 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 366a26a9..c3961beb 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 a0e8c04a..7e748ad8 100644 --- a/src/DurableBundle/DependencyInjection/Compiler/RegisterDurableMiddlewarePass.php +++ b/src/DurableBundle/DependencyInjection/Compiler/RegisterDurableMiddlewarePass.php @@ -19,6 +19,11 @@ * Hence a tag that belongs to the bundle, `durable.messenger.middleware`, and this pass to consume * it. The next middleware of the bundle installs itself by adding it, without a thought. * + * **Which buses.** All of them by default, and `durable.messenger.buses` names the ones that + * actually carry durable messages. The default cannot be finer: the bundle does not know which bus + * the application chose to route `ResumeWorkflowMessage` on, and guessing would take the lock off + * where it does its work. + * * The order comes from the `priority` attribute, descending: what matters is that two middleware * do not depend on the container's iteration order. They go in at the **head** because a lock must * wrap everything that follows, including a `doctrine_transaction` — releasing it before the @@ -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,43 @@ public function process(ContainerBuilder $container): void } } + /** + * The buses to install on, and none besides. + * + * The default stays **every bus**: it is the historical behaviour, and narrowing it on our own + * initiative would take the resume lock off the bus that really carries somebody's durable + * messages: a silent loss of durability, which is exactly what the lock exists against. + * + * @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; + } + + // A named bus that does not exist is a typo, and letting it through would produce the very + // silence this is meant to remove: the configuration looks set, and nothing installs. + $unknown = array_diff($chosen, $declared); + if ([] !== $unknown) { + throw new \LogicException(\sprintf( + 'durable.messenger.buses names %s, which is not a Messenger bus of this application. ' + . 'Declared buses: %s. A bus id is a service id, ' + . '"messenger.bus.default" for FrameworkBundle\'s default bus.', + implode(', ', array_map(static fn(string $id): string => '"' . $id . '"', $unknown)), + [] === $declared ? 'none' : 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 8ff172c7..fb1b516c 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('Ids of the Messenger buses the bundle installs its middleware on (resume lock, profiler). Empty, which is the default, installs them on every bus, and that is the historical behaviour. Naming buses avoids imposing a per-execution lock on the business command bus, which carries no durable message.') + ->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 5ad97ada..6d479f99 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); + // The pass that installs the middleware runs well after the extensions; it reads this + // choice back here rather than rediscovering it. + $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'))); + } +}