[DX] Extend adapter builder by maxhelias · Pull Request #186 · thephpleague/flysystem-bundle

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.

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.

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

+            ->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

+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.

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

+    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

+        /** @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.

maxhelias · 2026-03-27T13:03:59Z

It's time to move forward on this topic !

@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. 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()); } } ```

alexander-schranz · 2026-03-27T14:26:53Z

@maxhelias not sure if you are aware of this but current 3.x seems be broken:

wanted to let you know before you plan a new release :)

maxhelias · 2026-03-27T14:43:18Z

Oh thanks @alexander-schranz for the feedback, I'll take a look

…lias) This PR was merged into the 5.x branch. Discussion ---------- fix(docs): fix configuration on documentation file Since thephpleague/flysystem-bundle#186 Commits ------- dbef511 fix(docs): fix configuration on documentation file

Description ----------- The changes from thephpleague/flysystem-bundle#186 got released as `3.7.0` and as the `@internal` class `League\FlysystemBundle\Adapter\AdapterDefinitionFactory` was deleted there we need conflict with that version until we update our usage of that class. Commits ------- 21f5878 Conflict with league/flysystem-bundle 3.7

Description ----------- The changes from thephpleague/flysystem-bundle#186 got released as `3.7.0` and as the `@internal` class `League\FlysystemBundle\Adapter\AdapterDefinitionFactory` was deleted there we need conflict with that version until we update our usage of that class. Commits ------- 21f58781 Conflict with league/flysystem-bundle 3.7

…em adapter Introduces a new sub-package `azure-oss/storage-blob-flysystem-symfony` that registers an `azure_oss` adapter shortcut with `league/flysystem-bundle` 3.7+'s pluggable `AdapterDefinitionBuilderInterface` (thephpleague/flysystem-bundle#186). This is the canonical extension point: per-adapter builders now live in the adapter package rather than being merged into flysystem-bundle's core builder list. The author of PR thephpleague/flysystem-bundle#181 ("feat: add Azure OSS adapter") was pointed at #186 as the recommended approach; this package implements that. New sub-package: src/BlobFlysystemSymfony/ - AzureOssAdapterDefinitionBuilder: maps the `azure_oss` config key to a BlobContainerClient (built via a user-supplied BlobServiceClient service id) and the AzureBlobStorageAdapter. - AzureOssFlysystemBundle: extends Symfony's Bundle and registers the builder on flysystem-bundle's FlysystemExtension during build(), mirroring how FlysystemBundle wires its own built-in adapters. - composer.json declares `type: symfony-bundle` so Symfony Flex auto-registers the bundle for users who have flex installed. Configuration shape (see src/BlobFlysystemSymfony/README.md for the full reference): flysystem: storages: default.storage: azure_oss: client: azure_blob_service_client container: '%env(AZURE_STORAGE_CONTAINER)%' # optional: prefix, mime_type_detector, # visibility_handling (throw|ignore), public_container Tests under tests/BlobFlysystemSymfony/: - AzureOssAdapterDefinitionBuilderTest extends flysystem-bundle's AbstractAdapterDefinitionBuilderTest (the same base used by the bundle's built-in builders), driving minimal and full option fixtures through the Config tree and asserting on the resulting Definition objects. Adds extra coverage for getName(), getRequiredPackages(), and the null-mime-type-detector path. - AzureOssFlysystemBundleTest boots a ContainerBuilder, runs both FlysystemBundle::build() and AzureOssFlysystemBundle::build() in the same order Symfony's Kernel would, and asserts that the azure_oss builder is registered on the FlysystemExtension. Root composer.json gains four require-dev entries needed by the new package's CI path: `league/flysystem-bundle`, `symfony/config`, `symfony/dependency-injection`, `symfony/http-kernel`. No other config changes — autoload PSR-4 `AzureOss\\Storage\\` -> `src/` already covers the new directory; phpunit.xml's `tests/` glob picks up the new test suite; phpstan and pint configs apply unchanged. The new package follows the existing conventions: - final class with @internal PHPDoc, declare(strict_types=1) - pint Laravel preset with declare_strict_types + final_internal_class - PHPStan level 10 + strict-rules clean (no asserts/@var/ignores) Verification: - vendor/bin/phpstan analyse --memory-limit=2G : OK - vendor/bin/phpunit --filter 'BlobFlysystemSymfony' : 8 tests / 32 assertions, all green - vendor/bin/pint src tests --test : PASS (146 files) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

Comment thread src/Adapter/Builder/UnixPermissionTrait.php

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

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

maxhelias mentioned this pull request Sep 10, 2025

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

Open

GromNaN mentioned this pull request Nov 12, 2025

Make each handler type have its own configuration and enable custom handler types symfony/monolog-bundle#558

Open

maxhelias force-pushed the rfc-custom-adapter branch 3 times, most recently from 2b035a2 to 93d5eee Compare February 5, 2026 15:39

maxhelias force-pushed the rfc-custom-adapter branch from 93d5eee to 2a37a94 Compare March 27, 2026 12:56

maxhelias added 5 commits March 27, 2026 14:09

feat: extract unix persmission in a trait

e049307

feat: refacto required packages by a builder

0896389

fix: correct rebase

3173be3

feat: make builder options discoverable

6930e66

maxhelias force-pushed the rfc-custom-adapter branch from 2a37a94 to 6930e66 Compare March 27, 2026 13:09

maxhelias merged commit cd0da59 into thephpleague:3.x Mar 27, 2026
6 checks passed

maxhelias mentioned this pull request Mar 27, 2026

fix(webdav): Use string instead of a constant in the WebDAV adapter builder #205

Merged

ausi mentioned this pull request Apr 16, 2026

Conflict with league/flysystem-bundle 3.7 contao/contao#9756

Merged

maxhelias mentioned this pull request Apr 21, 2026

fix(docs): fix configuration on documentation file EasyCorp/EasyAdminBundle#7572

Merged

cancan101 mentioned this pull request May 24, 2026

feat(BlobFlysystemSymfony): add Symfony bundle bridge for the Flysystem adapter php-oss-for-azure/azure-php#34

Merged

5 tasks

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!

maxhelias commented Mar 27, 2026

Uh oh!

Uh oh!

alexander-schranz commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxhelias commented Mar 27, 2026

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

alexander-schranz commented Mar 27, 2026 •

edited

Loading