From 5ec87ec62e416bf722344d285a0042e99a47fdaf Mon Sep 17 00:00:00 2001
From: Matthew Turland <tobias382@gmail.com>
Date: Mon, 8 Jan 2024 20:58:27 -0600
Subject: [PATCH] Add buffer factory, refactor StreamWrapper DI into
 PhpStreamWrapper, remove ServiceLocator

---
 src/BufferFactory.php                         | 33 ++++++++++++++
 src/BufferFactoryInterface.php                |  8 ++++
 src/Container.php                             |  5 +--
 src/FilesystemRegistry.php                    |  2 +-
 src/FlystreamException.php                    | 25 +++++++++++
 src/OverflowBuffer.php                        |  5 +++
 src/PhpStreamWrapper.php                      | 27 ++++++++++++
 src/ServiceLocator.php                        | 38 ----------------
 src/StreamWrapper.php                         | 43 ++++++++-----------
 tests/BufferFactoryTest.php                   | 38 ++++++++++++++++
 tests/ContainerTest.php                       | 26 +++++------
 ...apperTest.php => PhpStreamWrapperTest.php} | 15 ++++---
 tests/ServiceLocatorTest.php                  | 36 ----------------
 13 files changed, 178 insertions(+), 123 deletions(-)
 create mode 100644 src/BufferFactory.php
 create mode 100644 src/BufferFactoryInterface.php
 create mode 100644 src/PhpStreamWrapper.php
 delete mode 100644 src/ServiceLocator.php
 create mode 100644 tests/BufferFactoryTest.php
 rename tests/{StreamWrapperTest.php => PhpStreamWrapperTest.php} (95%)
 delete mode 100644 tests/ServiceLocatorTest.php

diff --git a/src/BufferFactory.php b/src/BufferFactory.php
new file mode 100644
index 0000000..7e37fa6
--- /dev/null
+++ b/src/BufferFactory.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace Elazar\Flystream;
+
+class BufferFactory implements BufferFactoryInterface
+{
+    private function __construct(
+        private BufferInterface|string $bufferInstanceOrClassFqcn
+    ) { }
+
+    public static function fromInstance(BufferInterface $instance)
+    {
+        return new static($instance);
+    }
+
+    public static function fromClass(string $classFqcn)
+    {
+        if (!class_exists($classFqcn)) {
+            throw FlystreamException::bufferClassNotFound($classFqcn);
+        }
+        if (!is_a($classFqcn, BufferInterface::class, true)) {
+            throw FlystreamException::bufferClassMissingInterface($classFqcn);
+        }
+        return new static($classFqcn);
+    }
+
+    public function createBuffer(): BufferInterface
+    {
+        return $this->bufferInstanceOrClassFqcn instanceof BufferInterface
+            ? clone $this->bufferInstanceOrClassFqcn
+            : new $this->bufferInstanceOrClassFqcn;
+    }
+}
diff --git a/src/BufferFactoryInterface.php b/src/BufferFactoryInterface.php
new file mode 100644
index 0000000..3823190
--- /dev/null
+++ b/src/BufferFactoryInterface.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace Elazar\Flystream;
+
+interface BufferFactoryInterface
+{
+    public function createBuffer(): BufferInterface;
+}
diff --git a/src/Container.php b/src/Container.php
index 0d29833..442676b 100644
--- a/src/Container.php
+++ b/src/Container.php
@@ -41,10 +41,7 @@ public function __construct()
             LocalLockRegistry::class => fn() => new LocalLockRegistry,
             NullLogger::class => fn() => new NullLogger,
             LoggerInterface::class => fn() => $this->get(NullLogger::class),
-            BufferInterface::class => fn() => $this->get(MemoryBuffer::class),
-            MemoryBuffer::class => fn() => new MemoryBuffer,
-            OverflowBuffer::class => fn() => new OverflowBuffer,
-            FileBuffer::class => fn() => new FileBuffer,
+            BufferFactoryInterface::class => fn() => BufferFactory::fromClass(MemoryBuffer::class),
         ];
 
         $this->instances = [];
