Completed
Pull Request — master (#155)
by Rob
04:38 queued 01:36
created

Configuration   B

Complexity

Total Complexity 37

Size/Duplication

Total Lines 596
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 8

Test Coverage

Coverage 100%

Importance

Changes 0
Metric Value
wmc 37
lcom 1
cbo 8
dl 0
loc 596
ccs 257
cts 257
cp 1
rs 8.6
c 0
b 0
f 0

9 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 4 1
C getConfigTreeBuilder() 0 111 12
B configureClients() 0 38 1
A configureSharedPlugins() 0 11 1
B createClientPluginNode() 0 129 6
B addSharedPluginNodes() 0 91 6
B createAuthenticationPluginNode() 0 44 5
A validateAuthenticationType() 0 18 2
B createCachePluginNode() 0 91 3
1
<?php
2
3
namespace Http\HttplugBundle\DependencyInjection;
4
5
use Http\Client\Common\Plugin\Cache\Generator\CacheKeyGenerator;
6
use Http\Client\Common\Plugin\Journal;
7
use Http\Message\CookieJar;
8
use Http\Message\Formatter;
9
use Http\Message\StreamFactory;
10
use Psr\Cache\CacheItemPoolInterface;
11
use Psr\Log\LoggerInterface;
12
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
13
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
14
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
15
use Symfony\Component\Config\Definition\ConfigurationInterface;
16
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
17
use Symfony\Component\Config\Definition\Exception\InvalidTypeException;
18
19
/**
20
 * This class contains the configuration information for the bundle.
21
 *
22
 * This information is solely responsible for how the different configuration
23
 * sections are normalized, and merged.
24
 *
25
 * @author David Buchmann <[email protected]>
26
 * @author Tobias Nyholm <[email protected]>
27
 */
28
class Configuration implements ConfigurationInterface
29
{
30
    /**
31
     * Whether to use the debug mode.
32
     *
33
     * @see https://github.com/doctrine/DoctrineBundle/blob/v1.5.2/DependencyInjection/Configuration.php#L31-L41
34
     *
35
     * @var bool
36
     */
37
    private $debug;
38
39
    /**
40
     * @param bool $debug
41
     */
42 19
    public function __construct($debug)
43
    {
44 19
        $this->debug = (bool) $debug;
45 19
    }
46
47
    /**
48
     * {@inheritdoc}
49
     */
50 19
    public function getConfigTreeBuilder()
51
    {
52 19
        $treeBuilder = new TreeBuilder();
53 19
        $rootNode = $treeBuilder->root('httplug');
54
55 19
        $this->configureClients($rootNode);
56 19
        $this->configureSharedPlugins($rootNode);
57
58
        $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...
59 19
            ->validate()
60
                ->ifTrue(function ($v) {
61 15
                    return !empty($v['classes']['client'])
62 15
                        || !empty($v['classes']['message_factory'])
63 12
                        || !empty($v['classes']['uri_factory'])
64 15
                        || !empty($v['classes']['stream_factory']);
65 19
                })
66
                ->then(function ($v) {
67 3
                    foreach ($v['classes'] as $key => $class) {
68 3
                        if (null !== $class && !class_exists($class)) {
69 1
                            throw new InvalidConfigurationException(sprintf(
70 1
                                'Class %s specified for httplug.classes.%s does not exist.',
71 1
                                $class,
72
                                $key
73 1
                            ));
74
                        }
75 2
                    }
76
77 2
                    return $v;
78 19
                })
79 19
            ->end()
80 19
            ->beforeNormalization()
81
                ->ifTrue(function ($v) {
82 19
                    return is_array($v) && array_key_exists('toolbar', $v) && is_array($v['toolbar']);
83 19
                })
84
                ->then(function ($v) {
85 4
                    if (array_key_exists('profiling', $v)) {
86 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".');
87
                    }
88
89 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...
90
91 3
                    if (array_key_exists('enabled', $v['toolbar']) && 'auto' === $v['toolbar']['enabled']) {
92 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...
93 1
                        $v['toolbar']['enabled'] = $this->debug;
94 1
                    }
95
96 3
                    $v['profiling'] = $v['toolbar'];
97
98 3
                    unset($v['toolbar']);
99
100 3
                    return $v;
101 19
                })
102 19
            ->end()
103 19
            ->fixXmlConfig('client')
104 19
            ->children()
105 19
                ->arrayNode('main_alias')
106 19
                    ->addDefaultsIfNotSet()
107 19
                    ->info('Configure which service the main alias point to.')
108 19
                    ->children()
109 19
                        ->scalarNode('client')->defaultValue('httplug.client.default')->end()
110 19
                        ->scalarNode('message_factory')->defaultValue('httplug.message_factory.default')->end()
111 19
                        ->scalarNode('uri_factory')->defaultValue('httplug.uri_factory.default')->end()
112 19
                        ->scalarNode('stream_factory')->defaultValue('httplug.stream_factory.default')->end()
113 19
                    ->end()
114 19
                ->end()
115 19
                ->arrayNode('classes')
116 19
                    ->addDefaultsIfNotSet()
117 19
                    ->info('Overwrite a service class instead of using the discovery mechanism.')
118 19
                    ->children()
119 19
                        ->scalarNode('client')->defaultNull()->end()
120 19
                        ->scalarNode('message_factory')->defaultNull()->end()
121 19
                        ->scalarNode('uri_factory')->defaultNull()->end()
122 19
                        ->scalarNode('stream_factory')->defaultNull()->end()
123 19
                    ->end()
124 19
                ->end()
125 19
                ->arrayNode('profiling')
126 19
                    ->addDefaultsIfNotSet()
127 19
                    ->treatFalseLike(['enabled' => false])
128 19
                    ->treatTrueLike(['enabled' => true])
129 19
                    ->treatNullLike(['enabled' => $this->debug])
130 19
                    ->info('Extend the debug profiler with information about requests.')
131 19
                    ->children()
132 19
                        ->booleanNode('enabled')
133 19
                            ->info('Turn the toolbar on or off. Defaults to kernel debug mode.')
134 19
                            ->defaultValue($this->debug)
135 19
                        ->end()
136 19
                        ->scalarNode('formatter')->defaultNull()->end()
137 19
                        ->integerNode('captured_body_length')
138 19
                            ->defaultValue(0)
139 19
                            ->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).')
140 19
                        ->end()
141 19
                    ->end()
142 19
                ->end()
143 19
                ->arrayNode('discovery')
144 19
                    ->addDefaultsIfNotSet()
145 19
                    ->info('Control what clients should be found by the discovery.')
146 19
                    ->children()
147 19
                        ->scalarNode('client')
148 19
                            ->defaultValue('auto')
149 19
                            ->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.')
150 19
                        ->end()
151 19
                        ->scalarNode('async_client')
152 19
                            ->defaultNull()
153 19
                            ->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.')
154 19
                        ->end()
155 19
                    ->end()
156 19
                ->end()
157 19
            ->end();
158
159 19
        return $treeBuilder;
160
    }
161
162 19
    private function configureClients(ArrayNodeDefinition $root)
163
    {
164 19
        $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...
165 19
            ->arrayNode('clients')
166 19
                ->useAttributeAsKey('name')
167 19
                ->prototype('array')
168 19
                ->fixXmlConfig('plugin')
169 19
                ->validate()
170
                    ->ifTrue(function ($config) {
171
                        // Make sure we only allow one of these to be true
172 7
                        return (bool) $config['flexible_client'] + (bool) $config['http_methods_client'] + (bool) $config['batch_client'] >= 2;
173 19
                    })
174 19
                    ->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")')
175 19
                ->end()
176 19
                ->children()
177 19
                    ->scalarNode('factory')
178 19
                        ->isRequired()
179 19
                        ->cannotBeEmpty()
180 19
                        ->info('The service id of a factory to use when creating the adapter.')
181 19
                    ->end()
182 19
                    ->booleanNode('flexible_client')
183 19
                        ->defaultFalse()
184 19
                        ->info('Set to true to get the client wrapped in a FlexibleHttpClient which emulates async or sync behavior.')
185 19
                    ->end()
186 19
                    ->booleanNode('http_methods_client')
187 19
                        ->defaultFalse()
188 19
                        ->info('Set to true to get the client wrapped in a HttpMethodsClient which emulates provides functions for HTTP verbs.')
189 19
                    ->end()
190 19
                    ->booleanNode('batch_client')
191 19
                        ->defaultFalse()
192 19
                        ->info('Set to true to get the client wrapped in a BatchClient which allows you to send multiple request at the same time.')
193 19
                    ->end()
194 19
                    ->variableNode('config')->defaultValue([])->end()
195 19
                    ->append($this->createClientPluginNode())
196 19
                ->end()
197 19
            ->end()
198 19
        ->end();
199 19
    }
200
201
    /**
202
     * @param ArrayNodeDefinition $root
203
     */
204 19
    private function configureSharedPlugins(ArrayNodeDefinition $root)
205
    {
206
        $pluginsNode = $root
207 19
            ->children()
208 19
                ->arrayNode('plugins')
209 19
                ->info('Global plugin configuration. Plugins need to be explicitly added to clients.')
210 19
                ->addDefaultsIfNotSet()
211
            // don't call end to get the plugins node
212 19
        ;
213 19
        $this->addSharedPluginNodes($pluginsNode);
214 19
    }
215
216
    /**
217
     * Createplugins node of a client.
218
     *
219
     * @return ArrayNodeDefinition The plugin node
220
     */
221 19
    private function createClientPluginNode()
222
    {
223 19
        $builder = new TreeBuilder();
224 19
        $node = $builder->root('plugins');
225
226
        /** @var ArrayNodeDefinition $pluginList */
227
        $pluginList = $node
228 19
            ->info('A list of plugin service ids and client specific plugin definitions. The order is important.')
229 19
            ->prototype('array')
230 19
        ;
231
        $pluginList
232
            // support having just a service id in the list
233 19
            ->beforeNormalization()
234
                ->always(function ($plugin) {
235 8
                    if (is_string($plugin)) {
236
                        return [
237
                            'reference' => [
238 7
                                'enabled' => true,
239 7
                                'id' => $plugin,
240 7
                            ],
241 7
                        ];
242
                    }
243
244 5
                    return $plugin;
245 19
                })
246 19
            ->end()
247
248 19
            ->validate()
249 19
                ->always(function ($plugins) {
250 7
                    foreach ($plugins as $name => $definition) {
251 7
                        if ('authentication' === $name) {
252 7
                            if (!count($definition)) {
253 7
                                unset($plugins['authentication']);
254 7
                            }
255 7
                        } elseif (!$definition['enabled']) {
256 7
                            unset($plugins[$name]);
257 7
                        }
258 7
                    }
259
260 7
                    return $plugins;
261 19
                })
262 19
            ->end()
263
        ;
264 19
        $this->addSharedPluginNodes($pluginList, true);
265
266
        $pluginList
267 19
            ->children()
268 19
                ->arrayNode('reference')
269 19
                    ->canBeEnabled()
270 19
                    ->info('Reference to a plugin service')
271 19
                    ->children()
272 19
                        ->scalarNode('id')
273 19
                            ->info('Service id of a plugin')
274 19
                            ->isRequired()
275 19
                            ->cannotBeEmpty()
276 19
                        ->end()
277 19
                    ->end()
278 19
                ->end()
279 19
                ->arrayNode('add_host')
280 19
                    ->canBeEnabled()
281 19
                    ->addDefaultsIfNotSet()
282 19
                    ->info('Set scheme, host and port in the request URI.')
283 19
                    ->children()
284 19
                        ->scalarNode('host')
285 19
                            ->info('Host name including protocol and optionally the port number, e.g. https://api.local:8000')
286 19
                            ->isRequired()
287 19
                            ->cannotBeEmpty()
288 19
                        ->end()
289 19
                        ->scalarNode('replace')
290 19
                            ->info('Whether to replace the host if request already specifies one')
291 19
                            ->defaultValue(false)
292 19
                        ->end()
293 19
                    ->end()
294 19
                ->end()
295 19
                ->arrayNode('header_append')
296 19
                    ->canBeEnabled()
297 19
                    ->info('Append headers to the request. If the header already exists the value will be appended to the current value.')
298 19
                    ->fixXmlConfig('header')
299 19
                    ->children()
300 19
                        ->arrayNode('headers')
301 19
                            ->info('Keys are the header names, values the header values')
302 19
                            ->normalizeKeys(false)
303 19
                            ->useAttributeAsKey('name')
304 19
                            ->prototype('scalar')->end()
305 19
                        ->end()
306 19
                    ->end()
307 19
                ->end()
308 19
                ->arrayNode('header_defaults')
309 19
                    ->canBeEnabled()
310 19
                    ->info('Set header to default value if it does not exist.')
311 19
                    ->fixXmlConfig('header')
312 19
                    ->children()
313 19
                        ->arrayNode('headers')
314 19
                            ->info('Keys are the header names, values the header values')
315 19
                            ->normalizeKeys(false)
316 19
                            ->useAttributeAsKey('name')
317 19
                            ->prototype('scalar')->end()
318 19
                        ->end()
319 19
                    ->end()
320 19
                ->end()
321 19
                ->arrayNode('header_set')
322 19
                    ->canBeEnabled()
323 19
                    ->info('Set headers to requests. If the header does not exist it wil be set, if the header already exists it will be replaced.')
324 19
                    ->fixXmlConfig('header')
325 19
                    ->children()
326 19
                        ->arrayNode('headers')
327 19
                            ->info('Keys are the header names, values the header values')
328 19
                            ->normalizeKeys(false)
329 19
                            ->useAttributeAsKey('name')
330 19
                            ->prototype('scalar')->end()
331 19
                        ->end()
332 19
                    ->end()
333 19
                ->end()
334 19
                ->arrayNode('header_remove')
335 19
                    ->canBeEnabled()
336 19
                    ->info('Remove headers from requests.')
337 19
                    ->fixXmlConfig('header')
338 19
                    ->children()
339 19
                        ->arrayNode('headers')
340 19
                            ->info('List of header names to remove')
341 19
                            ->prototype('scalar')->end()
342 19
                        ->end()
343 19
                    ->end()
344 19
                ->end()
345 19
            ->end()
346 19
        ->end();
347
348 19
        return $node;
349
    }
350
351
    /**
352
     * Add the definitions for shared plugin configurations.
353
     *
354
     * @param ArrayNodeDefinition $pluginNode The node to add to.
355
     * @param bool                $disableAll Some shared plugins are enabled by default. On the client, all are disabled by default.
356
     */
357 19
    private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableAll = false)
