Skip to content

Commit 50a90f9

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

File tree

10 files changed

+149
-113
lines changed

10 files changed

+149
-113
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99

1010
### Added
1111

12-
- You can now configure plugins on each client in `extra_plugins`. Plugins that you previously had to define your own services can be configured on the client.
12+
- You can now also configure client specific plugins in the `plugins` option of a client.
13+
Some plugins that you previously had to define in your own services can now be configured on the client.
1314
- Support for BatchClient
1415
- The stopwatch plugin in included by default when using profiling.
1516

‎DependencyInjection/Configuration.php

Lines changed: 69 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
namespace Http\HttplugBundle\DependencyInjection;
44

5-
use Symfony\Component\Config\Definition\ArrayNode;
65
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
6+
use Symfony\Component\Config\Definition\Builder\NodeBuilder;
77
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
88
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
99
use Symfony\Component\Config\Definition\ConfigurationInterface;
@@ -46,7 +46,7 @@ public function getConfigTreeBuilder()
4646
$rootNode = $treeBuilder->root('httplug');
4747

4848
$this->configureClients($rootNode);
49-
$this->configurePlugins($rootNode);
49+
$this->configureSharedPlugins($rootNode);
5050

5151
$rootNode
5252
->validate()
@@ -154,8 +154,9 @@ public function getConfigTreeBuilder()
154154

155155
private function configureClients(ArrayNodeDefinition $root)
156156
{
157-
$root->children()
157+
$pluginNode = $root->children()
158158
->arrayNode('clients')
159+
->fixXmlConfig('plugin')
159160
->validate()
160161
->ifTrue(function ($clients) {
161162
foreach ($clients as $name => $config) {
@@ -186,55 +187,82 @@ private function configureClients(ArrayNodeDefinition $root)
186187
->defaultFalse()
187188
->info('Set to true to get the client wrapped in a BatchClient which allows you to send multiple request at the same time.')
188189
->end()
189-
->arrayNode('plugins')
190-
->info('A list of service ids of plugins. The order is important.')
191-
->prototype('scalar')->end()
192-
->end()
193190
->variableNode('config')->defaultValue([])->end()
194-
->append($this->createExtraPluginsNode())
195-
->end()
196-
->end();
191+
->arrayNode('plugins')
192+
->info('A list of plugin service ids and client specific plugin definitions. The order is important.')
193+
->prototype('array')
194+
;
195+
196+
$this->configureClientPlugins($pluginNode);
197197
}
198198

199199
/**
200200
* @param ArrayNodeDefinition $root
201201
*/
202-
private function configurePlugins(ArrayNodeDefinition $root)
202+
private function configureSharedPlugins(ArrayNodeDefinition $root)
203203
{
204204
$pluginsNode = $root
205205
->children()
206206
->arrayNode('plugins')
207207
->addDefaultsIfNotSet()
208208
;
209-
$this->configureSharedPluginNodes($pluginsNode);
209+
$this->addSharedPluginNodes($pluginsNode);
210210
}
211211

212212
/**
213-
* Create configuration for the extra_plugins node inside the client.
213+
* Configure plugins node of a client.
214214
*
215-
* @return NodeDefinition Definition of the extra_plugins node in the client.
215+
* @param ArrayNodeDefinition $pluginNode The node to add plugin definitions to.
216216
*/
217-
private function createExtraPluginsNode()
217+
private function configureClientPlugins(ArrayNodeDefinition $pluginNode)
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+
$pluginNode
220+
// support having just a service id in the list
221+
->beforeNormalization()
222+
->always(function ($plugin) {
223+
if (is_string($plugin)) {
224+
return [
225+
'reference' => [
226+
'enabled' => true,
227+
'id' => $plugin,
228+
],
229+
];
229230
}
230-
}
231231

232-
return $plugins;
233-
})
232+
return $plugin;
233+
})
234+
->end()
235+
236+
->validate()
237+
->always(function ($plugins) {
238+
if (isset($plugins['authentication']) && !count($plugins['authentication'])) {
239+
unset($plugins['authentication']);
240+
}
241+
foreach ($plugins as $name => $definition) {
242+
if (!$definition['enabled']) {
243+
unset($plugins[$name]);
244+
}
245+
}
246+
247+
return $plugins;
248+
})
249+
->end()
234250
;
235-
$this->configureSharedPluginNodes($node, true);
236-
$node
251+
$this->addSharedPluginNodes($pluginNode, true);
252+
253+
$pluginNode
237254
->children()
255+
->arrayNode('reference')
256+
->canBeEnabled()
257+
->info('Reference to a plugin service')
258+
->children()
259+
->scalarNode('id')
260+
->info('Service id of a plugin')
261+
->isRequired()
262+
->cannotBeEmpty()
263+
->end()
264+
->end()
265+
->end()
238266
->arrayNode('add_host')
239267
->canBeEnabled()
240268
->addDefaultsIfNotSet()
@@ -251,17 +279,19 @@ private function createExtraPluginsNode()
251279
->end()
252280
->end()
253281
->end()
282+
283+
// TODO add aditional plugins that are only usable on a specific client
254284
->end()
255285
->end();
256-
257-
return $node;
258286
}
259287

