Completed
Push — master ( fc1bf9...6b91a0 )
by Keith
06:58
created

Configuration::getProvidersNode()   A

Complexity

Conditions 3
Paths 1

Size

Total Lines 71
Code Lines 54

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 46
CRAP Score 3

Importance

Changes 1
Bugs 0 Features 0
Metric Value
dl 0
loc 71
rs 9.1369
c 1
b 0
f 0
ccs 46
cts 47
cp 0.9787
cc 3
eloc 54
nc 1
nop 0
crap 3

How to fix   Long Method   

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
/**
4
 * Copyright 2014 Underground Elephant
5
 *
6
 * Licensed under the Apache License, Version 2.0 (the "License");
7
 * you may not use this file except in compliance with the License.
8
 * You may obtain a copy of the License at
9
 *
10
 *     http://www.apache.org/licenses/LICENSE-2.0
11
 *
12
 * Unless required by applicable law or agreed to in writing, software
13
 * distributed under the License is distributed on an "AS IS" BASIS,
14
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15
 * See the License for the specific language governing permissions and
16
 * limitations under the License.
17
 *
18
 * @package     qpush-bundle
19
 * @copyright   Underground Elephant 2014
20
 * @license     Apache License, Version 2.0
21
 */
22
23
namespace Uecode\Bundle\QPushBundle\DependencyInjection;
24
25
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
26
use Symfony\Component\Config\Definition\ConfigurationInterface;
27
28
/**
29
 * @author Keith Kirk <[email protected]>
30
 */
