Implement functional tests and improve architecture

Author: tgalopin

composer.json

@@ -26,12 +26,10 @@
         "symfony/options-resolver": "^4.2"
     },
     "require-dev": {
-        "jackalope/jackalope-doctrine-dbal": "^1.3",
         "league/flysystem-aws-s3-v3": "^1.0",
         "league/flysystem-azure-blob-storage": "^0.1.5",
         "league/flysystem-cached-adapter": "^1.0",
         "league/flysystem-memory": "^1.0",
-        "league/flysystem-phpcr": "^1.1",
         "league/flysystem-rackspace": "^1.0",
         "league/flysystem-replicate-adapter": "^1.0",
         "league/flysystem-sftp": "^1.0",

docs/1-getting-started.md

@@ -23,9 +23,9 @@ use Flysystem in your application as soon as you install the bundle:
 # config/packages/flysystem.yaml
 
 flysystem:
-    default_filesystem: 'app'
+    default_filesystem: 'default.storage'
     filesystems:
-        app:
+        default.storage:
             adapter: 'flysystem.adapter.local'
 ```
 
@@ -41,11 +41,11 @@ use League\Flysystem\FilesystemInterface;
 
 class MyService
 {
-    private $filesystem;
+    private $storage;
     
-    public function __construct(FilesystemInterface $filesystem)
+    public function __construct(FilesystemInterface $storage)
     {
-        $this->filesystem = $filesystem;
+        $this->storage = $storage;
     }
     
     // ...
@@ -59,7 +59,7 @@ use League\Flysystem\FilesystemInterface;
 
 class MyController
 {
-    public function index(FilesystemInterface $filesystem)
+    public function index(FilesystemInterface $storage)
     {
         // ...
     }
@@ -87,26 +87,26 @@ name to get this specific filesystem:
 flysystem:
     default_filesystem: 'app'
     filesystems:
-        app:
-            adapter: 'flysystem.adapter.local'
+        upload.storage:
+            adapter: 'local'
+            options:
+                directory: '%kernel.project_dir%/storage'
 
-        # Defines an additional filesystem named "myFilesystem"
-        myFilesystem:
+        tmp.storage:
             adapter: 'flysystem.adapter.local'
+            options:
+                directory: '/tmp'
 ```
 
 ```php
 use League\Flysystem\FilesystemInterface;
 
 class MyController
 {
-    public function index(FilesystemInterface $fs, FilesystemInterface $myFilesystem)
+    public function index(FilesystemInterface $fs, FilesystemInterface $tmpStorage)
     {
         // $fs is referencing the default filesystem ("app") because the variable name is not a filesystem name
-        // $myFilesystem is referencing the "myFilesystem" filesystem
+        // $tmpStorage is referencing the "tmp.storage" filesystem
     }
 }
 ```
-
-If you are not using autowiring, you can inject the `flysystem.filesystem.<filesystem-name>` service into 
-your services manually to get a specific filesystem (for instance here: `flysystem.filesystem.myFilesystem`).

src/Adapter/AdapterDefinitionFactory.php

@@ -36,7 +36,6 @@ public function __construct()
             new Builder\FtpAdapterDefinitionBuilder($this),
             new Builder\GcloudAdapterDefinitionBuilder($this),
             new Builder\LocalAdapterDefinitionBuilder($this),
-            new Builder\PhpcrAdapterDefinitionBuilder($this),
             new Builder\RackspaceAdapterDefinitionBuilder($this),
             new Builder\ReplicateAdapterDefinitionBuilder($this),
             new Builder\SftpAdapterDefinitionBuilder($this),

src/Adapter/Builder/CacheAdapterDefinitionBuilder.php

@@ -12,6 +12,7 @@
 namespace League\FlysystemBundle\Adapter\Builder;
 
 use League\Flysystem\Cached\CachedAdapter;
+use League\Flysystem\Cached\Storage\Psr6Cache;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\OptionsResolver\OptionsResolver;
@@ -39,6 +40,6 @@ protected function configureDefinition(Definition $definition, array $options)
     {
         $definition->setClass(CachedAdapter::class);
         $definition->setArgument(0, new Reference('flysystem.adapter.'.$options['source']));
-        $definition->setArgument(1, new Reference($options['store']));
+        $definition->setArgument(1, new Definition(Psr6Cache::class, [new Reference($options['store'])]));
     }
 }

src/Adapter/Builder/PhpcrAdapterDefinitionBuilder.php

@@ -11,7 +11,6 @@
 
 namespace League\FlysystemBundle\Adapter\Builder;
 
-use League\Flysystem\Adapter\Ftp;
 use League\Flysystem\Sftp\SftpAdapter;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\OptionsResolver\OptionsResolver;

src/Adapter/Builder/SftpAdapterDefinitionBuilder.php

@@ -12,7 +12,6 @@
 namespace League\FlysystemBundle\Adapter\Builder;
 
 use League\Flysystem\WebDAV\WebDAVAdapter;
-use Spatie\FlysystemDropbox\DropboxAdapter;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\OptionsResolver\OptionsResolver;

src/Adapter/Builder/WebdavAdapterDefinitionBuilder.php

