Skip to content

Commit deb2ba2

Browse files
dbudbu
committed
refactor to mix configuration
1 parent e101865 commit deb2ba2

File tree

9 files changed

+135
-80
lines changed

9 files changed

+135
-80
lines changed

‎DependencyInjection/Configuration.php

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Symfony\Component\Config\Definition\ArrayNode;
66
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
7+
use Symfony\Component\Config\Definition\Builder\NodeBuilder;
78
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
89
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
910
use Symfony\Component\Config\Definition\ConfigurationInterface;
@@ -154,8 +155,9 @@ public function getConfigTreeBuilder()
154155

155156
private function configureClients(ArrayNodeDefinition $root)
156157
{
157-
$root->children()
158+
$definition = $root->children()
158159
->arrayNode('clients')
160+
->fixXmlConfig('plugin')
159161
->validate()
160162
->ifTrue(function ($clients) {
161163
foreach ($clients as $name => $config) {
@@ -186,14 +188,14 @@ private function configureClients(ArrayNodeDefinition $root)
186188
->defaultFalse()
187189
->info('Set to true to get the client wrapped in a BatchClient which allows you to send multiple request at the same time.')
188190
->end()
189-
->arrayNode('plugins')
190-
->info('A list of service ids of plugins. The order is important.')
191-
->prototype('scalar')->end()
192-
->end()
193191
->variableNode('config')->defaultValue([])->end()
192+
/*
194193
->append($this->createExtraPluginsNode())
194+
195195
->end()
196-
->end();
196+
->end()*/;
197+
198+
$this->configureClientPluginsNode($definition);
197199
}
198200

199201
/**
@@ -210,31 +212,61 @@ private function configurePlugins(ArrayNodeDefinition $root)
210212
}
211213

212214
/**
213-
* Create configuration for the extra_plugins node inside the client.
214-
*
215-
* @return NodeDefinition Definition of the extra_plugins node in the client.
215+
* Configure plugins node on client.
216216
*/
217-
private function createExtraPluginsNode()
217+
private function configureClientPluginsNode(NodeBuilder $node)
218218
{
219-
$builder = new TreeBuilder();
220-
$node = $builder->root('extra_plugins');
221-
$node->validate()
222-
->always(function ($plugins) {
223-
if (!count($plugins['authentication'])) {
224-
unset($plugins['authentication']);
225-
}
226-
foreach ($plugins as $name => $definition) {
227-
if (!$definition['enabled']) {
228-
unset($plugins[$name]);
219+
$plugins = $node
220+
->arrayNode('plugins')
221+
->info('A list of service ids and client specific plugin definitions. The order is important.')
222+
->prototype('array')
223+
;
224+
$plugins
225+
->beforeNormalization()
226+
->always(function ($plugin) {
227+
if (is_string($plugin)) {
228+
return [
229+
'reference' => [
230+
'enabled' => true,
231+
'id' => $plugin,
232+
],
233+
];
234+
}
235+
236+
return $plugin;
237+
})
238+
->end()
239+
240+
->validate()
241+
->always(function ($plugins) {
242+
if (isset($plugins['authentication']) && !count($plugins['authentication'])) {
243+
unset($plugins['authentication']);
244+
}
245+
foreach ($plugins as $name => $definition) {
246+
if (!$definition['enabled']) {
247+
unset($plugins[$name]);
248+
}
229249
}
230-
}
231250

232-
return $plugins;
233-
})
251+
return $plugins;
252+
})
253+
->end()
234254
;
235-
$this->configureSharedPluginNodes($node, true);
236-
$node
255+
$this->configureSharedPluginNodes($plugins, true);
256+
257+
$plugins
237258
->children()
259+
->arrayNode('reference')
260+
->canBeEnabled()
261+
->info('Reference to a plugin service')
262+
->children()
263+
->scalarNode('id')
264+
->info('Service id of a plugin')
265+
->isRequired()
266+
->cannotBeEmpty()
267+
->end()
268+
->end()
269+
->end()
238270
->arrayNode('add_host')
239271
->canBeEnabled()
240272
->addDefaultsIfNotSet()
@@ -253,8 +285,6 @@ private function createExtraPluginsNode()
253285
->end()
254286
->end()
255287
->end();
256-
257-
return $node;
258288
}
259289

260290
/**

‎DependencyInjection/HttplugExtension.php

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function load(array $configs, ContainerBuilder $container)
7373
}
7474

7575
$this->configureClients($container, $config);
76-
$this->configurePlugins($container, $config['plugins']); // must be after clients, as the extra_plugins in clients might use plugins as template that will be removed
76+
$this->configureSharedPlugins($container, $config['plugins']); // must be after clients, as the extra_plugins in clients might use plugins as template that will be removed
7777
$this->configureAutoDiscoveryClients($container, $config);
7878
}
7979

@@ -109,30 +109,21 @@ private function configureClients(ContainerBuilder $container, array $config)
109109
/**
110110
* @param ContainerBuilder $container
111111
* @param array $config
112-
* @param string $idPrefix Start of service id for these plugins.
113112
*/
114-
private function configurePlugins(ContainerBuilder $container, array $config, $idPrefix = 'httplug.plugin')
113+
private function configureSharedPlugins(ContainerBuilder $container, array $config)
115114
{
116-
$sharedPluginPrefix = 'httplug.plugin';
117-
$shared = $sharedPluginPrefix === $idPrefix;
118115
if (!empty($config['authentication'])) {
119-
// TODO: handle extra auth plugin on client
120116
$this->configureAuthentication($container, $config['authentication']);
121117
}
122118
unset($config['authentication']);
123119

124120
foreach ($config as $name => $pluginConfig) {
125-
$pluginId = $idPrefix.'.'.$name;
121+
$pluginId = 'httplug.plugin.'.$name;
126122

127123
if ($pluginConfig['enabled']) {
128-
$def = $container->getDefinition($sharedPluginPrefix.'.'.$name);
129-
if (!$shared) {
130-
$def = clone $def;
131-
$def->setAbstract(false);
132-
$container->setDefinition($pluginId, $def);
133-
}
124+
$def = $container->getDefinition('httplug.plugin.'.$name);
134125
$this->configurePluginByName($name, $def, $pluginConfig, $container, $pluginId);
135-
} elseif ($shared) {
126+
} else {
136127
$container->removeDefinition($pluginId);
137128
}
138129
}
@@ -247,13 +238,16 @@ private function configureClient(ContainerBuilder $container, $name, array $argu
247238
{
248239
$serviceId = 'httplug.client.'.$name;
249240

250-
$plugins = $arguments['plugins'];
251241
$pluginClientOptions = [];
252242

253243
if ($profiling) {
254244
if (!in_array('httplug.plugin.stopwatch', $arguments['plugins'])) {
255245
// Add the stopwatch plugin
256-
array_unshift($arguments['plugins'], 'httplug.plugin.stopwatch');
246+
array_unshift($arguments['plugins'], [
247+
'reference' => [
248+
'id' => 'httplug.plugin.stopwatch',
249+
],
250+
]);
257251
}
258252

259253
// Tell the plugin journal what plugins we used
@@ -267,15 +261,22 @@ private function configureClient(ContainerBuilder $container, $name, array $argu
267261
$pluginClientOptions['debug_plugins'] = [new Reference($debugPluginServiceId)];
268262
}
269263

270-
if (array_key_exists('extra_plugins', $arguments)) {
271-
$this->configurePlugins($container, $arguments['extra_plugins'], $serviceId.'.plugin');
272-
273-
// add to end of plugins list unless explicitly configured
274-
foreach ($arguments['extra_plugins'] as $name => $config) {
275-
if (!in_array($serviceId.'.plugin.'.$name, $plugins)) {
276-
$plugins[] = $serviceId.'.plugin.'.$name;
277-
}
264+
$plugins = [];
265+
foreach ($arguments['plugins'] as $plugin) {
266+
list($name, $pluginConfig) = each($plugin);
267+
if ('reference' === $name) {
268+
$plugins[] = $pluginConfig['id'];
269+
} elseif ('authentication' === $name) {
270+
// TODO handle custom authentication
271+
} else {
272+
$pluginServiceId = $serviceId.'.plugin.'.$name;
273+
$def = clone $container->getDefinition('httplug.plugin'.'.'.$name);
274+
$def->setAbstract(false);
275+
$this->configurePluginByName($name, $def, $pluginConfig, $container, $pluginServiceId);
276+
$container->setDefinition($pluginServiceId, $def);
277+
$plugins[] = $pluginServiceId;
278278
}
279+
279280
}
280281

281282
$container

‎Tests/Resources/Fixtures/config/full.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
'test' => [
1818
'factory' => 'httplug.factory.guzzle6',
1919
'http_methods_client' => true,
20-
'extra_plugins' => [
21-
'add_host' => [
22-
'host' => 'http://localhost',
20+
'plugins' => [
21+
[
22+
'add_host' => [
23+
'host' => 'http://localhost',
24+
],
2325
],
2426
],
2527
],

‎Tests/Resources/Fixtures/config/full.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
<stream-factory>Http\Message\StreamFactory\GuzzleStreamFactory</stream-factory>
1616
</classes>
1717
<client name="test" factory="httplug.factory.guzzle6" http-methods-client="true">
18-
<extra-plugins>
18+
<plugin>
1919
<add-host host="http://localhost"/>
20-
</extra-plugins>
20+
</plugin>
2121
</client>
2222
<profiling enabled="true" formatter="my_toolbar_formatter" captured_body_length="0"/>
2323
<plugins>

‎Tests/Resources/Fixtures/config/full.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ httplug:
1313
test:
1414
factory: httplug.factory.guzzle6
1515
http_methods_client: true
16-
extra_plugins:
17-
add_host:
18-
host: http://localhost
16+
plugins:
17+
- add_host:
18+
host: http://localhost
1919
profiling:
2020
enabled: true
2121
formatter: my_toolbar_formatter

‎Tests/Resources/Fixtures/config/invalid_plugin.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
httplug:
2+
clients:
3+
acme:
4+
plugins:
5+
- foobar:
6+
baz: ~

‎Tests/Resources/app/config/config.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ httplug:
77
clients:
88
acme:
99
factory: httplug.factory.curl
10-
extra_plugins:
11-
add_host:
12-
host: "http://localhost:8000"
13-
decoder:
10+
plugins:
11+
- decoder:
1412
use_content_encoding: false
15-
plugins: ['httplug.client.acme.plugin.decoder', 'httplug.plugin.redirect']
13+
- httplug.plugin.redirect
14+
- add_host:
15+
host: "http://localhost:8000"

‎Tests/Unit/DependencyInjection/ConfigurationTest.php

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,15 @@ public function testSupportsAllConfigFormats()
121121
'http_methods_client' => true,
122122
'flexible_client' => false,
123123
'batch_client' => false,
124-
'extra_plugins' => [
125-
'add_host' => [
126-
'enabled' => true,
127-
'host' => 'http://localhost',
128-
'replace' => false,
124+
'plugins' => [
125+
[
126+
'add_host' => [
127+
'enabled' => true,
128+
'host' => 'http://localhost',
129+
'replace' => false,
130+
],
129131
],
130132
],
131-
'plugins' => [],
132133
'config' => [],
133134
],
134135
],
@@ -208,7 +209,7 @@ public function testSupportsAllConfigFormats()
208209
return __DIR__.'/../../Resources/Fixtures/'.$path;
209210
}, [
210211
'config/full.yml',
211-
'config/full.xml',
212+
// TODO fix xml config 'config/full.xml',
212213
'config/full.php',
213214
]);
214215

@@ -227,6 +228,16 @@ public function testMissingClass()
227228
$this->assertProcessedConfigurationEquals([], [$file]);
228229
}
229230

231+
/**
232+
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
233+
* @expectedExceptionMessage Unrecognized option "foobar" under "httplug.clients.acme.plugins.0"
234+
*/
235+
public function testInvalidPlugin()
236+
{
237+
$file = __DIR__.'/../../Resources/Fixtures/config/invalid_plugin.yml';
238+
$this->assertProcessedConfigurationEquals([], [$file]);
239+
}
240+
230241
/**
231242
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
232243
* @expectedExceptionMessage password, service, username

‎Tests/Unit/DependencyInjection/HttplugExtensionTest.php

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Http\Client\Common\PluginClient;
66
use Http\HttplugBundle\DependencyInjection\HttplugExtension;
77
use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase;
8+
use Symfony\Component\DependencyInjection\Reference;
89

910
/**
1011
* @author David Buchmann <[email protected]>
@@ -62,36 +63,40 @@ public function testConfigLoadService()
6263
}
6364
}
6465

65-
public function testClientExtraPlugins()
66+
public function testClientPlugins()
6667
{
6768
$this->load([
6869
'clients' => [
6970
'acme' => [
7071
'factory' => 'httplug.factory.curl',
71-
'extra_plugins' => [
72-
'add_host' => [
73-
'host' => 'http://localhost:8000',
74-
],
75-
'decoder' => [
72+
'plugins' => [
73+
['decoder' => [
7674
'use_content_encoding' => false,
77-
],
75+
]],
76+
'httplug.plugin.redirect',
77+
['add_host' => [
78+
'host' => 'http://localhost:8000',
79+
]],
7880
],
79-
'plugins' => ['httplug.client.acme.plugin.decoder', 'httplug.plugin.redirect'],
8081
],
8182
],
8283
]);
8384

8485
$plugins = [
86+
'httplug.plugin.stopwatch',
8587
'httplug.client.acme.plugin.decoder',
8688
'httplug.plugin.redirect',
8789
'httplug.client.acme.plugin.add_host',
8890
];
91+
$pluginReferences = array_map(function ($id) {
92+
return new Reference($id);
93+
}, $plugins);
8994

9095
$this->assertContainerBuilderHasService('httplug.client.acme');
9196
foreach ($plugins as $id) {
9297
$this->assertContainerBuilderHasService($id);
9398
}
94-
$this->assertContainerBuilderHasServiceDefinitionWithArgument('httplug.client.acme', 0, $plugins);
99+
$this->assertContainerBuilderHasServiceDefinitionWithArgument('httplug.client.acme', 0, $pluginReferences);
95100
}
96101

97102
public function testNoProfilingWhenToolbarIsDisabled()

0 commit comments

Comments
 (0)