diff --git a/src/FilesystemRegistry.php b/src/FilesystemRegistry.php
index 666b2b6..26476a2 100644
--- a/src/FilesystemRegistry.php
+++ b/src/FilesystemRegistry.php
@@ -24,7 +24,7 @@ public function register(
             throw FlystreamException::protocolRegistered($protocol);
         }
         $this->filesystems[$protocol] = $filesystem;
-        stream_wrapper_register($protocol, StreamWrapper::class);
+        stream_wrapper_register($protocol, PhpStreamWrapper::class);
     }
 
     public function unregister(
diff --git a/src/FlystreamException.php b/src/FlystreamException.php
index e43b2ab..bb6386f 100644
--- a/src/FlystreamException.php
+++ b/src/FlystreamException.php
@@ -9,6 +9,8 @@ class FlystreamException extends \RuntimeException
     public const CODE_PROTOCOL_REGISTERED = 1;
     public const CODE_PROTOCOL_NOT_REGISTERED = 2;
     public const CODE_CONTAINER_ENTRY_NOT_FOUND = 3;
+    public const CODE_BUFFER_CLASS_NOT_FOUND = 4;
+    public const CODE_BUFFER_CLASS_MISSING_INTERFACE = 5;
 
     public static function protocolRegistered(string $protocol): self
     {
@@ -42,4 +44,27 @@ public static function containerEntryNotFound(string $id): self
             self::CODE_CONTAINER_ENTRY_NOT_FOUND
         ) extends FlystreamException implements NotFoundExceptionInterface { };
     }
+
+    public static function bufferClassNotFound(string $bufferClassFqcn): self
+    {
+        return new self(
+            sprintf(
+                'Specified buffer class not found: %s',
+                $bufferClassFqcn
+            ),
+            self::CODE_BUFFER_CLASS_NOT_FOUND
+        );
+    }
+
+    public static function bufferClassMissingInterface(string $bufferClassFqcn): self
+    {
+        return new self(
+            sprintf(
+                'Specified buffer class does not implement %s: %s',
+                BufferInterface::class,
+                $bufferClassFqcn
+            ),
+            self::CODE_BUFFER_CLASS_MISSING_INTERFACE
+        );
+    }
 }
diff --git a/src/OverflowBuffer.php b/src/OverflowBuffer.php
index a4f3012..c7f78c0 100644
--- a/src/OverflowBuffer.php
+++ b/src/OverflowBuffer.php
@@ -36,4 +36,9 @@ public function setMaxMemory(int $maxMemory): void
     {
         $this->maxMemory = $maxMemory;
     }
+
+    public function getMaxMemory(): ?int
+    {
+        return $this->maxMemory;
+    }
 }
diff --git a/src/PhpStreamWrapper.php b/src/PhpStreamWrapper.php
new file mode 100644
index 0000000..0a08786
--- /dev/null
+++ b/src/PhpStreamWrapper.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace Elazar\Flystream;
+
+use League\Flysystem\UnixVisibility\VisibilityConverter;
+use Psr\Log\LoggerInterface;
+
+class PhpStreamWrapper extends StreamWrapper
+{
+    private static Container $container;
+
+    public static function setContainer(Container $container): void
+    {
+        static::$container = $container;
+    }
+    
+    public function __construct()
+    {
+        parent::__construct(
+            static::$container->get(VisibilityConverter::class),
+            static::$container->get(FilesystemRegistry::class),
+            static::$container->get(LockRegistryInterface::class),
+            static::$container->get(BufferFactoryInterface::class),
+            static::$container->get(LoggerInterface::class)
+        );
+    }
+}
diff --git a/src/ServiceLocator.php b/src/ServiceLocator.php
deleted file mode 100644
index 0d7e7da..0000000
--- a/src/ServiceLocator.php
+++ /dev/null
@@ -1,38 +0,0 @@
-<?php
-
-namespace Elazar\Flystream;
-
-class ServiceLocator
-{
-    private static ?self $instance = null;
-
-    private Container $container;
-
-    public function __construct()
-    {
-        $this->container = new Container;
-    }
-
-    public static function getInstance(): self
-    {
-        if (self::$instance === null) {
-            self::$instance = new self;
-        }
-        return self::$instance;
-    }
-
-    public static function get(string $class): object
-    {
-        return self::getInstance()->container->get($class);
-    }
-
-    public static function set(string $class, string|object $instanceOrClass): void
-    {
-        self::getInstance()->container->set($class, $instanceOrClass);
-    }
-
-    public static function setInstance(self $instance): void
-    {
-        self::$instance = $instance;
-    }
-}
diff --git a/src/StreamWrapper.php b/src/StreamWrapper.php
index 51dc5d1..8672dd7 100644
--- a/src/StreamWrapper.php
+++ b/src/StreamWrapper.php
@@ -9,15 +9,11 @@
 use League\Flysystem\FilesystemOperator;
 use League\Flysystem\UnableToWriteFile;
 use League\Flysystem\UnixVisibility\VisibilityConverter;
