[DX] Extend adapter builder #186

maxhelias · 2025-06-13T19:24:49Z

Before merging this PR: #182

I suggested in this comment a rework of parts of the bundle to improve separation of concerns and provide a better developer experience (DX), in preparation for allowing third parties to create their own custom adapters.

Proposed changes

Externalize the getRequiredPackages() logic
- Move this method to the AdapterDefinitionBuilderInterface.
- Let the AdapterDefinitionFactory handle the package check before calling createDefinition().
- Remove the related logic from AbstractAdapterDefinitionBuilder
Extract Unix visibility/permissions logic
- Move configureUnixOptions() and createUnixDefinition() out of the abstract base class.
- Possible solutions:
  - A trait (ProvidesUnixPermissions) – easier to share but harder to test
  - A static helper class (UnixVisibilityHelper) – more testable but less flexible
- Remove this logic from AbstractAdapterDefinitionBuilder
Make builder options discoverable via Symfony's config system
- Introduce a new optional method in the interface: addConfiguration(NodeBuilder $builder) (similar to what Symfony Security does)
- This allows builders to describe their expected configuration structure and default value
- BUT we must not allow several adapters on the same flysystem.
- Deprecate the current adapter and options-based configuration style.
Clean up the abstract class
- Strip AbstractAdapterDefinitionBuilder down to only keep createDefinition().
  - Remove configureOptions
  - Remove configureDefinition
- The class will be much lighter — and perhaps not needed at all — which improves separation of concerns.

I started this PR as a first step, like an RFC. Any feedback is welcome 😉

Benefits of this new config format :

Auto-completion: Full IDE support for all adapter options when using PHP-based configuration files.
Validation & Type safety: Native Symfony configuration validation ensures required options are present and types are correct.
Documentation: Built-in option descriptions allow developers to understand available options without external documentation.
Discovery: Commands like debug:config flysystem and config:dump-reference show all available adapters and their options.
Better DX: Developers can discover, configure, and validate adapters without guessing option names or defaults.

maxhelias · 2025-07-17T13:10:06Z

After a second thought, we could open it this way and think about the last 2 points later.

src/Adapter/Builder/UnixPermissionTrait.php

@inheritdoc

This PR implements a solution to the issue discussed in thephpleague#181, enabling external bundles to register their custom storage adapters with Flysystem Bundle without requiring them to be directly added to the main bundle's codebase. I had to remove the `@internal` annotation from `AdapterDefinitionBuilderInterface` and `AbstractAdapterDefinitionBuilder` to allow them to be referenced by other bundles. ## Usage Example External bundles can now register their adapters as follows: ``` php class AzureBlobStorageAdapterBundle extends Bundle { /** * {@inheritdoc} */ public function build(ContainerBuilder $container): void { parent::build($container); /** @var FlysystemExtension $extension */ $extension = $container->getExtension('flysystem'); $extension->addAdapterDefinitionBuilder(new AzureOssAdapterDefinitionBuilder()); } } ```

maxhelias · 2025-09-08T12:46:12Z

@GromNaN and @dunglas since you’ve both been active recently on GridFS and WebDAV, I’d love to get your thoughts on this! Thanks

GromNaN

I added some random comments. I think it's a good idea to review this.

docs/1-getting-started.md

GromNaN · 2025-09-08T20:16:02Z

docs/5-creating-a-custom-adapter.md

+            ->end();
+    }
+
+    public function createAdapter(ContainerBuilder $container, string $storageName, array $options, ?string $defaultVisibilityForDirectories): string


This method should receive a ContainerConfigurator. The PHP-DSL for service declaration is very convenient. It's the same as service.php config files.

Example of how it's used in the MicroKernelTrait: https://github.com/symfony/symfony/blob/e48850bdb8161e1aa1338432ea469932857b2df5/src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php#L194

That's probably very convenient, but we need to dynamically register adapter services based on configuration. Since the conditions aren't always known in advance, the ContainerBuilder gives us direct access to service registration methods, which seems more straightforward for this conditional, programmatic service creation.

GromNaN · 2025-09-08T20:21:00Z

docs/5-creating-a-custom-adapter.md

