Simplify instantiator logic

Author: curry684

src/DataTable.php

@@ -108,7 +108,7 @@ class DataTable
      */
     public function __construct(array $options = [], Instantiator $instantiator = null)
     {
-        $this->instantiator = $instantiator;
+        $this->instantiator = $instantiator ?? new Instantiator();
 
         $resolver = new OptionsResolver();
         $this->configureOptions($resolver);
@@ -128,7 +128,7 @@ public function add(string $name, string $type, array $options = [])
             throw new InvalidArgumentException(sprintf("There already is a column with name '%s'", $name));
         }
 
-        $column = $this->createColumn($type);
+        $column = $this->instantiator->getColumn($type);
         $column->initialize($name, count($this->columns), $options, $this);
 
         $this->columns[] = $column;
@@ -158,28 +158,7 @@ public function addOrderBy($column, string $direction = self::SORT_ASCENDING)
      */
     public function createAdapter(string $adapter, array $options = []): self
     {
-        if (null !== $this->instantiator && $instance = $this->instantiator->getAdapter($adapter)) {
-            return $this->setAdapter($instance, $options);
-        } elseif (class_exists($adapter) && in_array(AdapterInterface::class, class_implements($adapter), true)) {
-            return $this->setAdapter(new $adapter(), $options);
-        } else {
-            throw new InvalidArgumentException(sprintf('Could not resolve adapter type "%s" to a service or class implementing AdapterInterface', $adapter));
-        }
-    }
-
-    /**
-     * @param string $column
-     * @return AbstractColumn
-     */
-    protected function createColumn(string $column): AbstractColumn
-    {
-        if (null !== $this->instantiator && $instance = $this->instantiator->getColumn($column)) {
-            return $instance;
-        } elseif (class_exists($column) && is_subclass_of($column, AbstractColumn::class)) {
-            return new $column();
-        } else {
-            throw new InvalidArgumentException(sprintf('Could not resolve column type "%s" to a service or class extending AbstractColumn', $column));
-        }
+        return $this->setAdapter($this->instantiator->getAdapter($adapter), $options);
     }
 
     /**

src/DataTableFactory.php

@@ -13,7 +13,6 @@
 namespace Omines\DataTablesBundle;
 
 use Omines\DataTablesBundle\DependencyInjection\Instantiator;
-use Omines\DataTablesBundle\Exception\InvalidArgumentException;
 use Symfony\Component\HttpFoundation\Request;
 
 class DataTableFactory
@@ -36,17 +35,10 @@ class DataTableFactory
      * @param array $config
      * @param DataTableRendererInterface $renderer
      */
-    public function __construct(array $config, DataTableRendererInterface $renderer)
+    public function __construct(array $config, DataTableRendererInterface $renderer, Instantiator $instantiator)
     {
         $this->config = $config;
         $this->renderer = $renderer;
-    }
-
-    /**
-     * @param Instantiator $instantiator
-     */
-    public function setInstantiator(Instantiator $instantiator)
-    {
         $this->instantiator = $instantiator;
     }
 
@@ -83,29 +75,12 @@ public function createFromType($type, array $typeOptions = [], array $options =
             if (isset($this->resolvedTypes[$name])) {
                 $type = $this->resolvedTypes[$name];
             } else {
-                $this->resolvedTypes[$name] = $type = $this->resolveType($name);
+                $this->resolvedTypes[$name] = $type = $this->instantiator->getType($name);
             }
         }
 
         $type->configure($dataTable, $typeOptions);
 
         return $dataTable;
     }
-
-    /**
-     * Resolves a dynamic type to an instance via services or instantiation.
-     *
-     * @param string $type
-     * @return DataTableTypeInterface
-     */
-    private function resolveType(string $type): DataTableTypeInterface
-    {
-        if (null !== $this->instantiator && $type = $this->instantiator->getType($type)) {
-            return $type;
-        } elseif (class_exists($type) && in_array(DataTableTypeInterface::class, class_implements($type), true)) {
-            return new $type();
-        }
-
-        throw new InvalidArgumentException(sprintf('Could not resolve type "%s" to a service or class', $type));
-    }
 }

