Skip to content

Commit 6ddad31

Browse files
norberttechnorberttech
authored
Merge pull request #2356 from flow-php/2355-bug-sf-filesystem-bundle---s3-client-is-private
refactor: accessing services through service container in symfony bun…
2 parents 4a25dc3 + 3831bb9 commit 6ddad31

33 files changed

Lines changed: 1284 additions & 975 deletions

File tree

‎phpstan.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ parameters:
113113
- src/bridge/symfony/filesystem-bundle/src/Flow/Bridge/Symfony/FilesystemBundle/Resources/config/*
114114
- src/bridge/symfony/postgresql-bundle/src/Flow/Bridge/Symfony/PostgreSqlBundle/Resources/config/*
115115
- src/bridge/symfony/postgresql-bundle/tests/Flow/Bridge/Symfony/PostgreSqlBundle/Tests/Integration/Command/*
116+
- src/bridge/symfony/postgresql-bundle/tests/Flow/Bridge/Symfony/PostgreSqlBundle/Tests/Unit/DependencyInjection/ConfigurationTest.php
116117
- src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/Resources/config/instrumentation/*
117118
- src/bridge/symfony/telemetry-bundle/tests/Flow/Bridge/Symfony/TelemetryBundle/Tests/Unit/DependencyInjection/ConfigurationTest.php
118119
- src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/Instrumentation/Messenger/TracingMiddleware.php

‎rector.src.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
ArrayToFirstClassCallableRector::class => [
3535
__DIR__ . '/src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/DependencyInjection/FlowTelemetryExtension.php',
3636
__DIR__ . '/src/bridge/symfony/filesystem-bundle/src/Flow/Bridge/Symfony/FilesystemBundle/DependencyInjection/Compiler/BuildFstabsPass.php',
37-
__DIR__ . '/src/bridge/symfony/postgresql-bundle/src/Flow/Bridge/Symfony/PostgreSqlBundle/DependencyInjection/FlowPostgreSqlExtension.php',
37+
__DIR__ . '/src/bridge/symfony/postgresql-bundle/src/Flow/Bridge/Symfony/PostgreSqlBundle/FlowPostgreSqlBundle.php',
3838
],
3939
])
4040
->withCache(__DIR__ . '/var/rector/src')

‎src/bridge/symfony/filesystem-bundle/src/Flow/Bridge/Symfony/FilesystemBundle/DependencyInjection/Compiler/BuildFstabsPass.php‎

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ public function process(ContainerBuilder $container) : void
4242
$availableTypes = $this->collectAvailableTypes($container);
4343

4444
foreach ($fstabs as $fstabName => $fstabConfig) {
45+
$resolvedFilesystems = [];
46+
4547
foreach ($fstabConfig['filesystems'] as $mountName => $entry) {
4648
if (!\array_key_exists($entry['type'], $availableTypes)) {
4749
throw new LogicException(\sprintf(
@@ -52,6 +54,8 @@ public function process(ContainerBuilder $container) : void
5254
\implode(', ', \array_keys($availableTypes)),
5355
));
5456
}
57+
58+
$resolvedFilesystems[$mountName] = $this->resolveServiceReferences($entry);
5559
}
5660

5761
$telemetryReference = $this->buildTelemetryConfigReference($container, $fstabName, $fstabConfig['telemetry'] ?? []);
@@ -61,7 +65,7 @@ public function process(ContainerBuilder $container) : void
6165
$definition->setArguments([
6266
new Reference(RegisterFilesystemFactoriesPass::REGISTRY_SERVICE_ID),
6367
$fstabName,
64-
$fstabConfig['filesystems'],
68+
$resolvedFilesystems,
6569
$telemetryReference,
6670
]);
6771
$definition->setPublic(false);
@@ -143,4 +147,84 @@ private function collectAvailableTypes(ContainerBuilder $container) : array
143147

144148
return $types;
145149
}
150+
151+
/**
152+
* @param array<string, mixed>&array{type: string} $entry
153+
*
154+
* @return array<string, mixed>&array{type: string}
155+
*/
156+
private function resolveAwsS3References(array $entry) : array
157+
{
158+
if (\array_key_exists('client_service_id', $entry) && \is_string($entry['client_service_id']) && $entry['client_service_id'] !== '') {
159+
$entry['client'] = new Reference($entry['client_service_id']);
160+
unset($entry['client_service_id']);
161+
}
162+
163+
if (\array_key_exists('client', $entry) && \is_array($entry['client'])) {
164+
$client = $entry['client'];
165+
166+
if (\array_key_exists('http_client_service_id', $client) && \is_string($client['http_client_service_id']) && $client['http_client_service_id'] !== '') {
167+
$client['http_client'] = new Reference($client['http_client_service_id']);
168+
unset($client['http_client_service_id']);
169+
}
170+
171+
if (\array_key_exists('logger_service_id', $client) && \is_string($client['logger_service_id']) && $client['logger_service_id'] !== '') {
172+
$client['logger'] = new Reference($client['logger_service_id']);
173+
unset($client['logger_service_id']);
174+
}
175+
176+
$entry['client'] = $client;
177+
}
178+
179+
return $entry;
180+
}
181+
182+
/**
183+
* @param array<string, mixed>&array{type: string} $entry
184+
*
185+
* @return array<string, mixed>&array{type: string}
186+
*/
187+
private function resolveAzureBlobReferences(array $entry) : array
188+
{
189+
if (\array_key_exists('client_service_id', $entry) && \is_string($entry['client_service_id']) && $entry['client_service_id'] !== '') {
190+
$entry['client'] = new Reference($entry['client_service_id']);
191+
unset($entry['client_service_id']);
192+
}
193+
194+
if (\array_key_exists('client', $entry) && \is_array($entry['client'])) {
195+
$client = $entry['client'];
196+
197+
$serviceKeyMap = [
198+
'http_client_service' => 'http_client',
199+
'request_factory_service' => 'request_factory',
200+
'stream_factory_service' => 'stream_factory',
201+
'logger_service_id' => 'logger',
202+
];
203+
204+
foreach ($serviceKeyMap as $configKey => $resolvedKey) {
205+
if (\array_key_exists($configKey, $client) && \is_string($client[$configKey]) && $client[$configKey] !== '') {
206+
$client[$resolvedKey] = new Reference($client[$configKey]);
207+
unset($client[$configKey]);
208+
}
209+
}
210+
211+
$entry['client'] = $client;
212+
}
213+
214+
return $entry;
215+
}
216+
217+
/**
218+
* @param array<string, mixed>&array{type: string} $entry
219+
*
220+
* @return array<string, mixed>&array{type: string}
221+
*/
222+
private function resolveServiceReferences(array $entry) : array
223+
{
224+
return match ($entry['type']) {
225+
'aws_s3' => $this->resolveAwsS3References($entry),
226+
'azure_blob' => $this->resolveAzureBlobReferences($entry),
227+
default => $entry,
228+
};
229+
}
146230
}