358
    {
359 19
        $children = $pluginNode->children();
360
361 19
        $children->append($this->createAuthenticationPluginNode());
362 19
        $children->append($this->createCachePluginNode());
363
364
        $children
365 19
            ->arrayNode('cookie')
366 19
                ->canBeEnabled()
367 19
                ->children()
368 19
                    ->scalarNode('cookie_jar')
369
                        ->info('This must be a service id to a service implementing '.CookieJar::class)
370
                        ->isRequired()
371
                        ->cannotBeEmpty()
372
                    ->end()
373
                ->end()
374
            ->end();
375
        // End cookie plugin
376
377
        $children
378
            ->arrayNode('history')
379
                ->canBeEnabled()
380
                ->children()
381
                    ->scalarNode('journal')
382
                        ->info('This must be a service id to a service implementing '.Journal::class)
383
                        ->isRequired()
384
                        ->cannotBeEmpty()
385
                    ->end()
386
                ->end()
387
            ->end();
388
        // End history plugin
389
390
        $decoder = $children->arrayNode('decoder');
391
        $disableAll ? $decoder->canBeEnabled() : $decoder->canBeDisabled();
392
        $decoder->addDefaultsIfNotSet()
393
            ->children()
394
                ->scalarNode('use_content_encoding')->defaultTrue()->end()
395
            ->end()
396
        ->end();
397
        // End decoder plugin
398
399
        $logger = $children->arrayNode('logger');
400
        $disableAll ? $logger->canBeEnabled() : $logger->canBeDisabled();
401
        $logger->addDefaultsIfNotSet()
402
            ->children()
403
                ->scalarNode('logger')
404
                    ->info('This must be a service id to a service implementing '.LoggerInterface::class)
405
                    ->defaultValue('logger')
406
                    ->cannotBeEmpty()
407
                ->end()
408
                ->scalarNode('formatter')
409
                    ->info('This must be a service id to a service implementing '.Formatter::class)
410
                    ->defaultNull()
411
                ->end()
412
            ->end()
413
        ->end();
414
        // End logger plugin
415
416
        $redirect = $children->arrayNode('redirect');
417
        $disableAll ? $redirect->canBeEnabled() : $redirect->canBeDisabled();
418
        $redirect->addDefaultsIfNotSet()
419
            ->children()
420
                ->scalarNode('preserve_header')->defaultTrue()->end()
421
                ->scalarNode('use_default_for_multiple')->defaultTrue()->end()
422
            ->end()
423
        ->end();
424
        // End redirect plugin
425
426
        $retry = $children->arrayNode('retry');
427
        $disableAll ? $retry->canBeEnabled() : $retry->canBeDisabled();
428
        $retry->addDefaultsIfNotSet()
429
            ->children()
430
                ->scalarNode('retry')->defaultValue(1)->end() // TODO: should be called retries for consistency with the class
431
            ->end()
432
        ->end();
433
        // End retry plugin
434
435
        $stopwatch = $children->arrayNode('stopwatch');
436
        $disableAll ? $stopwatch->canBeEnabled() : $stopwatch->canBeDisabled();
437
        $stopwatch->addDefaultsIfNotSet()
438
            ->children()
439
                ->scalarNode('stopwatch')
440
                    ->info('This must be a service id to a service extending Symfony\Component\Stopwatch\Stopwatch')
441
                    ->defaultValue('debug.stopwatch')
442
                    ->cannotBeEmpty()
443
                ->end()
444
            ->end()
445
        ->end();
446
        // End stopwatch plugin
447
    }
