Remove discovery dependency, fixes #40, closes #54

POC how to not be dependent on discovery

Fix unit tests

Fix factory method

Remove duplicated service

Move discovery to compiler pass

Applied fixes from StyleCI

Fix puli executable

Use class constant

Fix puli command order

Fix lowest dependency issue

Fix HHVM tests

Improve tests

Author: sagikazarmark

.gitignore

@@ -5,3 +5,4 @@ composer.lock
 phpspec.yml
 phpunit.xml
 puli.json
+puli.phar

.travis.yml

@@ -34,6 +34,7 @@ before_install:
     - travis_retry composer self-update
 
 install:
+    - wget https://github.com/puli/cli/releases/download/1.0.0-beta10/puli.phar && chmod +x puli.phar
     - composer require symfony/symfony:${SYMFONY_VERSION} --no-update
     - travis_retry composer update ${COMPOSER_FLAGS} --prefer-source --no-interaction
 

DependencyInjection/Compiler/DiscoveryPass.php

@@ -0,0 +1,85 @@
+<?php
+
+namespace Http\HttplugBundle\DependencyInjection\Compiler;
+
+use Http\Client\HttpClient;
+use Http\HttplugBundle\HttplugFactory;
+use Http\Message\MessageFactory;
+use Http\Message\StreamFactory;
+use Http\Message\UriFactory;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+use Symfony\Component\DependencyInjection\Reference;
+
+/**
+ * Adds fallback and discovery services.
+ *
+ * @author Márk Sági-Kazár <mark.sagikazar@gmail.com>
+ */
+final class DiscoveryPass implements CompilerPassInterface
+{
+    /**
+     * Fallback services and classes.
+     *
+     * @var array
+     */
+    private $services = [
+        'client' => HttpClient::class,
+        'message_factory' => MessageFactory::class,
+        'uri_factory' => UriFactory::class,
+        'stream_factory' => StreamFactory::class,
+    ];
+
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        $useDiscovery = false;
+
+        foreach ($this->services as $service => $class) {
+            $serviceId = sprintf('httplug.%s.default', $service);
+
+            if (false === $container->has($serviceId)) {
+                // Register and create factory for the first time
+                if (false === $useDiscovery) {
+                    $this->registerFactory($container);
+
+                    $factory = [
+                        new Reference('httplug.factory'),
+                        'find',
+                    ];
+
+                    $useDiscovery = true;
+                }
+
+                $definition = $container->register($serviceId, $class);
+
+                $definition->setFactory($factory);
+                $definition->addArgument($class);
+            }
+        }
+    }
+
+    /**
+     * @param ContainerBuilder $container
+     *
+     * @throws RuntimeException
+     */
+    private function registerFactory(ContainerBuilder $container)
+    {
+        if (false === $container->has('puli.discovery')) {
+            throw new RuntimeException(
+                'You need to install puli/symfony-bundle or add configuration at httplug.classes in order to use this bundle. Refer to http://docs.php-http/en/latest/integrations/index.html'
+            );
+        }
+
+        $definition = $container->register('httplug.factory', HttplugFactory::class);
+
+        $definition
+            ->addArgument(new Reference('puli.discovery'))
+            ->setPublic(false)
+        ;
+    }
+}

DependencyInjection/HttplugExtension.php

@@ -33,7 +33,6 @@ public function load(array $configs, ContainerBuilder $container)
 
         $loader->load('services.xml');
         $loader->load('plugins.xml');
-        $loader->load('discovery.xml');
 
         $enabled = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug');
         if ($enabled) {
@@ -48,14 +47,14 @@ public function load(array $configs, ContainerBuilder $container)
 
         foreach ($config['classes'] as $service => $class) {
             if (!empty($class)) {
-                $container->removeDefinition(sprintf('httplug.%s.default', $service));
                 $container->register(sprintf('httplug.%s.default', $service), $class);
             }
         }
 
         foreach ($config['main_alias'] as $type => $id) {
             $container->setAlias(sprintf('httplug.%s', $type), $id);
         }
+
         $this->configurePlugins($container, $config['plugins']);
         $this->configureClients($container, $config);
     }