@@ -32,11 +32,7 @@ public function getConfigTreeBuilder()
                 ->arrayNode('filesystems')
                     ->arrayPrototype()
                         ->children()
-                            ->scalarNode('adapter')->defaultNull()->end()
-                            ->arrayNode('mounts')
-                                ->scalarPrototype()
-                                ->end()
-                            ->end()
+                            ->scalarNode('adapter')->isRequired()->end()
                             ->arrayNode('options')
                                 ->variablePrototype()
                                 ->end()

src/DependencyInjection/Configuration.php

@@ -11,15 +11,10 @@
 
 namespace League\FlysystemBundle\DependencyInjection;
 
-use League\Flysystem\Cached\CachedAdapter;
-use League\Flysystem\Cached\Storage\Memory;
-use League\Flysystem\Cached\Storage\Psr6Cache;
 use League\Flysystem\Filesystem;
 use League\Flysystem\FilesystemInterface;
 use League\FlysystemBundle\Adapter\AdapterDefinitionFactory;
-use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
@@ -36,51 +31,50 @@ public function load(array $configs, ContainerBuilder $container)
         $configuration = new Configuration();
         $config = $this->processConfiguration($configuration, $configs);
 
-        $this->registerCacheProviders($container);
         $this->registerFilesystems($container, $config['filesystems']);
 
         // Create default filesystem alias
         if (!isset($config['filesystems'][$config['default_filesystem']])) {
             throw new \LogicException('Default filesystem "'.$config['default_filesystem'].'" is not defined in the "flysystem.filesystems" configuration key.');
         }
 
-        $defaultFsName = $config['default_filesystem'];
-        $container->setAlias(FilesystemInterface::class, 'flysystem.filesystem.'.$defaultFsName)->setPublic(false);
-        $container->setAlias('flysystem', 'flysystem.filesystem.'.$defaultFsName)->setPublic(false);
-    }
-
-    private function registerCacheProviders(ContainerBuilder $container)
-    {
-        $container->setDefinition(
-            'flysystem.cache.memory',
-            (new Definition(Memory::class))
-                ->setPrivate(true)
-        );
-
-        $container->setDefinition(
-            'flysystem.cache.app',
-            (new Definition(Psr6Cache::class))
-                ->setPrivate(true)
-                ->setArgument(0, new Reference('cache.app', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE))
-        );
+        $container->setAlias(FilesystemInterface::class, $config['default_filesystem'])->setPublic(false);
+        $container->setAlias('flysystem', $config['default_filesystem'])->setPublic(false);
     }
 
     private function registerFilesystems(ContainerBuilder $container, array $filesystems)
     {
-        $definitionFactory = new AdapterDefinitionFactory();
+        $adapterFactory = new AdapterDefinitionFactory();
 
         foreach ($filesystems as $fsName => $fsConfig) {
-            if ($fsConfig['adapter'] && $fsConfig['mounts']) {
-                throw new \LogicException('Definition of the filesystem "'.$fsName.'" is invalid: configuring both "adapter" and "mounts" keys is not allowed.');
-            }
-
-            if ($fsConfig['adapter']) {
-                $adapterDefinition = $definitionFactory->createDefinition($fsConfig['adapter'], $fsConfig['options']);
-            } elseif ($fsConfig['mounts']) {
-                $adapterDefinition = $definitionFactory->createDefinition('mount', $fsConfig['mounts']);
+            // Create adapter service definition
+            if ($adapter = $adapterFactory->createDefinition($fsConfig['adapter'], $fsConfig['options'])) {
+                // Native adapter
+                $container->setDefinition('flysystem.adapter.'.$fsName, $adapter)->setPublic(false);
             } else {
-                throw new \LogicException('Definition of the filesystem "'.$fsName.'" is invalid: one of the "adapter" and "mounts" keys is required.');
+                // Custom adapter
+                $container->setAlias('flysystem.adapter.'.$fsName, $fsConfig['adapter'])->setPublic(false);
             }
+
+            // Create filesystem service definition
+            $definition = $this->createFilesystemDefinition(new Reference('flysystem.adapter.'.$fsName), $fsConfig);
+
+            $container->setDefinition($fsName, $definition);
+            $container->registerAliasForArgument($fsName, FilesystemInterface::class, $fsName)->setPublic(false);
         }
     }
+
+    private function createFilesystemDefinition(Reference $adapter, array $config)
+    {
+        $definition = new Definition(Filesystem::class);
+        $definition->setPublic(false);
+        $definition->setArgument(0, $adapter);
+        $definition->setArgument(1, [
+            'visibility' => $config['visibility'],
+            'case_sensitive' => $config['case_sensitive'],
+            'disable_asserts' => $config['disable_asserts'],
+        ]);
+
+        return $definition;
+    }
 }

src/DependencyInjection/FlysystemExtension.php

@@ -20,7 +20,7 @@ class AdapterDefinitionFactoryTest extends TestCase
 {
     public function provideConfigOptions()
     {
-        $config = Yaml::parseFile(__DIR__.'/Fixtures/full_config.yaml');
+        $config = Yaml::parseFile(__DIR__.'/options.yaml');
 
         foreach ($config as $fs) {
             yield $fs['adapter'] => [$fs['adapter'], $fs['options']];