Completed
Push — fm-matches ( f867e2...7fc64f )
by Vladimir
14:12
created

HTMLController   C

Complexity

Total Complexity 44

Size/Duplication

Total Lines 332
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 18

Test Coverage

Coverage 94.67%

Importance

Changes 4
Bugs 1 Features 2
Metric Value
wmc 44
c 4
b 1
f 2
lcom 1
cbo 18
dl 0
loc 332
ccs 71
cts 75
cp 0.9467
rs 5.9076

16 Methods

Rating   Name   Duplication   Size   Complexity  
A addTwigGlobals() 0 21 2
A prepareTwig() 0 3 1
A render() 0 6 1
A findModelInParameters() 0 17 4
A callAction() 0 11 4
A saveURL() 0 15 2
A getHomeURL() 0 4 1
A getPreviousURL() 0 15 3
A goBack() 0 4 1
A goHome() 0 4 1
A getFlashBag() 0 4 1
A canSee() 0 8 2
A requireLogin() 0 7 2
B showConfirmationForm() 0 34 4
C decompose() 0 57 13
A assertVisibility() 0 6 2

How to fix   Complexity   

Complex Class

Complex classes like HTMLController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use HTMLController, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
use BZIon\Form\Creator\ConfirmationFormCreator;
4
use BZIon\Twig\AppGlobal;
5
use BZIon\Twig\ModelFetcher;
6
use Symfony\Component\HttpFoundation\RedirectResponse;
7
use Symfony\Component\HttpFoundation\Response;
8
9
/**
10
 * @package BZiON\Controllers
11
 */