+Once registered, you can use the `debug:config flysystem` command to see your custom adapter 
+and all its available options in the configuration tree.
+
+### Testing your custom builder


Thanks for adding this section. Tests are too often skipped because people don't know how to write them.

docs/A-security-disclosure-procedure.md

GromNaN · 2025-09-08T20:23:38Z

docs/5-creating-a-custom-adapter.md

+    public function getRequiredPackages(): array
+    {
+        // Return required packages for your adapter
+        // Format: ['ClassName' => 'vendor/package-name']
+        return [];
+    }


The required package is not only about the package name, but also the version. Does this syntax allow adding the version constraint after the package name?

No, this syntax doesn't support version constraints. It could be interesting, but the use cases are very limited (if not non-existent).
Currently, this mechanism is very useful for official adapters because their packages are separate and have very specific dependencies for each use case.
For a public builder API, the adapter package should theoretically be self-contained. For example, with azure-oss/storage-blob-flysystem, there are two approaches: either Azure OSS provides a dedicated bundle to manage their adapter, or the adapter package includes the AdapterBuilder and users manually register it via the Kernel.

GromNaN · 2025-09-08T20:33:17Z

src/FlysystemBundle.php

+        /** @var FlysystemExtension $extension */
+        $extension = $container->getExtension('flysystem');
+        $extension->addAdapterDefinitionBuilder(new Builder\AsyncAwsAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\AwsAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\AzureAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\FtpAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\GcloudAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\GridFSAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\LazyAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\LocalAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\MemoryAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\SftpAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\WebDAVAdapterDefinitionBuilder());
+        $extension->addAdapterDefinitionBuilder(new Builder\BunnyCDNAdapterDefinitionBuilder());


Features such as autoconfiguration cannot be used to automatically inject an adapter just by creating a class. This function must be called in the bundle or kernel to explicitly add the adapter class.

Yes, it follows the same design pattern as Symfony's security authenticators. I don't think there's any other way (at least for now) to make them available in the TreeBuilder configuration.

src/Adapter/Builder/AdapterDefinitionBuilderInterface.php

maxhelias changed the title ~~[DX] Extend adapter~~ [DX] Extend adapter builder Jun 13, 2025

maxhelias mentioned this pull request Jun 27, 2025

feat: add Azure OSS adapter #181

Open

xabbuh reviewed Jul 17, 2025

View reviewed changes

src/Adapter/Builder/UnixPermissionTrait.php Show resolved Hide resolved

maxhelias and others added 4 commits August 25, 2025 10:26

feat: extract unix persmission in a trait

34c25a8

feat: refacto required packages by a builder

e583a78

fix: correct rebase

856dbe6

maxhelias force-pushed the rfc-custom-adapter branch from 4e314a1 to 856dbe6 Compare August 25, 2025 08:32

maxhelias mentioned this pull request Aug 25, 2025

feat: Allow custom adapter registration #182

Closed

maxhelias force-pushed the rfc-custom-adapter branch 2 times, most recently from ceb7865 to 409f306 Compare August 31, 2025 17:15

maxhelias requested review from tgalopin and xabbuh and removed request for xabbuh August 31, 2025 17:29

GromNaN reviewed Sep 8, 2025

View reviewed changes

feat: make builder options discoverable

ef2675f

maxhelias force-pushed the rfc-custom-adapter branch from 409f306 to ef2675f Compare September 8, 2025 22:33

This was referenced Sep 9, 2025

Symfony integration with Flysystem bundle Azure-OSS/azure-storage-php-adapter-flysystem#19

Open

Symfony integration with Flysystem bundle PlatformCommunity/flysystem-bunnycdn#85

Open

[DX] Extend adapter builder #186

Are you sure you want to change the base?

[DX] Extend adapter builder #186

Uh oh!

Conversation

maxhelias commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Benefits of this new config format :

Uh oh!

maxhelias commented Jul 17, 2025

Uh oh!

Uh oh!

maxhelias commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GromNaN Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

maxhelias Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GromNaN Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GromNaN Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

maxhelias Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

GromNaN Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

maxhelias Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maxhelias commented Jun 13, 2025 •

edited

Loading

maxhelias commented Sep 8, 2025 •

edited

Loading

maxhelias Sep 8, 2025 •

edited

Loading