260288
/**
261-
* @param ArrayNodeDefinition $pluginNode
289+
* Add the definitions for shared plugin configurations.
290+
*
291+
* @param ArrayNodeDefinition $pluginNode The node to add to.
262292
* @param bool $disableAll Some shared plugins are enabled by default. On the client, all are disabled by default.
263293
*/
264-
private function configureSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableAll = false)
294+
private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableAll = false)
265295
{
266296
$children = $pluginNode->children();
267297

@@ -305,11 +335,7 @@ private function configureSharedPluginNodes(ArrayNodeDefinition $pluginNode, $di
305335
// End cookie plugin
306336

307337
$decoder = $children->arrayNode('decoder');
308-
if ($disableAll) {
309-
$decoder->canBeEnabled();
310-
} else {
311-
$decoder->canBeDisabled();
312-
}
338+
$disableAll ? $decoder->canBeEnabled() : $decoder->canBeDisabled();
313339
$decoder->addDefaultsIfNotSet()
314340
->children()
315341
->scalarNode('use_content_encoding')->defaultTrue()->end()
@@ -330,11 +356,7 @@ private function configureSharedPluginNodes(ArrayNodeDefinition $pluginNode, $di
330356
// End history plugin
331357

332358
$logger = $children->arrayNode('logger');
333-
if ($disableAll) {
334-
$logger->canBeEnabled();
335-
} else {
336-
$logger->canBeDisabled();
337-
}
359+
$disableAll ? $logger->canBeEnabled() : $logger->canBeDisabled();
338360
$logger->addDefaultsIfNotSet()
339361
->children()
340362
->scalarNode('logger')
@@ -351,11 +373,7 @@ private function configureSharedPluginNodes(ArrayNodeDefinition $pluginNode, $di
351373
// End logger plugin
352374

353375
$redirect = $children->arrayNode('redirect');
354-
if ($disableAll) {
355-
$redirect->canBeEnabled();
356-
} else {
357-
$redirect->canBeDisabled();
358-
}
376+
$disableAll ? $redirect->canBeEnabled() : $redirect->canBeDisabled();
359377
$redirect->addDefaultsIfNotSet()
360378
->children()
361379
->scalarNode('preserve_header')->defaultTrue()->end()
@@ -365,11 +383,7 @@ private function configureSharedPluginNodes(ArrayNodeDefinition $pluginNode, $di
365383
// End redirect plugin
366384

367385
$retry = $children->arrayNode('retry');
368-
if ($disableAll) {
369-
$retry->canBeEnabled();
370-
} else {
371-
$retry->canBeDisabled();
372-
}
386+
$disableAll ? $retry->canBeEnabled() : $retry->canBeDisabled();
373387
$retry->addDefaultsIfNotSet()
374388
->children()
375389
->scalarNode('retry')->defaultValue(1)->end() // TODO: should be called retries for consistency with the class
@@ -378,11 +392,7 @@ private function configureSharedPluginNodes(ArrayNodeDefinition $pluginNode, $di
378392
// End retry plugin
379393

380394
$stopwatch = $children->arrayNode('stopwatch');
381-
if ($disableAll) {
382-
$stopwatch->canBeEnabled();
383-
} else {
384-
$stopwatch->canBeDisabled();
385-
}
395+
$disableAll ? $stopwatch->canBeEnabled() : $stopwatch->canBeDisabled();
386396
$stopwatch->addDefaultsIfNotSet()
387397
->children()
388398
->scalarNode('stopwatch')

‎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($pluginId);
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"

0 commit comments

Comments
 (0)