12
abstract class HTMLController extends Controller
13
{
14
    /**
15
     * Whether twig has been prepared
16
     * @var bool
17
     */
18
    public $twigReady = false;
19
20
    /**
21
     * Prepare the twig global variables
22
     */
23 1
    private function addTwigGlobals()
24
    {
25 1
        if ($this->twigReady) {
26
            return;
27
        }
28
29 1
        $request = $this->getRequest();
30
31
        // Add global variables to the twig templates
32 1
        $twig = $this->container->get('twig');
33 1
        $twig->addGlobal("me",      $this->getMe());
34 1
        $twig->addGlobal("model",   new ModelFetcher());
35 1
        $twig->addGlobal("request", $request);
36 1
        $twig->addGlobal("session", $request->getSession());
37
38 1
        $twig->addGlobal("app", new AppGlobal($this->parent, $this->container));
39
40 1
        $this->prepareTwig();
41
42 1
        $this->twigReady = true;
43 1
    }
44
45 1
    protected function prepareTwig()
46
    {
47 1
    }
48
49
    /**
50
     * {@inheritdoc}
51
     * @param string $view
52
     */
53 1
    protected function render($view, $parameters = array())
54
    {
55 1
        $this->addTwigGlobals();
56
57 1
        return parent::render($view, $parameters);
58
    }
59
60
    /**
61
     * {@inheritdoc}
62
     *
63
     * @throws ModelNotFoundException
64
     */
65 1
    protected function findModelInParameters($modelParameter, $routeParameters)
66
    {
67 1
        $model = parent::findModelInParameters($modelParameter, $routeParameters);
68
69 1
        if (!$model instanceof Model || $modelParameter->getName() === "me") {
70
            // `$me` can be invalid if, for example, no user is currently logged
71
            // in - in this case we can just pass the invalid Player model to
72
            // the controller without complaining
73 1
            return $model;
74 1
        } elseif (!$this->canSee($model)) {
75
            // If the model is not supposed to be visible to the player
76
            // requesting it, pretend it's not there
77 1
            throw new ModelNotFoundException($model->getTypeForHumans());
78
        }
79
80 1
        return $model;
81
    }
82
83
    /**
84
     * {@inheritdoc}
85
     */
86 1
    public function callAction($action = null)
87
    {
88 1
        $response = parent::callAction($action);
89 1
        if (!$response->isRedirection()
90 1
            && !$response->isNotFound()
91 1
            && !$this->getRequest()->isXmlHttpRequest()) {
92 1
            $this->saveURL();
93
        }
94
95 1
        return $response;
96
    }
97
98
    /**
99
     * Save the URL of the current page so that the user can be redirected back to it
100
     * if they login
101
     */
102 1
    protected function saveURL()
103
    {
104 1
        $session = $this->getRequest()->getSession();
105
106 1
        $urls = $session->get('previous_paths', array());
107 1
        array_unshift($urls, $this->getRequest()->getPathInfo());
108
109
        // No need to have more than 4 urls stored on the array
110 1
        while (count($urls) > 4) {
111 1
            array_pop($urls);
112
        }
113
114
        // Store the URLs in the session, removing any duplicate entries
115 1
        $session->set('previous_paths', array_unique($urls));
116 1
    }
117
118
    /**
119
     * Returns the path to the home page
120
     * @return string
121
     */
122 1
    protected function getHomeURL()
123
    {
124 1
        return Service::getGenerator()->generate('index');
125
    }
126
127
    /**
128
     * Returns the URL of the previous page
129
     * @return string
130
     */
131 1
    protected function getPreviousURL()
132
    {
133 1
        $request = $this->getRequest();
134
135 1
        $urls = $request->getSession()->get('previous_paths', array());
136 1
        foreach ($urls as $url) {
137 1
            if ($url != $request->getPathInfo()) {
138
                // Don't redirect to the same page
139 1
                return $request->getBaseUrl() . $url;
140
            }
141
        }
142
143
        // No stored URLs found, just redirect them to the home page
144
        return $this->getHomeURL();
145
    }
146
147
    /**
148
     * Returns a redirect response to the previous page
149
     * @return RedirectResponse
150
     */
151
    protected function goBack()
152
    {
153
        return new RedirectResponse($this->getPreviousURL());
154
    }
155
156
    /**
157
     * Returns a redirect response to the home page
158
     * @return RedirectResponse
159
     */
160 1
    protected function goHome()
161
    {
162 1
        return new RedirectResponse($this->getHomeURL());
163
    }
164
165
    /**
166
     * Get the session's flash bag
167
     * @return Symfony\Component\HttpFoundation\Session\Flash\FlashBag
168
     */
169 1
    public static function getFlashBag()
170
    {
171 1
        return self::getRequest()->getSession()->getFlashBag();
172
    }
173
174
    /**
175
     * Find out whether the currently logged in user can see a model
176
     *
177
     * Apart from the permissions of the user, this method takes the request
178
     * query into consideration to find out if the user wants to see deleted
179
     * models or not.
180
     *
181
     * @param  Model Model Model in question
182
     * @return bool
183
     */
184 1
    public static function canSee($model)
185
    {
186 1
        if (!$model instanceof PermissionModel) {
187
            return !$model->isDeleted();
188
        }
189
190 1
        return static::getMe()->canSee($model, static::getRequest()->get('showDeleted'));
191
    }
192
193
    /**
194
     * Assert that the user is logged in
195
     * @param  string        $message The message to show if the user is not logged in
196
     * @throws HTTPException
197
     * @return void
198
     */
199 1
    protected function requireLogin(
200
        $message = "You need to be signed in to do this"
201
    ) {
202 1
        if (!$this->getMe()->isValid()) {
203 1
            throw new ForbiddenException($message);
204
        }
205 1
    }
206
207
    /**
208
     * Show a confirmation (Yes, No) form to the user
209
     *
210
     * @param  callable $onYes          What to do if the user clicks on "Yes"
211
     * @param  string   $message        The message to show to the user, asking
212
     *                                  them to confirm their action (RAW text
213
     *                                  is shown - don't forget to properly
214
     *                                  escape your parameters!)
215
     * @param  string   $action         The text to show on the "Yes" button
216
     * @param  string   $successMessage A message to add on the session's
217
     *                                  flashbag on success (flashbags
218
     *                                  automatically escape text)
219
     * @param  callable $onNo           What to do if the user presses "No" -
220
     *                                  defaults to redirecting them back
221
     * @param  string   $view           The view to redirect to
222
     * @param  bool  $noButton       Whether to show a "No" instead of a
223
     *                                  "Cancel" button
224
     * @return mixed    The response
225
     */
226 1
    protected function showConfirmationForm(
227
        $onYes,
228
        $message = "Are you sure you want to do this?",
229
        $successMessage = "Operation completed successfully",
230
        $action = "Yes",
231
        $onNo = null,
232
        $view = 'confirmation.html.twig',
233
        $noButton = false
234
    ) {
235 1
        $creator = new ConfirmationFormCreator($action, $this->getPreviousURL(), $noButton);
236 1
        $form = $creator->create()->handleRequest($this->getRequest());
237
238 1
        if ($form->isValid()) {
239 1
            if ($form->get('confirm')->isClicked()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Form\FormInterface as the method isClicked() does only exist in the following implementations of said interface: Symfony\Component\Form\SubmitButton.

Let’s take a look at an example:

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

class MyUser implements 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 implementation 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 interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
240 1
                $return = $onYes();
241
242
                // If no exceptions are thrown, show a success message
243 1
                $this->getFlashBag()->add('success', $successMessage);
244
245 1
                return $return;
246 1
            } elseif (!$onNo) {
247
                // We didn't get told about what to do when the user presses no,
248
                // just get them back where they were
249 1
                return new RedirectResponse($form->get('original_url')->getData());
250
            } else {
251
                return $onNo($form->get('original_url')->getData());
252
            }
253
        }
254
255 1
        return $this->render($view, array(
256 1
            'form'    => $form->createView(),
257 1
            'message' => $message
258
        ));
259
    }
260
261
    /**
262
     * Decompose a list of object IDs into the corresponding IDs
263
     *
264
     * @param string $query  The user's query
265
     * @param array  $types  A list of the acceptable model types (will NOT be sanitized)
266
     * @param bool   $models Whether to return an array of models instead of an array of IDs
267
     * @param int|null $max  The largest number of models to accept, or null for infinite models
268
     *
269
     * @throws BadRequestException
270
     */
271
    protected function decompose($query, array $types, $models = true, $max = null)
272
    {
273
        $query = explode(',', $query);
274
275
        if ($max !== null && count($query) > $max) {
276
            throw new \BadRequestException("Too many objects provided");
277
        }
278
279
        $result = array();
280
        $firstType = reset($types);
281
282
        if (!$models) {
283
            // Make sure the result array has a subarray for each type
284
            foreach ($types as $type) {
285
                $result[$type] = array();
286
            }
287
        }
288
289
        foreach ($query as $object) {
290
            if ($object === '') {
291
                continue;
292
            }
293
294
            $object = explode(':', $object, 3);
295
            if (count($object) === 2) {
296
                $class = ucfirst($object[0]);
297
                $id = (int) $object[1];
298
299
                if (!in_array($class, $types)) {
300
                    throw new \BadRequestException("Invalid object type");
301
                }
302
303
                if ($models) {
304
                    $this->assertVisibility($result[] = $class::get($id));
305
                } else {
306
                    $result[$class][] = $id;
307
                }
308
            } elseif (count($object) === 1) {
309
                // No type was provided
310
                if (count('types') > 1) {
311
                    throw new \BadRequestException(
312
                        "You need to provide the type of the object"
313
                    );
314
                }
315
316
                if ($models) {
317
                    $this->assertVisibility($result[] = $firstType::get($id));
0 ignored issues
show
Bug introduced by
The variable $id does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
318
                } else {
319
                    $result[$firstType][] = (int) $object[0];
320
                }
321
            } else {
322
                throw new \BadRequestException("Malformed object");
323
            }
324
        }
325
326
        return $result;
327
    }
328
329
    /**
330
     * Throw an innocent exception if a player can't see a Model or if it
331
     * doesn't exist
332
     *
333
     * @param $model The model to test
334
     *
335
     * @throws BadRequestException
336
     */
337
    private function assertVisibility(PermissionModel $model)
338
    {
339
        if (!$this->getMe()->canSee($model)) {
340
            throw new \BadRequestException("Invalid object provided");
341
        }
342
    }
343
}
344