src/DependencyInjection/Compiler/LocatorRegistrationPass.php

@@ -12,7 +12,9 @@
 
 namespace Omines\DataTablesBundle\DependencyInjection\Compiler;
 
-use Omines\DataTablesBundle\DataTableFactory;
+use Omines\DataTablesBundle\Adapter\AdapterInterface;
+use Omines\DataTablesBundle\Column\AbstractColumn;
+use Omines\DataTablesBundle\DataTableTypeInterface;
 use Omines\DataTablesBundle\DependencyInjection\Instantiator;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -32,17 +34,12 @@ class LocatorRegistrationPass implements CompilerPassInterface
      */
     public function process(ContainerBuilder $container)
     {
-        $instantiator = $container->getDefinition(Instantiator::class);
-
-        $instantiator->setArguments([
-            $this->registerLocator($container, 'adapter'),
-            $this->registerLocator($container, 'column'),
-            $this->registerLocator($container, 'type'),
-        ]);
-
-        $container->getDefinition(DataTableFactory::class)
-            ->addMethodCall('setInstantiator', [$instantiator])
-        ;
+        $container->getDefinition(Instantiator::class)
+            ->setArguments([[
+                AdapterInterface::class => $this->registerLocator($container, 'adapter'),
+                AbstractColumn::class => $this->registerLocator($container, 'column'),
+                DataTableTypeInterface::class => $this->registerLocator($container, 'type'),
+            ]]);
     }
 
     /**
@@ -60,6 +57,7 @@ private function registerLocator(ContainerBuilder $container, string $baseTag):
         return $container
             ->register("datatables.{$baseTag}_locator", ServiceLocator::class)
             ->addTag('container.service_locator')
+            ->setPrivate(true)
             ->setArguments([$types])
         ;
     }

src/DependencyInjection/Instantiator.php

@@ -25,27 +25,17 @@
  */
 class Instantiator
 {
-    /** @var ServiceLocator */
-    private $adapterLocator;
-
-    /** @var ServiceLocator */
-    private $columnLocator;
-
-    /** @var ServiceLocator */
-    private $typeLocator;
+    /** @var ServiceLocator[] */
+    private $locators;
 
     /**
      * Instantiator constructor.
      *
-     * @param ServiceLocator $adapterLocator
-     * @param ServiceLocator $columnLocator
-     * @param ServiceLocator $typeLocator
+     * @param ServiceLocator[] $locators
      */
-    public function __construct(ServiceLocator $adapterLocator, ServiceLocator $columnLocator, ServiceLocator $typeLocator)
+    public function __construct(array $locators = [])
     {
-        $this->adapterLocator = $adapterLocator;
-        $this->columnLocator = $columnLocator;
-        $this->typeLocator = $typeLocator;
+        $this->locators = $locators;
     }
 
     /**
@@ -54,7 +44,7 @@ public function __construct(ServiceLocator $adapterLocator, ServiceLocator $colu
      */
     public function getAdapter(string $type): AdapterInterface
     {
-        return $this->getInstance($this->adapterLocator, $type, AdapterInterface::class);
+        return $this->getInstance($type, AdapterInterface::class);
     }
 
     /**
@@ -63,7 +53,7 @@ public function getAdapter(string $type): AdapterInterface
      */
     public function getColumn(string $type): AbstractColumn
     {
-        return $this->getInstance($this->columnLocator, $type, AbstractColumn::class);
+        return $this->getInstance($type, AbstractColumn::class);
     }
 
     /**
@@ -72,19 +62,18 @@ public function getColumn(string $type): AbstractColumn
      */
     public function getType(string $type): DataTableTypeInterface
     {
-        return $this->getInstance($this->typeLocator, $type, DataTableTypeInterface::class);
+        return $this->getInstance($type, DataTableTypeInterface::class);
     }
 
     /**
-     * @param ServiceLocator $locator
      * @param string $type
      * @param string $baseType
      * @return mixed
      */
-    private function getInstance(ServiceLocator $locator, string $type, string $baseType)
+    private function getInstance(string $type, string $baseType)
     {
-        if ($locator->has($type)) {
-            return $locator->get($type);
+        if (isset($this->locators[$baseType]) && $this->locators[$baseType]->has($type)) {
+            return $this->locators[$baseType]->get($type);
         } elseif (class_exists($type) && is_subclass_of($type, $baseType)) {
             return new $type();
         }

src/Resources/config/services.yaml

@@ -13,7 +13,7 @@ services:
         arguments: ['@?twig']
 
     Omines\DataTablesBundle\DataTableFactory:
-        arguments: ['%datatables.config%', '@datatables.renderer']
+        arguments: ['%datatables.config%', '@datatables.renderer', '@Omines\DataTablesBundle\DependencyInjection\Instantiator']
         public: true
 
     Omines\DataTablesBundle\DependencyInjection\Instantiator: ~

tests/Unit/DataTableTest.php

@@ -49,7 +49,7 @@ public function testBundle()
 
     public function testFactory()
     {
-        $factory = new DataTableFactory(['language_from_cdn' => false], $this->createMock(TwigRenderer::class));
+        $factory = new DataTableFactory(['language_from_cdn' => false], $this->createMock(TwigRenderer::class), new Instantiator());
 
         $table = $factory->create(['pageLength' => 684, 'dom' => 'bar']);
         $this->assertSame(684, $table->getOption('pageLength'));
@@ -63,7 +63,7 @@ public function testFactory()
 
     public function testFactoryRemembersInstances()
     {
-        $factory = new DataTableFactory([], $this->createMock(TwigRenderer::class));
+        $factory = new DataTableFactory([], $this->createMock(TwigRenderer::class), new Instantiator());
 
         $reflection = new \ReflectionClass(DataTableFactory::class);
         $property = $reflection->getProperty('resolvedTypes');
@@ -119,8 +119,7 @@ public function testFactoryFailsOnInvalidType()
         $container = new ContainerBuilder();
         (new DataTablesExtension())->load([], $container);
 
-        $factory = new DataTableFactory($container->getParameter('datatables.config'), $this->createMock(TwigRenderer::class));
-        $factory->setInstantiator(new Instantiator($dummy, $dummy, $dummy));
+        $factory = new DataTableFactory($container->getParameter('datatables.config'), $this->createMock(TwigRenderer::class), new Instantiator());
         $factory->createFromType('foobar');
     }
 
@@ -162,7 +161,7 @@ public function testDuplicateColumnNameThrows()
 
     /**
      * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage Could not resolve adapter type
+     * @expectedExceptionMessage Could not resolve type "foo\bar" to a service or class, are you missing a use statement? Or is it implemented but does it not correctly derive from "Omines\DataTablesBundle\Adapter\AdapterInterface"?
      */
     public function testInvalidAdapterThrows()
     {
@@ -173,7 +172,7 @@ public function testInvalidAdapterThrows()
 
     /**
      * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage Could not resolve column type "bar"
+     * @expectedExceptionMessage Could not resolve type "bar" to a service or class, are you missing a use statement? Or is it implemented but does it not correctly derive from "Omines\DataTablesBundle\Column\AbstractColumn"?
      */
     public function testInvalidColumnThrows()
     {
@@ -231,7 +230,7 @@ public function testMissingStateThrows()
      */
     public function testInvalidDataTableTypeThrows()
     {
-        (new DataTableFactory([], $this->createMock(DataTableRendererInterface::class)))
+        (new DataTableFactory([], $this->createMock(DataTableRendererInterface::class), new Instantiator()))
             ->createFromType('foo');
     }
 }