Revert "use service locator for configured client strategy"

DependencyInjection/HttplugExtension.php

@@ -20,7 +20,6 @@
 use Symfony\Component\DependencyInjection\DefinitionDecorator;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
-use Symfony\Component\DependencyInjection\ServiceLocator;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 
 /**
@@ -385,14 +384,6 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
 
             return;
         }
-
-        if (!class_exists(ServiceLocator::class)) {
-            $container->getDefinition('httplug.strategy')->setArguments([
-                new Reference('service_container'),
-                in_array($httpClient, ['auto', null], true) ? 'httplug.auto_discovery.auto_discovered_client' : $httpClient,
-                in_array($asyncHttpClient, ['auto', null], true) ? 'httplug.auto_discovery.auto_discovered_async' : $asyncHttpClient,
-            ]);
-        }
     }
 
     /**

Discovery/ConfiguredClientsStrategy.php

@@ -6,8 +6,6 @@
 use Http\Client\HttpAsyncClient;
 use Http\Discovery\HttpClientDiscovery;
 use Http\Discovery\Strategy\DiscoveryStrategy;
-use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\DependencyInjection\ServiceLocator;
 use Symfony\Component\EventDispatcher\Event;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
@@ -20,47 +18,39 @@
 class ConfiguredClientsStrategy implements DiscoveryStrategy, EventSubscriberInterface
 {
     /**
-     * @var ServiceLocator|ContainerInterface
-     */
-    private static $locator;
-
-    /**
-     * @var string
+     * @var HttpClient
      */
     private static $client;
 
     /**
-     * @var string
+     * @var HttpAsyncClient
      */
     private static $asyncClient;
 
     /**
-     * @param ServiceLocator|ContainerInterface $locator
-     * @param string                            $client
-     * @param string                            $asyncClient
+     * @param HttpClient      $httpClient
+     * @param HttpAsyncClient $asyncClient
      */