@@ -174,7 +173,6 @@ private function configurePluginByName($name, Definition $definition, array $con
 
     /**
      * @param ContainerBuilder $container
-     * @param Definition       $parent
      * @param array            $config
      */
     private function configureAuthentication(ContainerBuilder $container, array $config)

HttplugBundle.php

@@ -2,6 +2,8 @@
 
 namespace Http\HttplugBundle;
 
+use Http\HttplugBundle\DependencyInjection\Compiler\DiscoveryPass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 
 /**
@@ -10,4 +12,10 @@
  */
 class HttplugBundle extends Bundle
 {
+    public function build(ContainerBuilder $container)
+    {
+        parent::build($container);
+
+        $container->addCompilerPass(new DiscoveryPass());
+    }
 }

HttplugFactory.php

@@ -0,0 +1,107 @@
+<?php
+
+namespace Http\HttplugBundle;
+
+use Puli\Discovery\Api\Discovery;
+use Puli\Discovery\Binding\ClassBinding;
+use Webmozart\Expression\Expr;
+
+/**
+ * This class is a wrapper around Puli discovery.
+ *
+ * @author Márk Sági-Kazár <mark.sagikazar@gmail.com>
+ */
+final class HttplugFactory
+{
+    /**
+     * @var Discovery
+     */
+    private $discovery;
+
+    /**
+     * @param Discovery $discovery
+     */
+    public function __construct(Discovery $discovery)
+    {
+        $this->discovery = $discovery;
+    }
+
+    /**
+     * Creates a class of type.
+     *
+     * @param string $type
+     *
+     * @return object
+     */
+    public function find($type)
+    {
+        $class = $this->findOneByType($type);
+
+        // TODO: use doctrine instantiator?
+        return new $class();
+    }
+
+    /**
+     * Finds a class of type and resolves optional dependency conditions.
+     *
+     * @param string $type
+     *
+     * @return string
+     *
+     * @throws \RuntimeException if no class is found.
+     */
+    private function findOneByType($type)
+    {
+        /** @var ClassBinding[] $bindings */
+        $bindings = $this->discovery->findBindings(
+            $type,
+            Expr::isInstanceOf('Puli\Discovery\Binding\ClassBinding')
+        );
+
+        foreach ($bindings as $binding) {
+            if ($binding->hasParameterValue('depends')) {
+                $dependency = $binding->getParameterValue('depends');
+
+                if (false === $this->evaluateCondition($dependency)) {
+                    continue;
+                }
+            }
+
+            return $binding->getClassName();
+        }
+
+        throw new \RuntimeException(sprintf('Class binding of type "%s" is not found', $type));
+    }
+
+    /**
+     * Evaulates conditions to boolean.
+     *
+     * @param mixed $condition
+     *
+     * @return bool
+     *
+     * TODO: review this method
+     */
+    protected function evaluateCondition($condition)
+    {
+        if (is_string($condition)) {
+            // Should be extended for functions, extensions???
+            return class_exists($condition);
+        } elseif (is_callable($condition)) {
+            return $condition();
+        } elseif (is_bool($condition)) {
+            return $condition;
+        } elseif (is_array($condition)) {
+            $evaluatedCondition = true;
+
+            // Immediately stop execution if the condition is false
+            for ($i = 0; $i < count($condition) && false !== $evaluatedCondition; ++$i) {
+                $evaluatedCondition &= $this->evaluateCondition($condition[$i]);
+            }
+
+            return $evaluatedCondition;
+        }
+
+        return false;
+    }
+}

Resources/config/discovery.xml

@@ -10,10 +10,16 @@ class AppKernel extends Kernel
      */
     public function registerBundles()
     {
-        return [
+        $bundles =  [
             new \Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
             new \Http\HttplugBundle\HttplugBundle(),
         ];
+
+        if (false === defined('HHVM_VERSION')) {
+            $bundles[] = new \Puli\SymfonyBundle\PuliBundle();
+        }
+
+        return $bundles;
     }
 
     /**

Tests/Resources/app/AppKernel.php

@@ -0,0 +1,76 @@
+<?php
+
+namespace Http\HttplugBundle\Tests\Unit\DependencyInjection\Compiler;
+
+use Http\Client\HttpClient;
+use Http\HttplugBundle\DependencyInjection\Compiler\DiscoveryPass;
+use Http\HttplugBundle\HttplugFactory;
+use Http\Message\MessageFactory;
+use Http\Message\StreamFactory;
+use Http\Message\UriFactory;
+use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+/**
+ * @author Márk Sági-Kazár <mark.sagikazar@gmail.com>
+ */
+final class DiscoveryPassTest extends AbstractCompilerPassTestCase
+{
+    protected function registerCompilerPass(ContainerBuilder $container)
+    {
+        $container->addCompilerPass(new DiscoveryPass());
+    }
+
+    public function testDiscoveryFallbacks()
+    {
+        $this->setDefinition('puli.discovery', new Definition('Puli\Discovery\Api\Discovery'));
+
+        $this->compile();
+
+        $this->assertContainerBuilderHasService('httplug.factory', HttplugFactory::class);
+
+        $this->assertContainerBuilderHasService('httplug.client.default', HttpClient::class);
+        $this->assertContainerBuilderHasService('httplug.message_factory.default', MessageFactory::class);
+        $this->assertContainerBuilderHasService('httplug.uri_factory.default', UriFactory::class);
+        $this->assertContainerBuilderHasService('httplug.stream_factory.default', StreamFactory::class);
+    }
+
+    public function testDiscoveryPartialFallbacks()
+    {
+        $this->setDefinition('puli.discovery', new Definition('Puli\Discovery\Api\Discovery'));
+        $this->setDefinition('httplug.client.default', new Definition('Http\Adapter\Guzzle6\Client'));
+
+        $this->compile();
+
+        $this->assertContainerBuilderHasService('httplug.factory', HttplugFactory::class);
+
+        $this->assertContainerBuilderHasService('httplug.client.default', 'Http\Adapter\Guzzle6\Client');
+        $this->assertContainerBuilderHasService('httplug.message_factory.default', MessageFactory::class);
+        $this->assertContainerBuilderHasService('httplug.uri_factory.default', UriFactory::class);
+        $this->assertContainerBuilderHasService('httplug.stream_factory.default', StreamFactory::class);
+    }
+
+    public function testNoDiscoveryFallbacks()
+    {
+        $this->setDefinition('httplug.client.default', new Definition(HttpClient::class));
+        $this->setDefinition('httplug.message_factory.default', new Definition(MessageFactory::class));
+        $this->setDefinition('httplug.uri_factory.default', new Definition(UriFactory::class));
+        $this->setDefinition('httplug.stream_factory.default', new Definition(StreamFactory::class));
+
+        $this->compile();
+
+        $this->assertContainerBuilderNotHasService('httplug.factory');
+    }
+
+    /**
+     * Overridden test as we have dependencies in this compiler pass.
+     *
+     * @test
+     * @expectedException \RuntimeException
+     */
+    public function compilation_should_not_fail_with_empty_container()
+    {
+        $this->compile();
+    }
+}