‎src/bridge/symfony/filesystem-bundle/src/Flow/Bridge/Symfony/FilesystemBundle/Filesystem/Factory/AsyncAwsS3FilesystemFactory.php‎

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,14 @@
1010
use Flow\Bridge\Symfony\FilesystemBundle\Filesystem\FilesystemFactory;
1111
use Flow\Filesystem\Bridge\AsyncAWS\Options;
1212
use Flow\Filesystem\Filesystem;
13-
use Psr\Container\ContainerInterface;
13+
use Psr\Log\LoggerInterface;
14+
use Symfony\Contracts\HttpClient\HttpClientInterface;
1415

1516
final readonly class AsyncAwsS3FilesystemFactory implements FilesystemFactory
1617
{
17-
public function __construct(private ContainerInterface $container)
18-
{
19-
}
20-
2118
public function create(string $protocol, array $config) : Filesystem
2219
{
23-
$allowed = ['bucket', 'client_service_id', 'client', 'options'];
20+
$allowed = ['bucket', 'client', 'options'];
2421
$unknown = \array_diff(\array_keys($config), $allowed);
2522

2623
if ($unknown !== []) {
@@ -36,27 +33,25 @@ public function create(string $protocol, array $config) : Filesystem
3633
}
3734

3835
$bucket = $config['bucket'];
39-
$clientServiceId = $config['client_service_id'] ?? null;
40-
$clientConfig = $config['client'] ?? null;
4136

42-
if (($clientServiceId === null) === ($clientConfig === null)) {
37+
if (!\array_key_exists('client', $config) || $config['client'] === null) {
4338
throw new InvalidArgumentException('Filesystem factory for backend "aws_s3" requires exactly one of `client_service_id` or `client`.');
4439
}
4540

46-
if (\is_string($clientServiceId) && $clientServiceId !== '') {
47-
$client = $this->container->get($clientServiceId);
41+
$client = $config['client'];
4842

49-
if (!$client instanceof S3Client) {
50-
throw new InvalidArgumentException(\sprintf('Service "%s" is not an instance of %s.', $clientServiceId, S3Client::class));
51-
}
43+
if ($client instanceof S3Client) {
44+
$resolvedClient = $client;
45+
} elseif (\is_array($client)) {
46+
/** @var array<string, mixed> $client */
47+
$resolvedClient = $this->buildClient($client);
5248
} else {
53-
/** @var array<string, mixed> $clientConfig */
54-
$client = $this->buildClient($clientConfig ?? []);
49+
throw new InvalidArgumentException(\sprintf('Filesystem factory for backend "aws_s3" `client` must be an array or %s instance, got %s.', S3Client::class, \get_debug_type($client)));
5550
}
5651

5752
$options = $this->buildOptions($config['options'] ?? null);
5853

59-
return aws_s3_filesystem($bucket, $client, $options, $protocol);
54+
return aws_s3_filesystem($bucket, $resolvedClient, $options, $protocol);
6055
}
6156

6257
public function type() : string
@@ -69,7 +64,7 @@ public function type() : string
6964
*/
7065
private function buildClient(array $clientConfig) : S3Client
7166
{
72-
$allowed = ['region', 'access_key_id', 'access_key_secret', 'session_token', 'endpoint', 'path_style_endpoint', 'shared_credentials_file', 'shared_config_file', 'profile', 'debug', 'http_client_service_id', 'logger_service_id'];
67+
$allowed = ['region', 'access_key_id', 'access_key_secret', 'session_token', 'endpoint', 'path_style_endpoint', 'shared_credentials_file', 'shared_config_file', 'profile', 'debug', 'http_client', 'logger'];
7368
$unknown = \array_diff(\array_keys($clientConfig), $allowed);
7469

7570
if ($unknown !== []) {
@@ -83,22 +78,20 @@ private function buildClient(array $clientConfig) : S3Client
8378
$httpClient = null;
8479
$logger = null;
8580

86-
if (\array_key_exists('http_client_service_id', $clientConfig)) {
87-
$id = $clientConfig['http_client_service_id'];
88-
89-
if (\is_string($id) && $id !== '') {
90-
$httpClient = $this->container->get($id);
81+
if (\array_key_exists('http_client', $clientConfig) && $clientConfig['http_client'] !== null) {
82+
if (!$clientConfig['http_client'] instanceof HttpClientInterface) {
83+
throw new InvalidArgumentException(\sprintf('Filesystem factory for backend "aws_s3" `client.http_client_service_id` must reference a service implementing %s.', HttpClientInterface::class));
9184
}
92-
unset($clientConfig['http_client_service_id']);
85+
$httpClient = $clientConfig['http_client'];
86+
unset($clientConfig['http_client']);
9387
}
9488

95-
if (\array_key_exists('logger_service_id', $clientConfig)) {
96-
$id = $clientConfig['logger_service_id'];
97-
98-
if (\is_string($id) && $id !== '') {
99-
$logger = $this->container->get($id);
89+
if (\array_key_exists('logger', $clientConfig) && $clientConfig['logger'] !== null) {
90+
if (!$clientConfig['logger'] instanceof LoggerInterface) {
91+
throw new InvalidArgumentException(\sprintf('Filesystem factory for backend "aws_s3" `client.logger_service_id` must reference a service implementing %s.', LoggerInterface::class));
10092
}
101-
unset($clientConfig['logger_service_id']);
93+
$logger = $clientConfig['logger'];
94+
unset($clientConfig['logger']);
10295
}
10396

10497
$keyMap = [

‎src/bridge/symfony/filesystem-bundle/src/Flow/Bridge/Symfony/FilesystemBundle/Filesystem/Factory/AzureBlobFilesystemFactory.php‎

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,15 @@
1212
use Flow\Filesystem\Bridge\Azure\Options;
1313
use Flow\Filesystem\Filesystem;
1414
use Http\Discovery\{Psr17FactoryDiscovery, Psr18ClientDiscovery};
15-
use Psr\Container\ContainerInterface;
1615
use Psr\Http\Client\ClientInterface;
1716
use Psr\Http\Message\{RequestFactoryInterface, StreamFactoryInterface};
1817
use Psr\Log\LoggerInterface;
1918

2019
final readonly class AzureBlobFilesystemFactory implements FilesystemFactory
2120
{
22-
public function __construct(private ContainerInterface $container)
23-
{
24-
}
25-
2621
public function create(string $protocol, array $config) : Filesystem
2722
{
28-
$allowed = ['container', 'client_service_id', 'client', 'options'];
23+
$allowed = ['container', 'client', 'options'];
2924
$unknown = \array_diff(\array_keys($config), $allowed);
3025

3126
if ($unknown !== []) {
@@ -40,23 +35,21 @@ public function create(string $protocol, array $config) : Filesystem
4035
throw new InvalidArgumentException('Filesystem factory for backend "azure_blob" requires a non-empty `container` option.');
4136
}
4237

43-
$container = $config['container'];
44-
$clientServiceId = $config['client_service_id'] ?? null;
45-
$clientConfig = $config['client'] ?? null;
38+
$containerName = $config['container'];
4639

47-
if (($clientServiceId === null) === ($clientConfig === null)) {
40+
if (!\array_key_exists('client', $config) || $config['client'] === null) {
4841
throw new InvalidArgumentException('Filesystem factory for backend "azure_blob" requires exactly one of `client_service_id` or `client`.');
4942
}
5043

51-
if (\is_string($clientServiceId) && $clientServiceId !== '') {
52-
$blobService = $this->container->get($clientServiceId);
44+
$client = $config['client'];
5345

54-
if (!$blobService instanceof BlobServiceInterface) {
55-
throw new InvalidArgumentException(\sprintf('Service "%s" is not an instance of %s.', $clientServiceId, BlobServiceInterface::class));
56-
}
46+
if ($client instanceof BlobServiceInterface) {
47+
$blobService = $client;
48+
} elseif (\is_array($client)) {
49+
/** @var array<string, mixed> $client */
50+
$blobService = $this->buildBlobService($containerName, $client);
5751
} else {
58-
/** @var array<string, mixed> $clientConfig */
59-
$blobService = $this->buildBlobService($container, $clientConfig ?? []);
52+
throw new InvalidArgumentException(\sprintf('Filesystem factory for backend "azure_blob" `client` must be an array or %s instance, got %s.', BlobServiceInterface::class, \get_debug_type($client)));
6053
}
6154

6255
$options = $this->buildOptions($config['options'] ?? null);
@@ -74,7 +67,7 @@ public function type() : string
7467
*/
7568
private function buildBlobService(string $containerName, array $clientConfig) : BlobServiceInterface
7669
{
77-
$allowed = ['account_name', 'auth', 'url_factory', 'http_client_service', 'request_factory_service', 'stream_factory_service', 'logger_service_id'];
70+
$allowed = ['account_name', 'auth', 'url_factory', 'http_client', 'request_factory', 'stream_factory', 'logger'];
7871
$unknown = \array_diff(\array_keys($clientConfig), $allowed);
7972

8073
if ($unknown !== []) {
@@ -109,10 +102,10 @@ private function buildBlobService(string $containerName, array $clientConfig) :
109102
$authFactory = azure_shared_key_authorization_factory($accountName, $clientConfig['auth']['shared_key']);
110103
$configuration = azure_blob_service_config($accountName, $containerName);
111104

112-
$httpClient = $this->resolveService($clientConfig['http_client_service'] ?? null, ClientInterface::class) ?? Psr18ClientDiscovery::find();
113-
$requestFactory = $this->resolveService($clientConfig['request_factory_service'] ?? null, RequestFactoryInterface::class) ?? Psr17FactoryDiscovery::findRequestFactory();
114-
$streamFactory = $this->resolveService($clientConfig['stream_factory_service'] ?? null, StreamFactoryInterface::class) ?? Psr17FactoryDiscovery::findStreamFactory();
115-
$logger = $this->resolveService($clientConfig['logger_service_id'] ?? null, LoggerInterface::class);
105+
$httpClient = $this->resolveResolvedService($clientConfig['http_client'] ?? null, ClientInterface::class, 'http_client_service') ?? Psr18ClientDiscovery::find();
106+
$requestFactory = $this->resolveResolvedService($clientConfig['request_factory'] ?? null, RequestFactoryInterface::class, 'request_factory_service') ?? Psr17FactoryDiscovery::findRequestFactory();
107+
$streamFactory = $this->resolveResolvedService($clientConfig['stream_factory'] ?? null, StreamFactoryInterface::class, 'stream_factory_service') ?? Psr17FactoryDiscovery::findStreamFactory();
108+
$logger = $this->resolveResolvedService($clientConfig['logger'] ?? null, LoggerInterface::class, 'logger_service_id');
116109

117110
$httpFactory = azure_http_factory($requestFactory, $streamFactory);
118111

@@ -178,16 +171,14 @@ private function buildOptions(mixed $optionsConfig) : Options
178171
*
179172
* @return null|T
180173
*/
181-
private function resolveService(mixed $serviceId, string $expectedClass) : ?object
174+
private function resolveResolvedService(mixed $service, string $expectedClass, string $configKey) : ?object
182175
{
183-
if (!\is_string($serviceId) || $serviceId === '') {
176+
if ($service === null) {
184177
return null;
185178
}
186179

187-
$service = $this->container->get($serviceId);
188-
189180
if (!$service instanceof $expectedClass) {
190-
throw new InvalidArgumentException(\sprintf('Service "%s" is not an instance of %s.', $serviceId, $expectedClass));
181+
throw new InvalidArgumentException(\sprintf('Filesystem factory for backend "azure_blob" `client.%s` must reference a service implementing %s.', $configKey, $expectedClass));
191182
}
192183

193184
return $service;

‎src/bridge/symfony/filesystem-bundle/src/Flow/Bridge/Symfony/FilesystemBundle/Resources/config/services.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,13 @@
3232
$services
3333
->set('.flow.filesystem.factory.aws_s3', AsyncAwsS3FilesystemFactory::class)
3434
->private()
35-
->args([service('service_container')])
3635
->tag('flow.filesystem.factory', ['type' => 'aws_s3']);
3736
}
3837

3938
if (\class_exists(AzureBlobFilesystem::class)) {
4039
$services
4140
->set('.flow.filesystem.factory.azure_blob', AzureBlobFilesystemFactory::class)
4241
->private()
43-
->args([service('service_container')])
4442
->tag('flow.filesystem.factory', ['type' => 'azure_blob']);
4543
}
4644

0 commit comments

Comments
 (0)