-    public function __construct($locator, $client, $asyncClient)
+    public function __construct(HttpClient $httpClient = null, HttpAsyncClient $asyncClient = null)
     {
-        static::$locator = $locator;
-        static::$client = $client;
-        static::$asyncClient = $asyncClient;
+        self::$client = $httpClient;
+        self::$asyncClient = $asyncClient;
     }
 
     /**
      * {@inheritdoc}
      */
     public static function getCandidates($type)
     {
-        $locator = static::$locator;
-        if ($type === HttpClient::class && $locator->has(static::$client)) {
-            return [['class' => function () use ($locator) {
-                return $locator->get(static::$client);
+        if ($type === HttpClient::class && self::$client !== null) {
+            return [['class' => function () {
+                return self::$client;
             }]];
         }
 
-        if ($type === HttpAsyncClient::class && $locator->has(static::$asyncClient)) {
-            return [['class' => function () use ($locator) {
-                return $locator->get(static::$asyncClient);
+        if ($type === HttpAsyncClient::class && self::$asyncClient !== null) {
+            return [['class' => function () {
+                return self::$asyncClient;
             }]];
         }
 

Resources/config/services.xml

@@ -4,18 +4,9 @@
            xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
 
     <services>
-        <service id="httplug.strategy.locator" class="Symfony\Component\DependencyInjection\ServiceLocator" public="false">
-            <argument type="collection">
-                <argument key="httplug.auto_discovery.auto_discovered_client" type="service" id="httplug.auto_discovery.auto_discovered_client" />
-                <argument key="httplug.auto_discovery.auto_discovered_async" type="service" id="httplug.auto_discovery.auto_discovered_async" />
-            </argument>
-            <tag name="container.service_locator" />
-        </service>
         <service id="httplug.strategy" class="Http\HttplugBundle\Discovery\ConfiguredClientsStrategy">
-            <argument type="service" id="httplug.strategy.locator" />
-            <argument>httplug.auto_discovery.auto_discovered_client</argument>
-            <argument>httplug.auto_discovery.auto_discovered_async</argument>
-
+            <argument type="service" id="httplug.auto_discovery.auto_discovered_client" on-invalid="null"/>
+            <argument type="service" id="httplug.auto_discovery.auto_discovered_async" on-invalid="null"/>
             <tag name="kernel.event_subscriber"/>
         </service>
 

Tests/Functional/DiscoveredClientsTest.php

@@ -6,7 +6,6 @@
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\HttplugBundle\Collector\StackPlugin;
-use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
 use Nyholm\NSA;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
 
@@ -89,13 +88,10 @@ public function testForcedDiscovery()
         $this->assertFalse($container->has('httplug.auto_discovery.auto_discovered_async'));
         $this->assertTrue($container->has('httplug.strategy'));
 
-        $container->get('httplug.strategy');
+        $strategy = $container->get('httplug.strategy');
 
-        $httpClientCandidate = ConfiguredClientsStrategy::getCandidates(HttpClient::class)[0]['class'];
-        $httpAsyncClientCandidate = ConfiguredClientsStrategy::getCandidates(HttpAsyncClient::class)[0]['class'];
-
-        $this->assertEquals($container->get('httplug.client.acme'), $httpClientCandidate());
-        $this->assertEquals($container->get('httplug.client.acme'), $httpAsyncClientCandidate());
+        $this->assertEquals($container->get('httplug.client.acme'), NSA::getProperty($strategy, 'client'));
+        $this->assertEquals($container->get('httplug.client.acme'), NSA::getProperty($strategy, 'asyncClient'));
     }
 
     private function getContainer($debug, $environment = 'test')

Tests/Unit/Discovery/ConfiguredClientsStrategyTest.php

@@ -5,28 +5,14 @@
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
-use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\DependencyInjection\ServiceLocator;
 
 class ConfiguredClientsStrategyTest extends \PHPUnit_Framework_TestCase
 {
     public function testGetCandidates()
     {
         $httpClient = $this->getMockBuilder(HttpClient::class)->getMock();
         $httpAsyncClient = $this->getMockBuilder(HttpAsyncClient::class)->getMock();
-        $locator = $this->getLocatorMock();
-        $locator
-            ->expects($this->exactly(2))
-            ->method('has')
-            ->willReturn(true)
-        ;
-        $locator
-            ->expects($this->exactly(2))
-            ->method('get')
-            ->will($this->onConsecutiveCalls($httpClient, $httpAsyncClient))
-        ;
-
-        $strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
+        $strategy = new ConfiguredClientsStrategy($httpClient, $httpAsyncClient);
 
         $candidates = $strategy::getCandidates(HttpClient::class);
         $candidate = array_shift($candidates);
@@ -39,18 +25,7 @@ public function testGetCandidates()
 
     public function testGetCandidatesEmpty()
     {
-        $locator = $this->getLocatorMock();
-        $locator
-            ->expects($this->exactly(2))
-            ->method('has')
-            ->willReturn(false)
-        ;
-        $locator
-            ->expects($this->never())
-            ->method('get')
-        ;
-
-        $strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
+        $strategy = new ConfiguredClientsStrategy(null, null);
 
         $candidates = $strategy::getCandidates(HttpClient::class);
         $this->assertEquals([], $candidates);
@@ -62,23 +37,7 @@ public function testGetCandidatesEmpty()
     public function testGetCandidatesEmptyAsync()
     {
         $httpClient = $this->getMockBuilder(HttpClient::class)->getMock();
-
-        $locator = $this->getLocatorMock();
-        $locator
-            ->expects($this->exactly(2))
-            ->method('has')
-            ->willReturnMap([
-                ['httplug.auto_discovery.auto_discovered_client', true],
-                ['httplug.auto_discovery.auto_discovered_async', false],
-            ])
-        ;
-        $locator
-            ->expects($this->once())
-            ->method('get')
-            ->willReturn($httpClient)
-        ;
-
-        $strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
+        $strategy = new ConfiguredClientsStrategy($httpClient, null);
 
         $candidates = $strategy::getCandidates(HttpClient::class);
         $candidate = array_shift($candidates);
@@ -91,23 +50,7 @@ public function testGetCandidatesEmptyAsync()
     public function testGetCandidatesEmptySync()
     {
         $httpAsyncClient = $this->getMockBuilder(HttpAsyncClient::class)->getMock();
-
-        $locator = $this->getLocatorMock();
-        $locator
-            ->expects($this->exactly(2))
-            ->method('has')
-            ->willReturnMap([
-                ['httplug.auto_discovery.auto_discovered_client', false],
-                ['httplug.auto_discovery.auto_discovered_async', true],
-            ])
-        ;
-        $locator
-            ->expects($this->once())
-            ->method('get')
-            ->willReturn($httpAsyncClient)
-        ;
-
-        $strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
+        $strategy = new ConfiguredClientsStrategy(null, $httpAsyncClient);
 
         $candidates = $strategy::getCandidates(HttpClient::class);
         $this->assertEquals([], $candidates);
@@ -116,16 +59,4 @@ public function testGetCandidatesEmptySync()
         $candidate = array_shift($candidates);
         $this->assertEquals($httpAsyncClient, $candidate['class']());
     }
-
-    /**
-     * @return ContainerInterface|ServiceLocator
-     */
-    private function getLocatorMock()
-    {
-        if (class_exists(ServiceLocator::class)) {
-            return $this->getMockBuilder(ServiceLocator::class)->disableOriginalConstructor()->getMock();
-        }
-
-        return $this->getMockBuilder(ContainerInterface::class)->getMock();
-    }
 }