Skip to content

Commit e3df060

Browse files
committed
Simplify DI using configurator
1 parent 70520ba commit e3df060

File tree

6 files changed

+37
-80
lines changed

6 files changed

+37
-80
lines changed

config/services.php

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@
2020

2121
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
2222

23-
use MongoDB\Bundle\Client;
2423
use MongoDB\Bundle\Command\DebugCommand;
25-
use MongoDB\Bundle\DataCollector\DriverEventSubscriber;
2624
use MongoDB\Bundle\DataCollector\MongoDBDataCollector;
25+
use MongoDB\Client;
2726

2827
return static function (ContainerConfigurator $container): void {
2928
// default configuration for services in *this* file
@@ -46,15 +45,9 @@
4645
->arg('$driverOptions', abstract_arg('Should be defined by pass'))
4746
->abstract();
4847

49-
$services
50-
->set('mongodb.abstract.driver_event_subscriber', DriverEventSubscriber::class)
51-
->arg('$clientName', abstract_arg('Should be defined by pass'))
52-
->arg('$dataCollector', service('mongodb.data_collector'))
53-
->arg('$stopwatch', service('debug.stopwatch')->nullOnInvalid())
54-
->abstract();
55-
5648
$services
5749
->set('mongodb.data_collector', MongoDBDataCollector::class)
50+
->arg('$stopwatch', service('debug.stopwatch')->nullOnInvalid())
5851
->arg('$clients', tagged_iterator('mongodb.client', 'name'))
5952
->tag('data_collector', [
6053
'template' => '@MongoDB/Collector/mongodb.html.twig',

src/Client.php

Lines changed: 0 additions & 33 deletions
This file was deleted.

src/DataCollector/DriverEventSubscriber.php

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ final class DriverEventSubscriber implements CommandSubscriber
3939
private array $stopwatchEvents = [];
4040

4141
public function __construct(
42-
private readonly string $clientName,
42+
private readonly int $clientId,
4343
private readonly MongoDBDataCollector $dataCollector,
4444
private readonly ?Stopwatch $stopwatch = null,
4545
) {
@@ -52,8 +52,7 @@ public function commandStarted(CommandStartedEvent $event): void
5252
$command = (array) $event->getCommand();
5353
unset($command['lsid'], $command['$clusterTime']);
5454

55-
$this->dataCollector->collectCommandEvent($this->clientName, $requestId, [
56-
'clientName' => $this->clientName,
55+
$this->dataCollector->collectCommandEvent($this->clientId, $requestId, [
5756
'databaseName' => $event->getDatabaseName(),
5857
'commandName' => $event->getCommandName(),
5958
'command' => $command,
@@ -63,7 +62,7 @@ public function commandStarted(CommandStartedEvent $event): void
6362
]);
6463

6564
$this->stopwatchEvents[$requestId] = $this->stopwatch?->start(
66-
'mongodb.' . $this->clientName . '.' . $event->getCommandName(),
65+
'mongodb.' . $event->getCommandName(),
6766
'mongodb',
6867
);
6968
}
@@ -75,8 +74,7 @@ public function commandSucceeded(CommandSucceededEvent $event): void
7574
$this->stopwatchEvents[$requestId]?->stop();
7675
unset($this->stopwatchEvents[$requestId]);
7776

78-
$this->dataCollector->collectCommandEvent($this->clientName, $requestId, [
79-
'clientName' => $this->clientName,
77+
$this->dataCollector->collectCommandEvent($this->clientId, $requestId, [
8078
'durationMicros' => $event->getDurationMicros(),
8179
]);
8280
}
@@ -88,8 +86,7 @@ public function commandFailed(CommandFailedEvent $event): void
8886
$this->stopwatchEvents[$requestId]?->stop();
8987
unset($this->stopwatchEvents[$requestId]);
9088

91-
$this->dataCollector->collectCommandEvent($this->clientName, $requestId, [
92-
'clientName' => $this->clientName,
89+
$this->dataCollector->collectCommandEvent($this->clientId, $requestId, [
9390
'durationMicros' => $event->getDurationMicros(),
9491
'error' => $event->getError(),
9592
]);

src/DataCollector/MongoDBDataCollector.php

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,18 @@
2020

2121
namespace MongoDB\Bundle\DataCollector;
2222

23+
use LogicException;
2324
use MongoDB\Client;
2425
use MongoDB\Driver\Command;
2526
use Symfony\Component\HttpFoundation\Request;
2627
use Symfony\Component\HttpFoundation\Response;
2728
use Symfony\Component\HttpKernel\DataCollector\DataCollector;
2829
use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface;
30+
use Symfony\Component\Stopwatch\Stopwatch;
2931
use Throwable;
3032

3133
use function array_diff_key;
34+
use function spl_object_id;
3235

3336
/** @internal */
3437
final class MongoDBDataCollector extends DataCollector implements LateDataCollectorInterface
@@ -41,17 +44,23 @@ final class MongoDBDataCollector extends DataCollector implements LateDataCollec
4144
private array $requests = [];
4245

4346
public function __construct(
47+
private readonly ?Stopwatch $stopwatch = null,
4448
/** @var iterable<string, Client> */
4549
private readonly iterable $clients = [],
4650
) {
4751
}
4852

49-
public function collectCommandEvent(string $clientName, string $requestId, array $data): void
53+
public function configureClient(Client $client): void
5054
{
51-
if (isset($this->requests[$clientName][$requestId])) {
52-
$this->requests[$clientName][$requestId] += $data;
55+
$client->getManager()->addSubscriber(new DriverEventSubscriber(spl_object_id($client), $this, $this->stopwatch));
56+
}
57+
58+
public function collectCommandEvent(int $clientId, string $requestId, array $data): void
59+
{
60+
if (isset($this->requests[$clientId][$requestId])) {
61+
$this->requests[$clientId][$requestId] += $data;
5362
} else {
54-
$this->requests[$clientName][$requestId] = $data;
63+
$this->requests[$clientId][$requestId] = $data;
5564
}
5665
}
5766

@@ -61,21 +70,14 @@ public function collect(Request $request, Response $response, ?Throwable $except
6170

6271
public function lateCollect(): void
6372
{
64-
$requests = $this->requests;
6573
$requestCount = 0;
6674
$errorCount = 0;
6775
$durationMicros = 0;
6876

69-
foreach ($requests as $clientName => $requestsByClient) {
70-
foreach ($requestsByClient as $requestId => $request) {
71-
$requestCount++;
72-
$durationMicros += $request['durationMicros'] ?? 0;
73-
$errorCount += isset($request['error']) ? 1 : 0;
74-
}
75-
}
76-
7777
$clients = [];
78+
$clientIdMap = [];
7879
foreach ($this->clients as $name => $client) {
80+
$clientIdMap[spl_object_id($client)] = $name;
7981
$clients[$name] = [
8082
'serverBuildInfo' => array_diff_key(
8183
(array) $client->getManager()->executeCommand('admin', new Command(['buildInfo' => 1]))->toArray()[0],
@@ -85,6 +87,17 @@ public function lateCollect(): void
8587
];
8688
}
8789

90+
$requests = [];
91+
foreach ($this->requests as $clientId => $requestsByClientId) {
92+
$clientName = $clientIdMap[$clientId] ?? throw new LogicException('Client not found');
93+
foreach ($requestsByClientId as $requestId => $request) {
94+
$requests[$clientName][$requestId] = $request;
95+
$requestCount++;
96+
$durationMicros += $request['durationMicros'] ?? 0;
97+
$errorCount += isset($request['error']) ? 1 : 0;
98+
}
99+
}
100+
88101
$this->data = [
89102
'clients' => $clients,
90103
'requests' => $requests,

src/DependencyInjection/Compiler/DataCollectorPass.php

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,10 @@
2020

2121
namespace MongoDB\Bundle\DependencyInjection\Compiler;
2222

23-
use Symfony\Component\DependencyInjection\ChildDefinition;
2423
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
2524
use Symfony\Component\DependencyInjection\ContainerBuilder;
2625
use Symfony\Component\DependencyInjection\Reference;
2726

28-
use function sprintf;
29-
3027
/** @internal */
3128
final class DataCollectorPass implements CompilerPassInterface
3229
{
@@ -38,11 +35,7 @@ public function process(ContainerBuilder $container): void
3835

3936
// Add a subscriber to each client to collect driver events, and register the client to the data collector.
4037
foreach ($container->findTaggedServiceIds('mongodb.client', true) as $clientId => $attributes) {
41-
$subscriberId = sprintf('%s.subscriber', $clientId);
42-
$subscriber = new ChildDefinition('mongodb.abstract.driver_event_subscriber');
43-
$subscriber->replaceArgument('$clientName', $attributes[0]['name'] ?? $clientId);
44-
$container->setDefinition($subscriberId, $subscriber);
45-
$container->getDefinition($clientId)->addMethodCall('addSubscriber', [new Reference($subscriberId)]);
38+
$container->getDefinition($clientId)->setConfigurator([new Reference('mongodb.data_collector'), 'configureClient']);
4639
}
4740
}
4841
}

tests/Unit/DependencyInjection/MongoDBExtensionTest.php

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function testLoadWithSingleClient(): void
7070
// Check service definition
7171
$definition = $container->getDefinition('mongodb.client.default');
7272
$this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri'));
73-
$this->assertTrue($definition->hasMethodCall('addSubscriber'));
73+
$this->assertNotNull($definition->getConfigurator());
7474
$this->assertInstanceOf(ChildDefinition::class, $definition);
7575
$this->assertSame('mongodb.abstract.client', $definition->getParent());
7676
$parentDefinition = $container->getDefinition($definition->getParent());
@@ -132,20 +132,14 @@ public function testLoadWithMultipleClients(): void
132132
$this->assertSame('mongodb.abstract.client', $definition->getParent());
133133
$this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri'));
134134
$this->assertSame(['readPreference' => 'primary'], $definition->getArgument('$uriOptions'));
135-
$this->assertTrue($definition->hasMethodCall('addSubscriber'));
135+
$this->assertNotNull($definition->getConfigurator());
136136

137137
$definition = $container->getDefinition('mongodb.client.secondary');
138138
$this->assertInstanceOf(ChildDefinition::class, $definition);
139139
$this->assertSame('mongodb.abstract.client', $definition->getParent());
140140
$this->assertSame('mongodb://localhost:27018', $definition->getArgument('$uri'));
141141
$this->assertEquals(['serverApi' => new ServerApi((string) ServerApi::V1)], $definition->getArgument('$driverOptions'));
142-
$this->assertTrue($definition->hasMethodCall('addSubscriber'));
143-
144-
// Check data collector
145-
$definition = $container->getDefinition('mongodb.data_collector');
146-
$this->assertCount(2, $definition->getMethodCalls());
147-
$this->assertTrue($definition->hasMethodCall('addClient'));
148-
$this->assertSame(['default', 'secondary'], array_column(array_column($definition->getMethodCalls(), 1), 0));
142+
$this->assertNotNull($definition->getConfigurator());
149143
}
150144

151145
private function getContainer(array $config = [], array $thirdPartyDefinitions = []): ContainerBuilder

0 commit comments

Comments
 (0)