Skip to content

Commit 227eee2

Browse files
fbourigaultfbourigault
authored
Merge pull request #189 from php-http/revert-185
Revert "use service locator for configured client strategy"
2 parents 5bb6fcb + 0018e18 commit 227eee2

File tree

5 files changed

+22
-123
lines changed

5 files changed

+22
-123
lines changed

‎DependencyInjection/HttplugExtension.php‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use Symfony\Component\DependencyInjection\DefinitionDecorator;
2121
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
2222
use Symfony\Component\DependencyInjection\Reference;
23-
use Symfony\Component\DependencyInjection\ServiceLocator;
2423
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
2524

2625
/**
@@ -385,14 +384,6 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
385384

386385
return;
387386
}
388-
389-
if (!class_exists(ServiceLocator::class)) {
390-
$container->getDefinition('httplug.strategy')->setArguments([
391-
new Reference('service_container'),
392-
in_array($httpClient, ['auto', null], true) ? 'httplug.auto_discovery.auto_discovered_client' : $httpClient,
393-
in_array($asyncHttpClient, ['auto', null], true) ? 'httplug.auto_discovery.auto_discovered_async' : $asyncHttpClient,
394-
]);
395-
}
396387
}
397388

398389
/**

‎Discovery/ConfiguredClientsStrategy.php‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
use Http\Client\HttpAsyncClient;
77
use Http\Discovery\HttpClientDiscovery;
88
use Http\Discovery\Strategy\DiscoveryStrategy;
9-
use Symfony\Component\DependencyInjection\ContainerInterface;
10-
use Symfony\Component\DependencyInjection\ServiceLocator;
119
use Symfony\Component\EventDispatcher\Event;
1210
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1311

@@ -20,47 +18,39 @@
2018
class ConfiguredClientsStrategy implements DiscoveryStrategy, EventSubscriberInterface
2119
{
2220
/**
23-
* @var ServiceLocator|ContainerInterface
24-
*/
25-
private static $locator;
26-
27-
/**
28-
* @var string
21+
* @var HttpClient
2922
*/
3023
private static $client;
3124

3225
/**
33-
* @var string
26+
* @var HttpAsyncClient
3427
*/
3528
private static $asyncClient;
3629

3730
/**
38-
* @param ServiceLocator|ContainerInterface $locator
39-
* @param string $client
40-
* @param string $asyncClient
31+
* @param HttpClient $httpClient
32+
* @param HttpAsyncClient $asyncClient
4133
*/
42-
public function __construct($locator, $client, $asyncClient)
34+
public function __construct(HttpClient $httpClient = null, HttpAsyncClient $asyncClient = null)
4335
{
44-
static::$locator = $locator;
45-
static::$client = $client;
46-
static::$asyncClient = $asyncClient;
36+
self::$client = $httpClient;
37+
self::$asyncClient = $asyncClient;
4738
}
4839

