diff --git a/UPGRADE.md b/UPGRADE.md index 074f7922..1d420f63 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -22,6 +22,32 @@ 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é + +### Onze services internes du bundle passent en privé + +**Qui est concerné** : une application qui tire l'un de ces onze identifiants du conteneur par +`$container->get()`. Pas celle qui les reçoit par autowiring, ni celle qui passe par leur interface. + +Les implémentations concrètes derrière un alias et les décorateurs de projection n'ont pas à être +des points d'entrée du conteneur : un service public échappe à l'*inlining* et à la suppression des +définitions inutilisées, et devient une promesse de compatibilité que personne n'a voulu prendre. + +| Devenu privé | À demander à la place | +| --- | --- | +| `durable.event_store.dbal`, `durable.event_store.temporal`, `durable.event_store.inner`, `durable.event_store.*.projecting` | `Gplanchat\Durable\Store\EventStoreInterface` | +| `durable.workflow_metadata_store.inner`, `durable.workflow_metadata_store.*.projecting` | `Gplanchat\Durable\Store\WorkflowMetadataStore` | +| `durable.run_catalog.dbal`, `durable.run_catalog.in_memory`, `durable.run_catalog.temporal` | `Gplanchat\Durable\Port\WorkflowRunCatalogInterface` | + +Les trois interfaces restent **publiques** et autowirables, et elles pointent la même instance : ce +qui change est le chemin pour y arriver, pas ce qu'on obtient. Le reste de la surface publique du +bundle est inchangé — les workers Temporal, le magasin de liens parent/enfant, le collecteur de +profil et les classes du moteur restent joignables par leur identifiant. + +Rector ne peut rien : réécrire un `$container->get('durable.event_store.dbal')` en une injection +demande de savoir où l'objet est utilisé, ce qu'aucune règle ne devine. Le tableau ci-dessus est la +procédure. + ## 0.1.0-alpha8 ### Laravel refuse au démarrage un workflow dont les noms de paramètres divergent du contrat diff --git a/src/Durable/RegistryActivityExecutor.php b/src/Durable/RegistryActivityExecutor.php index 8a6babba..194f0619 100644 --- a/src/Durable/RegistryActivityExecutor.php +++ b/src/Durable/RegistryActivityExecutor.php @@ -4,11 +4,24 @@ namespace Gplanchat\Durable; +use Psr\Container\ContainerInterface; + final class RegistryActivityExecutor implements ActivityExecutor { /** @var array): mixed> */ private array $handlers = []; + /** + * @param ContainerInterface|null $lazyHandlers gestionnaires indexés par nom d'activité, résolus + * à l'appel et non à la construction + */ + public function __construct( + private readonly ?ContainerInterface $lazyHandlers = null, + ) {} + + /** + * Enregistrement direct, pour les hôtes qui n'ont pas de conteneur de services à offrir. + */ public function register(string $activityName, callable $handler): void { $this->handlers[$activityName] = $handler; @@ -17,6 +30,13 @@ public function register(string $activityName, callable $handler): void public function execute(string $activityName, array $payload): mixed { $handler = $this->handlers[$activityName] ?? null; + + // Le localisateur en dernier : un enregistrement direct l'emporte, ce qui laisse un test + // remplacer un gestionnaire sans reconstruire le conteneur. + if (null === $handler && $this->lazyHandlers?->has($activityName)) { + $handler = $this->lazyHandlers->get($activityName); + } + if (null === $handler) { throw new \RuntimeException(\sprintf('No handler registered for activity "%s"', $activityName)); } diff --git a/src/DurableBundle/DependencyInjection/Compiler/ActivityHandlerPass.php b/src/DurableBundle/DependencyInjection/Compiler/ActivityHandlerPass.php index 75acfce0..57282fb3 100644 --- a/src/DurableBundle/DependencyInjection/Compiler/ActivityHandlerPass.php +++ b/src/DurableBundle/DependencyInjection/Compiler/ActivityHandlerPass.php @@ -8,11 +8,18 @@ use Gplanchat\Durable\Activity\PayloadToContractMethodInvoker; use Gplanchat\Durable\ActivityExecutor; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; /** * Enregistre sur {@see ActivityExecutor} les activités exposées par les services tagués durable.activity_handler. + * + * Par un **localisateur de services**, et non par un tableau de callables. Poser + * `[new Reference($invoker), '__invoke']` obligerait le conteneur à résoudre chaque référence pour + * bâtir l'argument : il instancierait alors tous les gestionnaires de l'application — et leurs + * connexions, clients HTTP et autres dépendances — pour en appeler un seul. Sur un worker qui + * traite une activité par message, c'est payé à chaque message. */ final class ActivityHandlerPass implements CompilerPassInterface { @@ -39,6 +46,9 @@ public function process(ContainerBuilder $container): void $executor = $container->findDefinition($executorId); $resolver = new ActivityContractResolver(null); + /** @var array $handlerRefs */ + $handlerRefs = []; + foreach ($tagged as $serviceId => $tags) { foreach ($tags as $tag) { $contract = $tag['contract'] ?? null; @@ -72,13 +82,23 @@ public function process(ContainerBuilder $container): void ->setPublic(false) ; - $executor->addMethodCall('register', [ - $activityName, - [new Reference($invokerId), '__invoke'], - ]); + // Une référence dans le localisateur, pas un callable construit à la + // compilation : bâtir `[new Reference(...), '__invoke']` obligerait le + // conteneur à instancier **chaque** invoker — donc chaque gestionnaire et ses + // dépendances — pour en appeler un seul. + $handlerRefs[$activityName] = new Reference($invokerId); } } } + + if ([] === $handlerRefs) { + return; + } + + // Le localisateur ne construit que ce qu'on lui demande, et `ServiceLocatorTagPass` le + // déduplique entre passes : c'est le mécanisme amont pour « beaucoup de candidats, un seul + // appelé », celui qu'emploie `MessengerPass` pour les gestionnaires de messages. + $executor->setArgument('$lazyHandlers', ServiceLocatorTagPass::register($container, $handlerRefs)); } /** diff --git a/src/DurableBundle/DependencyInjection/DurableExtension.php b/src/DurableBundle/DependencyInjection/DurableExtension.php index f7aba215..d1bce7a2 100644 --- a/src/DurableBundle/DependencyInjection/DurableExtension.php +++ b/src/DurableBundle/DependencyInjection/DurableExtension.php @@ -154,7 +154,7 @@ private function registerDbalStores(ContainerBuilder $container, array $config): if ($eventStoreDbal) { $container->register('durable.event_store.dbal', DbalEventStore::class) ->setArguments([$connection, $schema, $config['event_store']['table_name']]) - ->setPublic(true) + ->setPublic(false) ; $container->setAlias(EventStoreInterface::class, 'durable.event_store.dbal')->setPublic(true); @@ -169,7 +169,7 @@ private function registerDbalStores(ContainerBuilder $container, array $config): if ($metadataDbal) { $container->register('durable.workflow_metadata_store.inner', DbalWorkflowMetadataStore::class) ->setArguments([$connection, $schema, $config['workflow_metadata']['table_name']]) - ->setPublic(true) + ->setPublic(false) ; $container->setAlias(WorkflowMetadataStore::class, 'durable.workflow_metadata_store.inner')->setPublic(true); } @@ -207,7 +207,7 @@ private function registerDbalRunCatalog(ContainerBuilder $container, Reference $ $container->register('durable.event_store.dbal.projecting', ProjectingEventStore::class) ->setArguments([new Reference('durable.event_store.dbal'), $projection]) - ->setPublic(true) + ->setPublic(false) ; $container->setAlias(EventStoreInterface::class, 'durable.event_store.dbal.projecting')->setPublic(true); @@ -224,13 +224,13 @@ private function registerDbalRunCatalog(ContainerBuilder $container, Reference $ $container->register('durable.workflow_metadata_store.projecting', ProjectingWorkflowMetadataStore::class) ->setArguments([new Reference('durable.workflow_metadata_store.inner'), $projection]) - ->setPublic(true) + ->setPublic(false) ; $container->setAlias(WorkflowMetadataStore::class, 'durable.workflow_metadata_store.projecting')->setPublic(true); $container->register('durable.run_catalog.dbal', DbalWorkflowRunCatalog::class) ->setArguments([$connection, $schema]) - ->setPublic(true) + ->setPublic(false) ; $container->setAlias(WorkflowRunCatalogInterface::class, 'durable.run_catalog.dbal')->setPublic(true); } @@ -260,14 +260,14 @@ private function registerInMemoryRunCatalog(ContainerBuilder $container): void $container->register('durable.run_catalog.in_memory', InMemoryWorkflowRunCatalog::class) ->setArguments([new Reference('durable.event_store.inner')]) - ->setPublic(true) + ->setPublic(false) ; $catalog = new Reference('durable.run_catalog.in_memory'); $container->setAlias(WorkflowRunCatalogInterface::class, 'durable.run_catalog.in_memory')->setPublic(true); $container->register('durable.event_store.in_memory.projecting', ProjectingEventStore::class) ->setArguments([new Reference('durable.event_store.inner'), $catalog]) - ->setPublic(true) + ->setPublic(false) ; $container->setAlias(EventStoreInterface::class, 'durable.event_store.in_memory.projecting')->setPublic(true); @@ -281,7 +281,7 @@ private function registerInMemoryRunCatalog(ContainerBuilder $container): void $container->register('durable.workflow_metadata_store.in_memory.projecting', ProjectingWorkflowMetadataStore::class) ->setArguments([new Reference('durable.workflow_metadata_store.inner'), $catalog]) - ->setPublic(true) + ->setPublic(false) ; $container->setAlias(WorkflowMetadataStore::class, 'durable.workflow_metadata_store.in_memory.projecting')->setPublic(true); } @@ -330,7 +330,7 @@ private static function isTemporalNative(array $config): bool */ private function registerEventStore(ContainerBuilder $container, array $config): void { - $container->register('durable.event_store.inner', InMemoryEventStore::class)->setPublic(true); + $container->register('durable.event_store.inner', InMemoryEventStore::class)->setPublic(false); $temporalConfig = $config['temporal'] ?? []; $dsn = $temporalConfig['dsn'] ?? null; @@ -378,7 +378,7 @@ private function registerEventStore(ContainerBuilder $container, array $config): new Reference('durable.temporal.connection'), new Reference(TemporalHistoryCursor::class), ]) - ->setPublic(true) + ->setPublic(false) ; if ($journal) { $container->setAlias(WorkflowRunCatalogInterface::class, 'durable.run_catalog.temporal')->setPublic(true); @@ -417,7 +417,7 @@ private function registerEventStore(ContainerBuilder $container, array $config): new Reference(TemporalHistoryCursor::class), new Reference(WorkflowClientInterface::class), ]) - ->setPublic(true) + ->setPublic(false) ; if ($journal) { diff --git a/tests/unit/DurableBundle/DependencyInjection/Compiler/ActivityHandlersAreLazyTest.php b/tests/unit/DurableBundle/DependencyInjection/Compiler/ActivityHandlersAreLazyTest.php new file mode 100644 index 00000000..75a862cd --- /dev/null +++ b/tests/unit/DurableBundle/DependencyInjection/Compiler/ActivityHandlersAreLazyTest.php @@ -0,0 +1,94 @@ +compile(); + + /** @var ActivityExecutor $executor */ + $executor = $container->get(ActivityExecutor::class); + + self::assertSame( + 0, + CompteurDInstances::total(), + 'obtenir l\'exécuteur ne doit construire aucun gestionnaire', + ); + + $executor->execute('premier.faire', ['quoi' => 'ceci']); + + self::assertSame(['premier'], CompteurDInstances::construits()); + } + + public function testLesDeuxGestionnairesRestentJoignables(): void + { + $container = $this->compile(); + + /** @var ActivityExecutor $executor */ + $executor = $container->get(ActivityExecutor::class); + + self::assertSame('premier:ceci', $executor->execute('premier.faire', ['quoi' => 'ceci'])); + self::assertSame('second:cela', $executor->execute('second.faire', ['quoi' => 'cela'])); + self::assertSame(['premier', 'second'], CompteurDInstances::construits()); + } + + public function testUneActiviteInconnueEchoueToujoursClairement(): void + { + $container = $this->compile(); + + /** @var ActivityExecutor $executor */ + $executor = $container->get(ActivityExecutor::class); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessageMatches('/inconnue/'); + + $executor->execute('inconnue', []); + } + + private function compile(): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.debug', false); + (new DurableExtension())->load([[]], $container); + $container->register('messenger.default_bus', \stdClass::class)->setPublic(true); + + foreach ([PremierHandler::class, SecondHandler::class] as $class) { + $container->register($class, $class)->setAutoconfigured(true)->setPublic(false); + } + + + (new DurableBundle())->build($container); + $container->compile(); + + return $container; + } +} diff --git a/tests/unit/DurableBundle/DependencyInjection/DurableContainerSurfaceTest.php b/tests/unit/DurableBundle/DependencyInjection/DurableContainerSurfaceTest.php new file mode 100644 index 00000000..ffb8560e --- /dev/null +++ b/tests/unit/DurableBundle/DependencyInjection/DurableContainerSurfaceTest.php @@ -0,0 +1,90 @@ +, 1: string}> + */ + public static function internesQuiNOntPasAEtrePublics(): iterable + { + $dbal = ['event_store' => ['type' => 'dbal'], 'workflow_metadata' => ['type' => 'dbal']]; + + yield 'journal DBAL' => [$dbal, 'durable.event_store.dbal']; + yield 'journal DBAL projeté' => [$dbal, 'durable.event_store.dbal.projecting']; + yield 'métadonnées DBAL, service interne' => [$dbal, 'durable.workflow_metadata_store.inner']; + yield 'métadonnées DBAL projetées' => [$dbal, 'durable.workflow_metadata_store.projecting']; + yield 'catalogue DBAL' => [$dbal, 'durable.run_catalog.dbal']; + yield 'journal mémoire projeté' => [[], 'durable.event_store.in_memory.projecting']; + yield 'métadonnées mémoire projetées' => [[], 'durable.workflow_metadata_store.in_memory.projecting']; + yield 'catalogue mémoire' => [[], 'durable.run_catalog.in_memory']; + } + + /** + * @param array $config + */ + #[\PHPUnit\Framework\Attributes\DataProvider('internesQuiNOntPasAEtrePublics')] + public function testUnInterneNEstPasPublic(array $config, string $serviceId): void + { + $container = $this->load($config); + + self::assertTrue($container->hasDefinition($serviceId), $serviceId . ' doit exister pour ce backend'); + self::assertFalse( + $container->getDefinition($serviceId)->isPublic(), + $serviceId . " n'a pas de raison d'être tiré du conteneur : on l'atteint par son interface", + ); + } + + /** + * @return iterable + */ + public static function surfacePubliqueQuiDoitLeRester(): iterable + { + yield 'le journal' => [EventStoreInterface::class]; + yield 'les métadonnées' => [WorkflowMetadataStore::class]; + yield 'le catalogue de runs' => [WorkflowRunCatalogInterface::class]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('surfacePubliqueQuiDoitLeRester')] + public function testLaSurfacePubliqueResteJoignable(string $alias): void + { + $container = $this->load(['event_store' => ['type' => 'dbal'], 'workflow_metadata' => ['type' => 'dbal']]); + + self::assertTrue($container->hasAlias($alias), $alias . ' doit rester un alias du conteneur'); + self::assertTrue( + $container->getAlias($alias)->isPublic(), + $alias . ' est ce que le trait de test livré résout, et ce que la documentation nomme', + ); + } + + /** + * @param array $config + */ + private function load(array $config): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.debug', false); + (new DurableExtension())->load([$config], $container); + + return $container; + } +} diff --git a/tests/unit/DurableBundle/Fixtures/CompteurDInstances.php b/tests/unit/DurableBundle/Fixtures/CompteurDInstances.php new file mode 100644 index 00000000..4614b2c4 --- /dev/null +++ b/tests/unit/DurableBundle/Fixtures/CompteurDInstances.php @@ -0,0 +1,37 @@ + */ + private static array $construits = []; + + public static function reset(): void + { + self::$construits = []; + } + + public static function note(string $quoi): void + { + self::$construits[] = $quoi; + } + + /** + * @return list + */ + public static function construits(): array + { + return self::$construits; + } + + public static function total(): int + { + return \count(self::$construits); + } +} diff --git a/tests/unit/DurableBundle/Fixtures/PremierContract.php b/tests/unit/DurableBundle/Fixtures/PremierContract.php new file mode 100644 index 00000000..44937c50 --- /dev/null +++ b/tests/unit/DurableBundle/Fixtures/PremierContract.php @@ -0,0 +1,15 @@ +