448
449
    /**
450
     * Create configuration for authentication plugin.
451
     *
452
     * @return NodeDefinition Definition for the authentication node in the plugins list.
453
     */
454
    private function createAuthenticationPluginNode()
455
    {
456
        $builder = new TreeBuilder();
457
        $node = $builder->root('authentication');
458
        $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...
459
            ->useAttributeAsKey('name')
460
            ->prototype('array')
461
                ->validate()
462
                    ->always()
463
                    ->then(function ($config) {
464
                        switch ($config['type']) {
465
                            case 'basic':
466
                                $this->validateAuthenticationType(['username', 'password'], $config, 'basic');
467
                                break;
468
                            case 'bearer':
469
                                $this->validateAuthenticationType(['token'], $config, 'bearer');
470
                                break;
471
                            case 'service':
472
                                $this->validateAuthenticationType(['service'], $config, 'service');
473
                                break;
474
                            case 'wsse':
475
                                $this->validateAuthenticationType(['username', 'password'], $config, 'wsse');
476
                                break;
477
                        }
478
479
                        return $config;
480
                    })
481
                ->end()
482
                ->children()
483
                    ->enumNode('type')
484
                        ->values(['basic', 'bearer', 'wsse', 'service'])
485
                        ->isRequired()
486
                        ->cannotBeEmpty()
487
                    ->end()
488
                    ->scalarNode('username')->end()
489
                    ->scalarNode('password')->end()
490
                    ->scalarNode('token')->end()
491
                    ->scalarNode('service')->end()
492
                    ->end()
493
                ->end()
494
            ->end(); // End authentication plugin
495
496
        return $node;
497
    }
