Completed
Pull Request — master (#145)
by Rob
05:57
created

Configuration::getConfigTreeBuilder()   C

Complexity

Conditions 12
Paths 1

Size

Total Lines 111
Code Lines 91

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 92
CRAP Score 12

Importance

Changes 0
Metric Value
dl 0
loc 111
ccs 92
cts 92
cp 1
rs 5.034
c 0
b 0
f 0
cc 12
eloc 91
nc 1
nop 0
crap 12

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
namespace Http\HttplugBundle\DependencyInjection;
4
5
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
6
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
7
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
8
use Symfony\Component\Config\Definition\ConfigurationInterface;
9
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
10
use Symfony\Component\Config\Definition\Exception\InvalidTypeException;
11
12
/**
13
 * This class contains the configuration information for the bundle.
14
 *
15
 * This information is solely responsible for how the different configuration
16
 * sections are normalized, and merged.
17
 *
18
 * @author David Buchmann <[email protected]>
19
 * @author Tobias Nyholm <[email protected]>
20
 */
21
class Configuration implements ConfigurationInterface
22
{
23
    /**
24
     * Whether to use the debug mode.
25
     *
26
     * @see https://github.com/doctrine/DoctrineBundle/blob/v1.5.2/DependencyInjection/Configuration.php#L31-L41
27
     *
28
     * @var bool
29
     */
30
    private $debug;
31
32
    /**
33
     * @param bool $debug
34
     */
35 18
    public function __construct($debug)
36
    {
37 18
        $this->debug = (bool) $debug;
38 18
    }
39
40
    /**
41
     * {@inheritdoc}
42
     */
43 18
    public function getConfigTreeBuilder()
44
    {
45 18
        $treeBuilder = new TreeBuilder();
46 18
        $rootNode = $treeBuilder->root('httplug');
47
48 18
        $this->configureClients($rootNode);
49 18
        $this->configureSharedPlugins($rootNode);
50
51
        $rootNode
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method fixXmlConfig() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
52 18
            ->validate()
53
                ->ifTrue(function ($v) {
54 14
                    return !empty($v['classes']['client'])
55 14
                        || !empty($v['classes']['message_factory'])
56 11
                        || !empty($v['classes']['uri_factory'])
57 14
                        || !empty($v['classes']['stream_factory']);
58 18
                })
59
                ->then(function ($v) {
60 3
                    foreach ($v['classes'] as $key => $class) {
61 3
                        if (null !== $class && !class_exists($class)) {
62 1
                            throw new InvalidConfigurationException(sprintf(
63 1
                                'Class %s specified for httplug.classes.%s does not exist.',
64 1
                                $class,
65
                                $key
66 1
                            ));
67
                        }
68 2
                    }
69
70 2
                    return $v;
71 18
                })
72 18
            ->end()
73 18
            ->beforeNormalization()
74
                ->ifTrue(function ($v) {
75 18
                    return is_array($v) && array_key_exists('toolbar', $v) && is_array($v['toolbar']);
76 18
                })
77
                ->then(function ($v) {
78 4
                    if (array_key_exists('profiling', $v)) {
79 1
                        throw new InvalidConfigurationException('Can\'t configure both "toolbar" and "profiling" section. The "toolbar" config is deprecated as of version 1.3.0, please only use "profiling".');
80
                    }
81
82 3
                    @trigger_error('"httplug.toolbar" config is deprecated since version 1.3 and will be removed in 2.0. Use "httplug.profiling" instead.', E_USER_DEPRECATED);
1 ignored issue
show
Security Best Practice introduced by
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
83
84 3
                    if (array_key_exists('enabled', $v['toolbar']) && 'auto' === $v['toolbar']['enabled']) {
85 1
                        @trigger_error('"auto" value in "httplug.toolbar" config is deprecated since version 1.3 and will be removed in 2.0. Use a boolean value instead.', E_USER_DEPRECATED);
1 ignored issue
show
Security Best Practice introduced by
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
86 1
                        $v['toolbar']['enabled'] = $this->debug;
87 1
                    }
88
89 3
                    $v['profiling'] = $v['toolbar'];
90
91 3
                    unset($v['toolbar']);
92
93 3
                    return $v;
94 18
                })
95 18
            ->end()
96 18
            ->fixXmlConfig('client')
97 18
            ->children()
98 18
                ->arrayNode('main_alias')
99 18
                    ->addDefaultsIfNotSet()
100 18
                    ->info('Configure which service the main alias point to.')
101 18
                    ->children()
102 18
                        ->scalarNode('client')->defaultValue('httplug.client.default')->end()
103 18
                        ->scalarNode('message_factory')->defaultValue('httplug.message_factory.default')->end()
104 18
                        ->scalarNode('uri_factory')->defaultValue('httplug.uri_factory.default')->end()
105 18
                        ->scalarNode('stream_factory')->defaultValue('httplug.stream_factory.default')->end()
106 18
                    ->end()
107 18
                ->end()
108 18
                ->arrayNode('classes')
109 18
                    ->addDefaultsIfNotSet()
110 18
                    ->info('Overwrite a service class instead of using the discovery mechanism.')
111 18
                    ->children()
112 18
                        ->scalarNode('client')->defaultNull()->end()
113 18
                        ->scalarNode('message_factory')->defaultNull()->end()
114 18
                        ->scalarNode('uri_factory')->defaultNull()->end()
115 18
                        ->scalarNode('stream_factory')->defaultNull()->end()
116 18
                    ->end()
117 18
                ->end()
118 18
                ->arrayNode('profiling')
119 18
                    ->addDefaultsIfNotSet()
120 18
                    ->treatFalseLike(['enabled' => false])
121 18
                    ->treatTrueLike(['enabled' => true])
122 18
                    ->treatNullLike(['enabled' => $this->debug])
123 18
                    ->info('Extend the debug profiler with information about requests.')
124 18
                    ->children()
125 18
                        ->booleanNode('enabled')
126 18
                            ->info('Turn the toolbar on or off. Defaults to kernel debug mode.')
127 18
                            ->defaultValue($this->debug)
128 18
                        ->end()
129 18
                        ->scalarNode('formatter')->defaultNull()->end()
130 18
                        ->integerNode('captured_body_length')
131 18
                            ->defaultValue(0)
132 18
                            ->info('Limit long HTTP message bodies to x characters. If set to 0 we do not read the message body. Only available with the default formatter (FullHttpMessageFormatter).')
133 18
                        ->end()
134 18
                    ->end()
135 18
                ->end()
136 18
                ->arrayNode('discovery')
137 18
                    ->addDefaultsIfNotSet()
138 18
                    ->info('Control what clients should be found by the discovery.')
139 18
                    ->children()
140 18
                        ->scalarNode('client')
141 18
                            ->defaultValue('auto')
142 18
                            ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.')
143 18
                        ->end()
144 18
                        ->scalarNode('async_client')
145 18
                            ->defaultNull()
146 18
                            ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.')
147 18
                        ->end()
148 18
                    ->end()
149 18
                ->end()
150 18
            ->end();
151
152 18
        return $treeBuilder;
153
    }
154
155 18
    private function configureClients(ArrayNodeDefinition $root)
156
    {
157 18
        $root->children()
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method fixXmlConfig() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
158 18
            ->arrayNode('clients')
159 18
                ->useAttributeAsKey('name')
160 18
                ->prototype('array')
161 18
                ->fixXmlConfig('plugin')
162 18
                ->validate()
163
                    ->ifTrue(function ($config) {
164
                        // Make sure we only allow one of these to be true
165 7
                        return (bool) $config['flexible_client'] + (bool) $config['http_methods_client'] + (bool) $config['batch_client'] >= 2;
166 18
                    })
167 18
                    ->thenInvalid('A http client can\'t be decorated with several of FlexibleHttpClient, HttpMethodsClient and BatchClient. Only one of the following options can be true. ("flexible_client", "http_methods_client", "batch_client")')
168 18
                ->end()
169 18
                ->children()
170 18
                    ->scalarNode('factory')
171 18
                        ->isRequired()
172 18
                        ->cannotBeEmpty()
173 18
                        ->info('The service id of a factory to use when creating the adapter.')
174 18
                    ->end()
175 18
                    ->booleanNode('flexible_client')
176 18
                        ->defaultFalse()
177 18
                        ->info('Set to true to get the client wrapped in a FlexibleHttpClient which emulates async or sync behavior.')
178 18
                    ->end()
179 18
                    ->booleanNode('http_methods_client')
180 18
                        ->defaultFalse()
181 18
                        ->info('Set to true to get the client wrapped in a HttpMethodsClient which emulates provides functions for HTTP verbs.')
182 18
                    ->end()
183 18
                    ->booleanNode('batch_client')
184 18
                        ->defaultFalse()
185 18
                        ->info('Set to true to get the client wrapped in a BatchClient which allows you to send multiple request at the same time.')
186 18
                    ->end()
187 18
                    ->variableNode('config')->defaultValue([])->end()
188 18
                    ->append($this->createClientPluginNode())
189 18
                ->end()
190 18
            ->end()
191 18
        ->end();
192 18
    }
193
194
    /**
195
     * @param ArrayNodeDefinition $root
196
     */
197 18
    private function configureSharedPlugins(ArrayNodeDefinition $root)
198
    {
199
        $pluginsNode = $root
200 18
            ->children()
201 18
                ->arrayNode('plugins')
202 18
                ->info('Global plugin configuration. Plugins need to be explicitly added to clients.')
203 18
                ->addDefaultsIfNotSet()
204
            // don't call end to get the plugins node
205 18
        ;
206 18
        $this->addSharedPluginNodes($pluginsNode);
207 18
    }
208
209
    /**
210
     * Createplugins node of a client.
211
     *
212
     * @return ArrayNodeDefinition The plugin node
213
     */
214 18
    private function createClientPluginNode()
215
    {
216 18
        $builder = new TreeBuilder();
217 18
        $node = $builder->root('plugins');
218
219
        /** @var ArrayNodeDefinition $pluginList */
220
        $pluginList = $node
221 18
            ->info('A list of plugin service ids and client specific plugin definitions. The order is important.')
222 18
            ->prototype('array')
223 18
        ;
224
        $pluginList
225
            // support having just a service id in the list
226 18
            ->beforeNormalization()
227
                ->always(function ($plugin) {
228 8
                    if (is_string($plugin)) {
229
                        return [
230
                            'reference' => [
231 7
                                'enabled' => true,
232 7
                                'id' => $plugin,
233 7
                            ],
234 7
                        ];
235
                    }
236
237 5
                    return $plugin;
238 18
                })
239 18
            ->end()
240
241 18
            ->validate()
242
                ->always(function ($plugins) {
243 7
                    foreach ($plugins as $name => $definition) {
244 7
                        if ('authentication' === $name) {
245 7
                            if (!count($definition)) {
246 7
                                unset($plugins['authentication']);
247 7
                            }
248 7
                        } elseif (!$definition['enabled']) {
249 7
                            unset($plugins[$name]);
250 7
                        }
251 7
                    }
252
253 7
                    return $plugins;
254 18
                })
255 18
            ->end()
256
        ;
257 18
        $this->addSharedPluginNodes($pluginList, true);
258
259
        $pluginList
260 18
            ->children()
261 18
                ->arrayNode('reference')
262 18
                    ->canBeEnabled()
263 18
                    ->info('Reference to a plugin service')
264 18
                    ->children()
265 18
                        ->scalarNode('id')
266 18
                            ->info('Service id of a plugin')
267 18
                            ->isRequired()
268 18
                            ->cannotBeEmpty()
269 18
                        ->end()
270 18
                    ->end()
271 18
                ->end()
272 18
                ->arrayNode('add_host')
273 18
                    ->canBeEnabled()
274 18
                    ->addDefaultsIfNotSet()
275 18
                    ->info('Set scheme, host and port in the request URI.')
276 18
                    ->children()
277 18
                        ->scalarNode('host')
278 18
                            ->info('Host name including protocol and optionally the port number, e.g. https://api.local:8000')
279 18
                            ->isRequired()
280 18
                            ->cannotBeEmpty()
281 18
                        ->end()
282 18
                        ->scalarNode('replace')
283 18
                            ->info('Whether to replace the host if request already specifies one')
284 18
                            ->defaultValue(false)
285 18
                        ->end()
286 18
                    ->end()
287 18
                ->end()
288 18
                ->arrayNode('header_append')
289 18
                    ->canBeEnabled()
290 18
                    ->info('Append headers to the request. If the header already exists the value will be appended to the current value.')
291 18
                    ->fixXmlConfig('header')
292 18
                    ->children()
293 18
                        ->arrayNode('headers')
294 18
                            ->info('Keys are the header names, values the header values')
295 18
                            ->normalizeKeys(false)
296 18
                            ->useAttributeAsKey('name')
297 18
                            ->prototype('scalar')->end()
298 18
                        ->end()
299 18
                    ->end()
300 18
                ->end()
301 18
                ->arrayNode('header_defaults')
302 18
                    ->canBeEnabled()
303 18
                    ->info('Set header to default value if it does not exist.')
304 18
                    ->fixXmlConfig('header')
305 18
                    ->children()
306 18
                        ->arrayNode('headers')
307 18
                            ->info('Keys are the header names, values the header values')
308 18
                            ->normalizeKeys(false)
309 18
                            ->useAttributeAsKey('name')
310 18
                            ->prototype('scalar')->end()
311 18
                        ->end()
312 18
                    ->end()
313 18
                ->end()
314 18
                ->arrayNode('header_set')
315 18
                    ->canBeEnabled()
316 18
                    ->info('Set headers to requests. If the header does not exist it wil be set, if the header already exists it will be replaced.')
317 18
                    ->fixXmlConfig('header')
318 18
                    ->children()
319 18
                        ->arrayNode('headers')
320 18
                            ->info('Keys are the header names, values the header values')
321 18
                            ->normalizeKeys(false)
322 18
                            ->useAttributeAsKey('name')
323 18
                            ->prototype('scalar')->end()
324 18
                        ->end()
325 18
                    ->end()
326 18
                ->end()
327 18
                ->arrayNode('header_remove')
328 18
                    ->canBeEnabled()
329 18
                    ->info('Remove headers from requests.')
330 18
                    ->fixXmlConfig('header')
331 18
                    ->children()
332 18
                        ->arrayNode('headers')
333 18
                            ->info('List of header names to remove')
334 18
                            ->prototype('scalar')->end()
335 18
                        ->end()
336 18
                    ->end()
337 18
                ->end()
338 18
            ->end()
339 18
        ->end();
340
341 18
        return $node;
342
    }
343
344
    /**
345
     * Add the definitions for shared plugin configurations.
346
     *
347
     * @param ArrayNodeDefinition $pluginNode The node to add to.
348
     * @param bool                $disableAll Some shared plugins are enabled by default. On the client, all are disabled by default.
349
     */
350 18
    private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableAll = false)
351
    {
352 18
        $children = $pluginNode->children();
353
354 18
        $children->append($this->createAuthenticationPluginNode());
355
356 18
        $children->arrayNode('cache')
357 18
            ->canBeEnabled()
358 18
            ->addDefaultsIfNotSet()
359 18
                ->children()
360 18
                    ->scalarNode('cache_pool')
361 18
                        ->info('This must be a service id to a service implementing Psr\Cache\CacheItemPoolInterface')
362 18
                        ->isRequired()
363 18
                        ->cannotBeEmpty()
364 18
                    ->end()
365 18
                    ->scalarNode('stream_factory')
366 18
                        ->info('This must be a service id to a service implementing Http\Message\StreamFactory')
367 18
                        ->defaultValue('httplug.stream_factory')
368 18
                        ->cannotBeEmpty()
369 18
                    ->end()
370 18
                    ->arrayNode('config')
371 18
                        ->addDefaultsIfNotSet()
372 18
                        ->validate()
373
                        ->ifTrue(function ($config) {
374
                            // Cannot set both respect_cache_headers and respect_response_cache_directives
375 2
                            return isset($config['respect_cache_headers'], $config['respect_response_cache_directives']);
376 18
                        })
377 18
                        ->thenInvalid('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives" simultaniously. Use "respect_response_cache_directives" instead.')
378 18
                        ->end()
379 18
                        ->children()
380 18
                            ->scalarNode('default_ttl')->defaultValue(0)->end()
381 18
                            ->scalarNode('respect_cache_headers')->end()
382 18
                            ->variableNode('respect_response_cache_directives')
383 18
                                ->validate()
384
                                ->always(function ($v) {
385 2
                                    if (is_null($v) || is_array($v)) {
386 2
                                        return $v;
387
                                    }
388
                                    throw new InvalidTypeException();
389 18
                                })
390 18
                                ->end()
391 18
                            ->defaultNull()
392 18
                            ->end()
393 18
                        ->end()
394 18
                    ->end()
395 18
                ->end()
396 18
            ->end();
397
        // End cache plugin
398
399 18
        $children->arrayNode('cookie')
400 18
            ->canBeEnabled()
401 18
                ->children()
402 18
                    ->scalarNode('cookie_jar')
403 18
                        ->info('This must be a service id to a service implementing Http\Message\CookieJar')
404 18
                        ->isRequired()
405 18
                        ->cannotBeEmpty()
406 18
                    ->end()
407 18
                ->end()
408 18
            ->end();
409
        // End cookie plugin
410
411 18
        $decoder = $children->arrayNode('decoder');
412 18
        $disableAll ? $decoder->canBeEnabled() : $decoder->canBeDisabled();
413 18
        $decoder->addDefaultsIfNotSet()
414 18
            ->children()
415 18
                ->scalarNode('use_content_encoding')->defaultTrue()->end()
416 18
            ->end()
417 18
        ->end();
418
        // End decoder plugin
419
420 18
        $children->arrayNode('history')
421 18
            ->canBeEnabled()
422 18
                ->children()
423 18
                    ->scalarNode('journal')
424 18
                        ->info('This must be a service id to a service implementing Http\Client\Plugin\Journal')
425 18
                        ->isRequired()
426 18
                        ->cannotBeEmpty()
427 18
                    ->end()
428 18
                ->end()
429 18
            ->end();
430
        // End history plugin
431
432 18
        $logger = $children->arrayNode('logger');
433 18
        $disableAll ? $logger->canBeEnabled() : $logger->canBeDisabled();
434 18
        $logger->addDefaultsIfNotSet()
435 18
            ->children()
436 18
                ->scalarNode('logger')
437 18
                    ->info('This must be a service id to a service implementing Psr\Log\LoggerInterface')
438 18
                    ->defaultValue('logger')
439 18
                    ->cannotBeEmpty()
440 18
                ->end()
441 18
                ->scalarNode('formatter')
442 18
                    ->info('This must be a service id to a service implementing Http\Message\Formatter')
443 18
                    ->defaultNull()
444 18
                ->end()
445 18
            ->end()
446 18
        ->end();
447
        // End logger plugin
448
449 18
        $redirect = $children->arrayNode('redirect');
450 18
        $disableAll ? $redirect->canBeEnabled() : $redirect->canBeDisabled();
451 18
        $redirect->addDefaultsIfNotSet()
452 18
            ->children()
453 18
                ->scalarNode('preserve_header')->defaultTrue()->end()
454 18
                ->scalarNode('use_default_for_multiple')->defaultTrue()->end()
455 18
            ->end()
456 18
        ->end();
457
        // End redirect plugin
458
459 18
        $retry = $children->arrayNode('retry');
460 18
        $disableAll ? $retry->canBeEnabled() : $retry->canBeDisabled();
461 18
        $retry->addDefaultsIfNotSet()
462 18
            ->children()
463 18
                ->scalarNode('retry')->defaultValue(1)->end() // TODO: should be called retries for consistency with the class
464 18
            ->end()
465 18
        ->end();
466
        // End retry plugin
467
468 18
        $stopwatch = $children->arrayNode('stopwatch');
469 18
        $disableAll ? $stopwatch->canBeEnabled() : $stopwatch->canBeDisabled();
470 18
        $stopwatch->addDefaultsIfNotSet()
471 18
            ->children()
472 18
                ->scalarNode('stopwatch')
473 18
                    ->info('This must be a service id to a service extending Symfony\Component\Stopwatch\Stopwatch')
474 18
                    ->defaultValue('debug.stopwatch')
475 18
                    ->cannotBeEmpty()
476 18
                ->end()
477 18
            ->end()
478 18
        ->end();
479
        // End stopwatch plugin
480 18
    }
481
482
    /**
483
     * Create configuration for authentication plugin.
484
     *
485
     * @return NodeDefinition Definition for the authentication node in the plugins list.
486
     */
487 18
    private function createAuthenticationPluginNode()
488
    {
489 18
        $builder = new TreeBuilder();
490 18
        $node = $builder->root('authentication');
491
        $node
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method children() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
492 18
            ->useAttributeAsKey('name')
493 18
            ->prototype('array')
494 18
                ->validate()
495 18
                    ->always()
496 18
                    ->then(function ($config) {
497 5
                        switch ($config['type']) {
498 5
                            case 'basic':
499 4
                                $this->validateAuthenticationType(['username', 'password'], $config, 'basic');
500 4
                                break;
501 2
                            case 'bearer':
502 1
                                $this->validateAuthenticationType(['token'], $config, 'bearer');
503 1
                                break;
504 2
                            case 'service':
505 2
                                $this->validateAuthenticationType(['service'], $config, 'service');
506 1
                                break;
507 1
                            case 'wsse':
508 1
                                $this->validateAuthenticationType(['username', 'password'], $config, 'wsse');
509 1
                                break;
510 4
                        }
511
512 4
                        return $config;
513 18
                    })
514 18
                ->end()
515 18
                ->children()
516 18
                    ->enumNode('type')
517 18
                        ->values(['basic', 'bearer', 'wsse', 'service'])
518 18
                        ->isRequired()
519 18
                        ->cannotBeEmpty()
520 18
                    ->end()
521 18
                    ->scalarNode('username')->end()
522 18
                    ->scalarNode('password')->end()
523 18
                    ->scalarNode('token')->end()
524 18
                    ->scalarNode('service')->end()
525 18
                    ->end()
526 18
                ->end()
527 18
            ->end(); // End authentication plugin
528
529 18
        return $node;
530
    }
531
532
    /**
533
     * Validate that the configuration fragment has the specified keys and none other.
534
     *
535
     * @param array  $expected Fields that must exist
536
     * @param array  $actual   Actual configuration hashmap
537
     * @param string $authName Name of authentication method for error messages
538
     *
539
     * @throws InvalidConfigurationException If $actual does not have exactly the keys specified in $expected (plus 'type')
540
     */
541 5
    private function validateAuthenticationType(array $expected, array $actual, $authName)
542
    {
543 5
        unset($actual['type']);
544 5
        $actual = array_keys($actual);
545 5
        sort($actual);
546 5
        sort($expected);
547
548 5
        if ($expected === $actual) {
549 4
            return;
550
        }
551
552 1
        throw new InvalidConfigurationException(sprintf(
553 1
            'Authentication "%s" requires %s but got %s',
554 1
            $authName,
555 1
            implode(', ', $expected),
556 1
            implode(', ', $actual)
557 1
        ));
558
    }
559
}
560