-use Pimple\Container;
-use Psr\Http\Message\StreamInterface;
 use Psr\Log\LoggerInterface;
 use Throwable;
 
 class StreamWrapper
 {
-    private ?Lock $lock = null;
-
     private ?string $path = null;
 
     private ?string $mode = null;
@@ -29,11 +25,21 @@ class StreamWrapper
      */
     private $read = null;
 
-    private ?BufferInterface $buffer = null;
-
     /** @var resource */
     public $context;
 
+    private ?Lock $lock = null;
+
+    private ?BufferInterface $buffer = null;
+
+    public function __construct(
+        private VisibilityConverter $visibilityConverter,
+        private FilesystemRegistry $filesystemRegistry,
+        private LockRegistryInterface $lockRegistry,
+        private BufferFactoryInterface $bufferFactory,
+        private LoggerInterface $logger,
+    ) { }
+
     public function dir_closedir(): bool
     {
         $this->log('info', __METHOD__);
@@ -86,12 +92,11 @@ public function dir_rewinddir(): bool
     public function mkdir(string $path, int $mode, int $options): bool
     {
         $this->log('info', __METHOD__, func_get_args());
-        $visibility = $this->get(VisibilityConverter::class);
         $filesystem = $this->getFilesystem($path);
         try {
             $config = $this->getConfig($path, [
                 Config::OPTION_DIRECTORY_VISIBILITY =>
-                    $visibility->inverseForDirectory($mode),
+                    $this->visibilityConverter->inverseForDirectory($mode),
             ]);
             $filesystem->createDirectory($path, $config);
             return true;
@@ -192,8 +197,6 @@ public function stream_lock(int $operation): bool
     {
         $this->log('info', __METHOD__, func_get_args());
 
-        $locks = $this->get(LockRegistryInterface::class);
-
         // For now, ignore non-blocking requests
         $operation &= ~LOCK_NB;
 
@@ -204,14 +207,14 @@ public function stream_lock(int $operation): bool
                 ? Lock::TYPE_SHARED
                 : Lock::TYPE_EXCLUSIVE;
             $lock = new Lock($this->path, $type);
-            $result = $locks->acquire($lock);
+            $result = $this->lockRegistry->acquire($lock);
             if ($result) {
                 $this->lock = $lock;
             }
             return $result;
         }
 
-        $result = $locks->release($this->lock);
+        $result = $this->lockRegistry->release($this->lock);
         if ($result) {
             $this->lock = null;
         }
@@ -331,7 +334,7 @@ public function stream_write(string $data)
                 );
             }
             if ($this->buffer === null) {
-                $this->buffer = $this->get(BufferInterface::class);
+                $this->buffer = $this->bufferFactory->createBuffer();
             }
             return $this->buffer->write($data);
         } catch (Throwable $e) {
@@ -369,13 +372,12 @@ public function url_stat(string $path, int $flags)
         $this->log('info', __METHOD__, func_get_args());
 
         $filesystem = $this->getFilesystem($path);
-        $visibility = $this->get(VisibilityConverter::class);
 
         if (!$filesystem->fileExists($path)) {
             return false;
         }
 
-        $mode = 0100000 | $visibility->forFile(
+        $mode = 0100000 | $this->visibilityConverter->forFile(
             $filesystem->visibility($path)
         );
         $size = $filesystem->fileSize($path);
@@ -412,13 +414,7 @@ private function getConfig(string $path, array $overrides = []): array
     private function getFilesystem(string $path): FilesystemOperator
     {
         $protocol = parse_url($path, PHP_URL_SCHEME);
-        $registry = $this->get(FilesystemRegistry::class);
-        return $registry->get($protocol);
-    }
-
-    private function get(string $key)
-    {
-        return ServiceLocator::get($key);
+        return $this->filesystemRegistry->get($protocol);
     }
 
     private function getDir(string $path): Iterator
@@ -451,7 +447,6 @@ private function log(
         string $message,
         array $context = []
     ): void {
-        $logger = $this->get(LoggerInterface::class);
-        $logger->log($level, $message, $context);
+        $this->logger->log($level, $message, $context);
     }
 }
diff --git a/tests/BufferFactoryTest.php b/tests/BufferFactoryTest.php
new file mode 100644
index 0000000..d293e92
--- /dev/null
+++ b/tests/BufferFactoryTest.php
@@ -0,0 +1,38 @@
+<?php
+
+use Elazar\Flystream\BufferFactory;
+use Elazar\Flystream\FlystreamException;
+use Elazar\Flystream\OverflowBuffer;
+
+it('creates a buffer from a buffer instance', function () {
+    $maxMemory = 1024 ** 2;
+    $instance = new OverflowBuffer($maxMemory);
+    $factory = BufferFactory::fromInstance($instance);
+    $buffer = $factory->createBuffer();
+    expect($buffer)
+        ->toBeInstanceOf(OverflowBuffer::class)
+        ->not->toBe($instance);
+    expect($buffer->getMaxMemory())->toBe($maxMemory);
+});
+
+it('creates a buffer from a class', function () {
+    $factory = BufferFactory::fromClass(OverflowBuffer::class);
+    $buffer = $factory->createBuffer();
+    expect($buffer)->toBeInstanceOf(OverflowBuffer::class);
+});
+
+it('does not create a buffer from a nonexistent class', function () {
+    BufferFactory::fromClass('NonexistentClass');
+})->throws(
+    FlystreamException::class,
+    null,
+    FlystreamException::CODE_BUFFER_CLASS_NOT_FOUND
+);
+
+it('does not create a buffer from a non-buffer class', function () {
+    BufferFactory::fromClass('stdClass');
+})->throws(
+    FlystreamException::class,
+    null,
+    FlystreamException::CODE_BUFFER_CLASS_MISSING_INTERFACE
+);
diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php
index c75968c..6b31d1f 100644
--- a/tests/ContainerTest.php
+++ b/tests/ContainerTest.php
@@ -1,14 +1,14 @@
 <?php
 
-use Elazar\Flystream\BufferInterface;
 use Elazar\Flystream\Container;
-use Elazar\Flystream\FileBuffer;
 use Elazar\Flystream\FlystreamException;
-use Elazar\Flystream\MemoryBuffer;
+use Elazar\Flystream\PassThruPathNormalizer;
+use Elazar\Flystream\StripProtocolPathNormalizer;
+use League\Flysystem\PathNormalizer;
 
 it('can iterate, detect, and get default entries', function () {
     $container = new Container;
-    $expectedDependencyCount = 15;
+    $expectedDependencyCount = 12;
     $actualDependencyCount = 0;
     foreach ($container as $class => $instance) {
         $actualDependencyCount++;
@@ -25,21 +25,21 @@
 it('can override a dependency using a class name', function () {
     $container = new Container;
 
-    $default = $container->get(BufferInterface::class);
-    expect($default)->toBeInstanceOf(MemoryBuffer::class);
+    $default = $container->get(PathNormalizer::class);
+    expect($default)->toBeInstanceOf(StripProtocolPathNormalizer::class);
 
-    $container->set(BufferInterface::class, FileBuffer::class);
+    $container->set(PathNormalizer::class, PassThruPathNormalizer::class);
 
-    $override = $container->get(BufferInterface::class);
-    expect($override)->toBeInstanceOf(FileBuffer::class);
+    $override = $container->get(PathNormalizer::class);
+    expect($override)->toBeInstanceOf(PassThruPathNormalizer::class);
 });
 
 it('can override a dependency using an instance', function () {
     $container = new Container;
 
-    $buffer = new FileBuffer;
-    $container->set(BufferInterface::class, $buffer);
+    $override = new PassThruPathNormalizer;
+    $container->set(PathNormalizer::class, $override);
 
-    $override = $container->get(BufferInterface::class);
-    expect($override)->toBe($buffer);
+    $actual = $container->get(PathNormalizer::class);
+    expect($actual)->toBe($override);
 });
diff --git a/tests/StreamWrapperTest.php b/tests/PhpStreamWrapperTest.php
similarity index 95%
rename from tests/StreamWrapperTest.php
rename to tests/PhpStreamWrapperTest.php
index dd3ac7c..97939a0 100644
--- a/tests/StreamWrapperTest.php
+++ b/tests/PhpStreamWrapperTest.php
@@ -1,8 +1,8 @@
 <?php
 
+use Elazar\Flystream\Container;
 use Elazar\Flystream\FilesystemRegistry;
-use Elazar\Flystream\ServiceLocator;
-use League\Flysystem\FileAttributes;
+use Elazar\Flystream\PhpStreamWrapper;
 use League\Flysystem\Filesystem;
 use League\Flysystem\InMemory\InMemoryFilesystemAdapter;
 use League\Flysystem\PathNormalizer;
@@ -11,14 +11,15 @@
 use Psr\Log\LoggerInterface;
 
 beforeEach(function () {
-    $serviceLocator = new ServiceLocator();
-    ServiceLocator::setInstance($serviceLocator);
+    $this->container = new Container();
 
     $this->logger = new Logger(__FILE__);
     $this->logger->pushHandler(new TestHandler);
-    ServiceLocator::set(LoggerInterface::class, $this->logger);
+    $this->container->set(LoggerInterface::class, $this->logger);
 
-    $this->registry = ServiceLocator::get(FilesystemRegistry::class);
+    PhpStreamWrapper::setContainer($this->container);
+
+    $this->registry = $this->container->get(FilesystemRegistry::class);
 
     $this->filesystem = new Filesystem(new InMemoryFilesystemAdapter);
     $this->registry->register('fly', $this->filesystem);
@@ -266,7 +267,7 @@
     $this->filesystem = new Filesystem(
         new InMemoryFilesystemAdapter(),
         [],
-        ServiceLocator::get(PathNormalizer::class),
+        $this->container->get(PathNormalizer::class),
     );
     $this->registry->register('mem', $this->filesystem);
 
diff --git a/tests/ServiceLocatorTest.php b/tests/ServiceLocatorTest.php
deleted file mode 100644
index 33ce875..0000000
--- a/tests/ServiceLocatorTest.php
+++ /dev/null
@@ -1,36 +0,0 @@
-<?php
-
-use Elazar\Flystream\FilesystemRegistry;
-use Elazar\Flystream\ServiceLocator;
-use League\Flysystem\PathNormalizer;
-use League\Flysystem\WhitespacePathNormalizer;
-
-it('initializes an instance on initial access', function () {
-    $locator = ServiceLocator::getInstance();
-    expect($locator)->toBeInstanceOf(ServiceLocator::class);
-});
-
-it('allows an instance to be set', function () {
-    $expected = new ServiceLocator();
-    ServiceLocator::setInstance($expected);
-    expect(ServiceLocator::getInstance())->toBe($expected);
-});
-
-it('provides a static accessor for dependencies', function () {
-    $registry = ServiceLocator::get(FilesystemRegistry::class);
-    expect($registry)->toBeInstanceOf(FilesystemRegistry::class);
-});
-
-it('supports static injection of dependencies', function () {
-    $locator = new ServiceLocator();
-    ServiceLocator::setInstance($locator);
-
-    $expected = new FilesystemRegistry();
-    ServiceLocator::set(FilesystemRegistry::class, $expected);
-    $actual = ServiceLocator::get(FilesystemRegistry::class);
-    expect($actual)->toBe($expected);
-
-    ServiceLocator::set(PathNormalizer::class, WhitespacePathNormalizer::class);
-    $actual = ServiceLocator::get(PathNormalizer::class);
-    expect($actual)->toBeInstanceOf(WhitespacePathNormalizer::class);
-});