31
class Configuration implements ConfigurationInterface
32
{
33 1
    public function getConfigTreeBuilder()
34
    {
35 1
        $treeBuilder = new TreeBuilder();
36 1
        $rootNode    = $treeBuilder->root('uecode_qpush');
37
38
        $rootNode
39 1
            ->addDefaultsIfNotSet()
40 1
            ->children()
41 1
                ->scalarNode('cache_service')
42 1
                    ->defaultNull()
43 1
                ->end()
44 1
                ->booleanNode('logging_enabled')
45 1
                    ->defaultTrue()
46 1
                ->end()
47 1
                ->append($this->getProvidersNode())
48 1
                ->append($this->getQueuesNode())
49 1
            ->end()
50
        ;
51
52 1
        return $treeBuilder;
53
    }
54
55 1
    private function getProvidersNode()
56
    {
57 1
        $treeBuilder    = new TreeBuilder();
58 1
        $node           = $treeBuilder->root('providers');
59
        $requirements   = [
60 1
            'aws' => [],
61
            'ironmq' => ['token', 'project_id'],
62
            'sync' => [],
63
            'custom' => ['service'],
64
            'file' => ['path'],
65
            'doctrine' => []
66
        ];
67
68
        $node
69 1
            ->useAttributeAsKey('name')
70 1
            ->prototype('array')
71 1
                ->treatNullLike([])
72 1
                ->children()
73 1
                    ->enumNode('driver')
74 1
                        ->isRequired()
75 1
                        ->values(array_keys($requirements))
76 1
                    ->end()
77
                    // IronMQ
78 1
                    ->scalarNode('token')->end()
79 1
                    ->scalarNode('project_id')->end()
80 1
                    ->scalarNode('service')->end()
81 1
                    ->enumNode('host')
82 1
                        ->defaultValue('mq-aws-eu-west-1-1')
83 1
                        ->values([
84 1
                            'mq-aws-eu-west-1-1',
85
                            'mq-aws-us-east-1-1',
86
                        ])
87 1
                    ->end()
88 1
                    ->scalarNode('port')
89 1
                        ->defaultValue('443')
90 1
                    ->end()
91 1
                    ->scalarNode('api_version')
92 1
                        ->defaultValue(3)
93 1
                    ->end()
94
                    // AWS
95 1
                    ->scalarNode('key')->end()
96 1
                    ->scalarNode('secret')->end()
97 1
                    ->scalarNode('region')
98 1
                        ->defaultValue('us-east-1')
99 1
                    ->end()
100
                    // File
101 1
                    ->scalarNode('path')->end()
102
                   // Doctrine
103 1
                    ->scalarNode('entity_manager')
104 1
                         ->defaultValue('doctrine.orm.default_entity_manager')
105 1
                    ->end()
106 1
                ->end()
107
108 1
                ->validate()
109 1
                ->always()
110 1
                ->then(function (array $provider) use ($node, $requirements) {
111 1
                    foreach ($requirements[$provider['driver']] as $requirement) {
112 1
                        if (empty($provider[$requirement])) {
113
                            throw new \InvalidArgumentException(
114 1
                                sprintf('%s queue providers must have a %s; none provided', $provider['driver'], $requirement)
115
                            );
116
                        }
117
                    }
118
119 1
                    return $provider;
120 1
                })
121 1
            ->end()
122
        ;
123
124 1
        return $node;
125
    }
126
127 1
    private function getQueuesNode()
128
    {
129 1
        $treeBuilder = new TreeBuilder();
130 1
        $node        = $treeBuilder->root('queues');
131
132
        $node
0 ignored issues
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...
133 1
            ->requiresAtLeastOneElement()
134 1
            ->useAttributeAsKey('name')
135 1
            ->prototype('array')
136 1
                ->children()
137 1
                    ->scalarNode('provider')
138 1
                        ->isRequired()
139 1
                        ->info('The Queue Provider to use')
140 1
                    ->end()
141 1
                    ->arrayNode('options')
142 1
                        ->children()
143 1
                            ->scalarNode('queue_name')
144 1
                                ->defaultNull()
145 1
                                ->info('The actual name of the queue')
146 1
                            ->end()
147 1
                            ->booleanNode('push_notifications')
148 1
                                ->defaultFalse()
149 1
                                ->info('Whether notifications are sent to the subscribers')
150 1
                            ->end()
151 1
                            ->scalarNode('push_type')
152 1
                                ->defaultValue('multicast')
153 1
                                ->info('Whether the push queue is multicast or unicast')
154 1
                                ->example('unicast')
155 1
                            ->end()
156 1
                            ->scalarNode('notification_retries')
157 1
                                ->defaultValue(3)
158 1
                                ->info('How many attempts the Push Notifications are retried if the Subscriber returns an error')
159 1
                                ->example(3)
160 1
                            ->end()
161 1
                            ->scalarNode('notification_retries_delay')
162 1
                                ->defaultValue(60)
163 1
                                ->info('Delay between each Push Notification retry in seconds')
164 1
                                ->example(3)
165 1
                                ->end()
166 1
                            ->scalarNode('message_delay')
167 1
                                ->defaultValue(0)
168 1
                                ->info('How many seconds before messages are inititally visible in the Queue')
169 1
                                ->example(0)
170 1
                            ->end()
171 1
                            ->scalarNode('message_timeout')
172 1
                                ->defaultValue(30)
173 1
                                ->info('How many seconds the Queue hides a message while its being processed')
174 1
                                ->example(30)
175 1
                            ->end()
176 1
                            ->scalarNode('message_expiration')
177 1
                                ->defaultValue(604800)
178 1
                                ->info('How many seconds a message is kept in Queue, the default is 7 days (604800 seconds)')
179 1
                                ->example(604800)
180 1
                            ->end()
181 1
                            ->scalarNode('messages_to_receive')
182 1
                                ->defaultValue(1)
183 1
                                ->info('Max amount of messages to receive at once - an event will be fired for each individually')
184 1
                                ->example(1)
185 1
                            ->end()
186 1
                            ->scalarNode('receive_wait_time')
187 1
                                ->defaultValue(0)
188 1
                                ->info('How many seconds to Long Poll when requesting messages - if supported')
189 1
                                ->example(3)
190 1
                            ->end()
191 1
                            ->scalarNode('rate_limit')
192 1
                                ->defaultValue(-1)
193 1
                                ->info('How many push requests per second will be triggered. -1 for unlimited, 0 disables push')
194 1
                                ->example(1)
195 1
                            ->end()
196 1
                            ->booleanNode('fifo')
197 1
                                ->defaultFalse()
198 1
                                ->info('If the queue is FIFO (aws)')
199 1
                            ->end()
200 1
                            ->booleanNode('content_based_deduplication')
201 1
                                ->defaultFalse()
202 1
                                ->info('If the FIFO queue has content based deduplication (aws)')
203 1
                            ->end()
204 1
                            ->append($this->getSubscribersNode())
205 1
                        ->end()
206 1
                    ->end()
207 1
                ->end()
208 1
            ->end()
209
        ;
210
211 1
        return $node;
212
    }
213
214 1
    private function getSubscribersNode()
215
    {
216 1
        $treeBuilder = new TreeBuilder();
217 1
        $node        = $treeBuilder->root('subscribers');
218
219
        $node
0 ignored issues
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...
220 1
            ->prototype('array')
221 1
                ->children()
222 1
                    ->scalarNode('endpoint')
223 1
                        ->info('The url or email address to notify')
224 1
                        ->example('http://foo.bar/qpush/')
225 1
                    ->end()
226 1
                    ->enumNode('protocol')
227 1
                        ->values(['email', 'http', 'https'])
228 1
                        ->info('The endpoint type')
229 1
                        ->example('http')
230 1
                    ->end()
231 1
                ->end()
232 1
            ->end()
233
        ;
234
235 1
        return $node;
236
    }
237
}
238