4940
/**
5041
* {@inheritdoc}
5142
*/
5243
public static function getCandidates($type)
5344
{
54-
$locator = static::$locator;
55-
if ($type === HttpClient::class && $locator->has(static::$client)) {
56-
return [['class' => function () use ($locator) {
57-
return $locator->get(static::$client);
45+
if ($type === HttpClient::class && self::$client !== null) {
46+
return [['class' => function () {
47+
return self::$client;
5848
}]];
5949
}
6050

61-
if ($type === HttpAsyncClient::class && $locator->has(static::$asyncClient)) {
62-
return [['class' => function () use ($locator) {
63-
return $locator->get(static::$asyncClient);
51+
if ($type === HttpAsyncClient::class && self::$asyncClient !== null) {
52+
return [['class' => function () {
53+
return self::$asyncClient;
6454
}]];
6555
}
6656

‎Resources/config/services.xml‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,9 @@
44
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
55

66
<services>
7-
<service id="httplug.strategy.locator" class="Symfony\Component\DependencyInjection\ServiceLocator" public="false">
8-
<argument type="collection">
9-
<argument key="httplug.auto_discovery.auto_discovered_client" type="service" id="httplug.auto_discovery.auto_discovered_client" />
10-
<argument key="httplug.auto_discovery.auto_discovered_async" type="service" id="httplug.auto_discovery.auto_discovered_async" />
11-
</argument>
12-
<tag name="container.service_locator" />
13-
</service>
147
<service id="httplug.strategy" class="Http\HttplugBundle\Discovery\ConfiguredClientsStrategy">
15-
<argument type="service" id="httplug.strategy.locator" />
16-
<argument>httplug.auto_discovery.auto_discovered_client</argument>
17-
<argument>httplug.auto_discovery.auto_discovered_async</argument>
18-
8+
<argument type="service" id="httplug.auto_discovery.auto_discovered_client" on-invalid="null"/>
9+
<argument type="service" id="httplug.auto_discovery.auto_discovered_async" on-invalid="null"/>
1910
<tag name="kernel.event_subscriber"/>
2011
</service>
2112

‎Tests/Functional/DiscoveredClientsTest.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use Http\Client\HttpAsyncClient;
77
use Http\Client\HttpClient;
88
use Http\HttplugBundle\Collector\StackPlugin;
9-
use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
109
use Nyholm\NSA;
1110
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
1211

@@ -89,13 +88,10 @@ public function testForcedDiscovery()
8988
$this->assertFalse($container->has('httplug.auto_discovery.auto_discovered_async'));
9089
$this->assertTrue($container->has('httplug.strategy'));
9190

92-
$container->get('httplug.strategy');
91+
$strategy = $container->get('httplug.strategy');
9392

94-
$httpClientCandidate = ConfiguredClientsStrategy::getCandidates(HttpClient::class)[0]['class'];
95-
$httpAsyncClientCandidate = ConfiguredClientsStrategy::getCandidates(HttpAsyncClient::class)[0]['class'];
96-
97-
$this->assertEquals($container->get('httplug.client.acme'), $httpClientCandidate());
98-
$this->assertEquals($container->get('httplug.client.acme'), $httpAsyncClientCandidate());
93+
$this->assertEquals($container->get('httplug.client.acme'), NSA::getProperty($strategy, 'client'));
94+
$this->assertEquals($container->get('httplug.client.acme'), NSA::getProperty($strategy, 'asyncClient'));
9995
}
10096

10197
private function getContainer($debug, $environment = 'test')

‎Tests/Unit/Discovery/ConfiguredClientsStrategyTest.php‎

Lines changed: 4 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,14 @@
55
use Http\Client\HttpAsyncClient;
66
use Http\Client\HttpClient;
77
use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
8-
use Symfony\Component\DependencyInjection\ContainerInterface;
9-
use Symfony\Component\DependencyInjection\ServiceLocator;
108

119
class ConfiguredClientsStrategyTest extends \PHPUnit_Framework_TestCase
1210
{
1311
public function testGetCandidates()
1412
{
1513
$httpClient = $this->getMockBuilder(HttpClient::class)->getMock();
1614
$httpAsyncClient = $this->getMockBuilder(HttpAsyncClient::class)->getMock();
17-
$locator = $this->getLocatorMock();
18-
$locator
19-
->expects($this->exactly(2))
20-
->method('has')
21-
->willReturn(true)
22-
;
23-
$locator
24-
->expects($this->exactly(2))
25-
->method('get')
26-
->will($this->onConsecutiveCalls($httpClient, $httpAsyncClient))
27-
;
28-
29-
$strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
15+
$strategy = new ConfiguredClientsStrategy($httpClient, $httpAsyncClient);
3016

3117
$candidates = $strategy::getCandidates(HttpClient::class);
3218
$candidate = array_shift($candidates);
@@ -39,18 +25,7 @@ public function testGetCandidates()
3925

4026
public function testGetCandidatesEmpty()
4127
{
42-
$locator = $this->getLocatorMock();
43-
$locator
44-
->expects($this->exactly(2))
45-
->method('has')
46-
->willReturn(false)
47-
;
48-
$locator
49-
->expects($this->never())
50-
->method('get')
51-
;
52-
53-
$strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
28+
$strategy = new ConfiguredClientsStrategy(null, null);
5429

5530
$candidates = $strategy::getCandidates(HttpClient::class);
5631
$this->assertEquals([], $candidates);
@@ -62,23 +37,7 @@ public function testGetCandidatesEmpty()
6237
public function testGetCandidatesEmptyAsync()
6338
{
6439
$httpClient = $this->getMockBuilder(HttpClient::class)->getMock();
65-
66-
$locator = $this->getLocatorMock();
67-
$locator
68-
->expects($this->exactly(2))
69-
->method('has')
70-
->willReturnMap([
71-
['httplug.auto_discovery.auto_discovered_client', true],
72-
['httplug.auto_discovery.auto_discovered_async', false],
73-
])
74-
;
75-
$locator
76-
->expects($this->once())
77-
->method('get')
78-
->willReturn($httpClient)
79-
;
80-
81-
$strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
40+
$strategy = new ConfiguredClientsStrategy($httpClient, null);
8241

8342
$candidates = $strategy::getCandidates(HttpClient::class);
8443
$candidate = array_shift($candidates);
@@ -91,23 +50,7 @@ public function testGetCandidatesEmptyAsync()
9150
public function testGetCandidatesEmptySync()
9251
{
9352
$httpAsyncClient = $this->getMockBuilder(HttpAsyncClient::class)->getMock();
94-
95-
$locator = $this->getLocatorMock();
96-
$locator
97-
->expects($this->exactly(2))
98-
->method('has')
99-
->willReturnMap([
100-
['httplug.auto_discovery.auto_discovered_client', false],
101-
['httplug.auto_discovery.auto_discovered_async', true],
102-
])
103-
;
104-
$locator
105-
->expects($this->once())
106-
->method('get')
107-
->willReturn($httpAsyncClient)
108-
;
109-
110-
$strategy = new ConfiguredClientsStrategy($locator, 'httplug.auto_discovery.auto_discovered_client', 'httplug.auto_discovery.auto_discovered_async');
53+
$strategy = new ConfiguredClientsStrategy(null, $httpAsyncClient);
11154

11255
$candidates = $strategy::getCandidates(HttpClient::class);
11356
$this->assertEquals([], $candidates);
@@ -116,16 +59,4 @@ public function testGetCandidatesEmptySync()
11659
$candidate = array_shift($candidates);
11760
$this->assertEquals($httpAsyncClient, $candidate['class']());
11861
}
119-
120-
/**
121-
* @return ContainerInterface|ServiceLocator
122-
*/
123-
private function getLocatorMock()
124-
{
125-
if (class_exists(ServiceLocator::class)) {
126-
return $this->getMockBuilder(ServiceLocator::class)->disableOriginalConstructor()->getMock();
127-
}
128-
129-
return $this->getMockBuilder(ContainerInterface::class)->getMock();
130-
}
13162
}

0 commit comments

Comments
 (0)