498
499
    /**
500
     * Validate that the configuration fragment has the specified keys and none other.
501
     *
502
     * @param array  $expected Fields that must exist
503
     * @param array  $actual   Actual configuration hashmap
504
     * @param string $authName Name of authentication method for error messages
505
     *
506
     * @throws InvalidConfigurationException If $actual does not have exactly the keys specified in $expected (plus 'type')
507
     */
508
    private function validateAuthenticationType(array $expected, array $actual, $authName)
509
    {
510
        unset($actual['type']);
511
        $actual = array_keys($actual);
512
        sort($actual);
513
        sort($expected);
514
515
        if ($expected === $actual) {
516
            return;
517
        }
518
519
        throw new InvalidConfigurationException(sprintf(
520
            'Authentication "%s" requires %s but got %s',
521
            $authName,
522
            implode(', ', $expected),
523
            implode(', ', $actual)
524
        ));
525
    }
526
527
    /**
528
     * Create configuration for cache plugin.
529
     *
530
     * @return NodeDefinition Definition for the cache node in the plugins list.
531
     */
532
    private function createCachePluginNode()
533
    {
534
        $builder = new TreeBuilder();
535
536
        $config = $builder->root('config');
537
        $config
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...
538
            ->fixXmlConfig('method')
539
            ->fixXmlConfig('respect_response_cache_directive')
540
            ->addDefaultsIfNotSet()
541
            ->validate()
542
                ->ifTrue(function ($config) {
543
                    // Cannot set both respect_cache_headers and respect_response_cache_directives
544
                    return isset($config['respect_cache_headers'], $config['respect_response_cache_directives']);
545
                })
546
                ->thenInvalid('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives" simultaniously. Use "respect_response_cache_directives" instead.')
547
            ->end()
548
            ->children()
549
                ->scalarNode('cache_key_generator')
550
                    ->info('This must be a service id to a service implementing '.CacheKeyGenerator::class)
551
                ->end()
552
                ->integerNode('cache_lifetime')
553
                    ->info('The minimum time we should store a cache item')
554
                ->end()
555
                ->integerNode('default_ttl')
556
                    ->info('The default max age of a Response')
557
                ->end()
558
                ->enumNode('hash_algo')
559
                    ->info('Hashing algorithm to use')
560
                    ->values(hash_algos())
561
                    ->cannotBeEmpty()
562
                ->end()
563
                ->arrayNode('methods')
564
                    ->info('Which request methods to cache')
565
                    ->defaultValue(['GET', 'HEAD'])
566
                    ->prototype('scalar')
567
                        ->validate()
568
                            ->ifTrue(function ($v) {
569
                                /* RFC7230 sections 3.1.1 and 3.2.6 except limited to uppercase characters. */
570
                                return preg_match('/[^A-Z0-9!#$%&\'*+\-.^_`|~]+/', $v);
571
                            })
572
                            ->thenInvalid('Invalid method: %s')
573
                        ->end()
574
                    ->end()
575
                ->end()
576
                ->enumNode('respect_cache_headers')
577
                    ->info('Whether we should care about cache headers or not [DEPRECATED]')
578
                    ->values([null, true, false])
579
                    ->beforeNormalization()
580
                        ->always(function ($v) {
581
                            @trigger_error('The option "respect_cache_headers" is deprecated since version 1.3 and will be removed in 2.0. Use "respect_response_cache_directives" 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...
582
583
                            return $v;
584
                        })
585
                    ->end()
586
                ->end()
587
                ->variableNode('respect_response_cache_directives')
588
                    ->info('A list of cache directives to respect when caching responses')
589
                    ->validate()
590
                        ->always(function ($v) {
591
                            if (is_null($v) || is_array($v)) {
592
                                return $v;
593
                            }
594
595
                            throw new InvalidTypeException();
596
                        })
597
                    ->end()
598
                ->end()
599
            ->end()
600
        ;
601
602
        $cache = $builder->root('cache');
603
        $cache
604
            ->canBeEnabled()
605
            ->addDefaultsIfNotSet()
606
            ->children()
607
                ->scalarNode('cache_pool')
608
                    ->info('This must be a service id to a service implementing '.CacheItemPoolInterface::class)
609
                    ->isRequired()
610
                    ->cannotBeEmpty()
611
                ->end()
612
                ->scalarNode('stream_factory')
613
                    ->info('This must be a service id to a service implementing '.StreamFactory::class)
614
                    ->defaultValue('httplug.stream_factory')
615
                    ->cannotBeEmpty()
616
                ->end()
617
            ->end()
618
            ->append($config)
619
        ;
620
621
        return $cache;
622
    }
623
}
624