Passed
Push — master ( 3d0d87...075b69 )
by Antonio Carlos
09:13
created

IpList   C

Complexity

Total Complexity 70

Size/Duplication

Total Lines 524
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 9

Test Coverage

Coverage 96.99%

Importance

Changes 1
Bugs 0 Features 0
Metric Value
wmc 70
lcom 1
cbo 9
dl 0
loc 524
ccs 161
cts 166
cp 0.9699
rs 5.6163
c 1
b 0
f 0

27 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 4 1
A getNonDatabaseIps() 0 16 1
A removeFromArrayList() 0 5 2
B whichList() 0 14 6
A removeFromArrayListType() 0 16 2
A removeFromDatabaseList() 0 10 2
A toModels() 0 10 2
A makeModel() 0 4 1
A readFile() 0 10 2
A formatIpArray() 0 6 1
A makeArrayOfIps() 0 12 3
A getIpsFromAnything() 0 14 3
B ipArraySearch() 0 12 6
A getAllFromDatabase() 0 8 2
A mergeLists() 0 5 1
A checkSecondaryLists() 0 10 4
C addToList() 0 34 7
A remove() 0 14 2
A addToArrayList() 0 10 2
A find() 0 12 3
A findByCountry() 0 6 3
A addToProperList() 0 6 2
A delete() 0 6 2
B all() 0 23 4
A findIp() 0 10 3
A nonDatabaseFind() 0 8 2
A addToDatabaseList() 0 13 1

How to fix   Complexity   

Complex Class

Complex classes like IpList 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 IpList, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
namespace PragmaRX\Firewall\Repositories;
4
5
use PragmaRX\Firewall\Support\ServiceInstances;
6
use PragmaRX\Firewall\Vendor\Laravel\Models\Firewall as FirewallModel;
7
8
class IpList
9
{
10
    use ServiceInstances;
11
12
    const IP_ADDRESS_LIST_CACHE_NAME = 'firewall.ip_address_list';
13
14
    /**
15
     * @var \PragmaRX\Firewall\Vendor\Laravel\Models\Firewall
16
     */
17
    private $model;
18
19
    /**
20
     * Create instance of DataRepository.
21
     *
22
     * @param \PragmaRX\Firewall\Vendor\Laravel\Models\Firewall $model
23
     */
24 52
    public function __construct(FirewallModel $model)
25
    {
26 52
        $this->model = $model;
27 52
    }
28
29
    /**
30
     * Get a list of non database ip addresses.
31
     *
32
     * @return array
33
     */
34 48
    private function getNonDatabaseIps()
35
    {
36 48
        return array_merge_recursive(
37 48
            array_map(function ($ip) {
38 7
                $ip['whitelisted'] = true;
39
40 7
                return $ip;
41 48
            }, $this->formatIpArray($this->config()->get('whitelist'))),
42
43 48
            array_map(function ($ip) {
44 16
                $ip['whitelisted'] = false;
45
46 16
                return $ip;
47 48
            }, $this->formatIpArray($this->config()->get('blacklist')))
48
        );
49
    }
50
51
    /**
52
     * Remove ip from all array lists.
53
     *
54
     * @param $ip
55
     *
56
     * @return bool
57
     */
58 3
    private function removeFromArrayList($ip)
59
    {
60 3
        return $this->removeFromArrayListType('whitelist', $ip) ||
61 3
            $this->removeFromArrayListType('blacklist', $ip);
62
    }
63
64
    /**
65
     * Remove the ip address from an array list.
66
     *
67
     * @param $type
68
     * @param $ip
69
     *
70
     * @return bool
71
     */
72 3
    private function removeFromArrayListType($type, $ip)
73
    {
74 3
        if (($key = array_search($ip, $data = $this->config()->get($type))) !== false) {
75 3
            unset($data[$key]);
76
77 3
            $this->cache()->forget($ip);
78
79 3
            $this->config()->set($type, $data);
80
81 3
            $this->messages()->addMessage(sprintf('%s removed from %s', $ip, $type));
82
83 3
            return true;
84
        }
85
86 1
        return false;
87
    }
88
89
    /**
90
     * Remove ip from database.
91
     *
92
     * @param \Illuminate\Database\Eloquent\Model $ip
93
     *
94
     * @return bool
95
     */
96 4
    private function removeFromDatabaseList($ip)
97
    {
98 4
        $ip = $this->find($ip);
99
100 4
        $ip->delete();
0 ignored issues
show
Bug introduced by
The call to delete() misses a required argument $ip.

This check looks for function calls that miss required arguments.

Loading history...
Bug introduced by
The method delete does only exist in Illuminate\Database\Eloq...all\Repositories\IpList, but not in Illuminate\Contracts\Cache\Repository.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
101
102 4
        $this->cache()->forget($ip->ip_address);
103
104 4
        $this->messages()->addMessage(sprintf('%s removed from %s', $ip, $ip->whitelisted ? 'whitelist' : 'blacklist'));
105 4
    }
106
107
    /**
108
     * Transform a list of ips to a list of models.
109
     *
110
     * @param $ipList
111
     *
112
     * @return \Illuminate\Support\Collection
113
     */
114 48
    private function toModels($ipList)
115
    {
116 48
        $ips = [];
117
118 48
        foreach ($ipList as $ip) {
119 7
            $ips[] = $this->makeModel($ip);
120
        }
121
122 48
        return collect($ips);
123
    }
124
125
    /**
126
     * Make a model instance.
127
     *
128
     * @param $ip
129
     *
130
     * @return \Illuminate\Database\Eloquent\Model
131
     */
132 21
    private function makeModel($ip)
133
    {
134 21
        return $this->model->newInstance($ip);
135
    }
136
137
    /**
138
     * Read a file contents.
139
     *
140
     * @param $file
141
     *
142
     * @return array
143
     */
144 1
    private function readFile($file)
145
    {
146 1
        if ($this->fileSystem()->exists($file)) {
147 1
            $lines = file($file, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
148
149 1
            return $this->makeArrayOfIps($lines);
150
        }
151
152 1
        return [];
153
    }
154
155
    /**
156
     * Format all ips in an array.
157
     *
158
     * @param $list
159
     *
160
     * @return array
161
     */
162
    private function formatIpArray($list)
163
    {
164 48
        return array_map(function ($ip) {
165 21
            return ['ip_address' => $ip];
166 48
        }, $this->makeArrayOfIps($list));
167
    }
168
169
    /**
170
     * Make a list of arrays from all sort of things.
171
     *
172
     * @param $list
173
     *
174
     * @return array
175
     */
176 48
    private function makeArrayOfIps($list)
177
    {
178 48
        $list = (array) $list ?: [];
179
180 48
        $ips = [];
181
182 48
        foreach ($list as $item) {
183 21
            $ips = array_merge($ips, $this->getIpsFromAnything($item));
184
        }
185
186 48
        return $ips;
187
    }
188
189
    /**
190
     * Get a list of ips from anything.
191
     *
192
     * @param $item
193
     *
194
     * @return array
195
     */
196 21
    private function getIpsFromAnything($item)
197
    {
198 21
        if (starts_with($item, 'country:')) {
199 2
            return [$item];
200
        }
201
202 19
        $item = $this->ipAddress()->hostToIp($item);
203
204 19
        if ($this->ipAddress()->ipV4Valid($item)) {
205 19
            return [$item];
206
        }
207
208 1
        return $this->readFile($item);
209
    }
210
211
    /**
212
     * Search for an ip in alist of ips.
213
     *
214
     * @param $ip
215
     * @param $ips
216
     *
217
     * @return null|\Illuminate\Database\Eloquent\Model
218
     */
219 47
    private function ipArraySearch($ip, $ips)
220
    {
221 47
        foreach ($ips as $key => $value) {
222
            if (
223 21
                (isset($value['ip_address']) && $value['ip_address'] == $ip) ||
224 10
                (strval($key) == $ip) ||
225 10
                ($value == $ip)
226
            ) {
227 17
                return $value;
228
            }
229
        }
230 47
    }
231
232
    /**
233
     * Get all IPs from database.
234
     *
235
     * @return \Illuminate\Support\Collection
236
     */
237 48
    private function getAllFromDatabase()
238
    {
239 48
        if ($this->config()->get('use_database')) {
240 18
            return $this->model->all();
0 ignored issues
show
Bug Compatibility introduced by
The expression $this->model->all(); of type Illuminate\Database\Eloq...base\Eloquent\Builder[] adds the type Illuminate\Database\Eloquent\Builder[] to the return on line 240 which is incompatible with the return type documented by PragmaRX\Firewall\Reposi...ist::getAllFromDatabase of type Illuminate\Support\Collection.
Loading history...
241
        } else {
242 30
            return collect([]);
243
        }
244
    }
245
246
    /**
247
     * Merge IP lists.
248
     *
249
     * @param $database_ips
250
     * @param $config_ips
251
     *
252
     * @return \Illuminate\Support\Collection
253
     */
254 48
    private function mergeLists($database_ips, $config_ips)
255
    {
256 48
        return collect($database_ips)
257 48
            ->merge(collect($config_ips));
258
    }
259
260
    /**
261
     * Check if an IP address is in a secondary (black/white) list.
262
     *
263
     * @param $ip_address
264
     *
265
     * @return bool|array
266
     */
267 47
    public function checkSecondaryLists($ip_address)
268
    {
269 47
        foreach ($this->all() as $range) {
270 14
            if ($this->ipAddress()->hostToIp($range->ip_address) == $ip_address || $this->ipAddress()->validRange($ip_address, $range)) {
271 4
                return $range;
272
            }
273
        }
274
275 46
        return false;
276
    }
277
278
    /**
279
     * Add an IP to black or whitelist.
280
     *
281
     * @param $whitelist
282
     * @param $ip
283
     * @param bool $force
284
     *
285
     * @return bool
286
     */
287 41
    public function addToList($whitelist, $ip, $force = false)
288
    {
289 41
        $list = $whitelist
290 17
            ? 'whitelist'
291 41
            : 'blacklist';
292
293 41
        if (!$this->ipAddress()->isValid($ip)) {
294 5
            return false;
295
        }
296
297 36
        $listed = $this->whichList($ip);
298
299 36
        if ($listed == $list) {
300 2
            $this->messages()->addMessage(sprintf('%s is already %s', $ip, $list.'ed'));
301
302 2
            return false;
303
        } else {
304 36
            if (empty($listed) || $force) {
305 36
                if (!empty($listed)) {
306 2
                    $this->remove($ip);
307
                }
308
309 36
                $this->addToProperList($whitelist, $ip);
310
311 36
                $this->messages()->addMessage(sprintf('%s is now %s', $ip, $list.'ed'));
312
313 36
                return true;
314
            }
315
        }
316
317 2
        $this->messages()->addMessage(sprintf('%s is currently %sed', $ip, $listed));
318
319 2
        return false;
320
    }
321
322
    /**
323
     * Tell in which list (black/white) an IP address is.
324
     *
325
     * @param $ip_address
326
     *
327
     * @return null|string
328
     */
329 47
    public function whichList($ip_address)
330
    {
331 47
        if (!$ip_found = $this->find($ip_address)) {
332 47
            if (!$ip_found = $this->findByCountry($ip_address)) {
333 47
                if (!$ip_found = $this->checkSecondaryLists($ip_address)) {
334 46
                    return;
335
                }
336
            }
337
        }
338
339 31
        return !is_null($ip_found)
340 31
            ? ($ip_found['whitelisted'] ? 'whitelist' : 'blacklist')
341 31
            : null;
342
    }
343
344
    /**
345
     * Remove IP from all lists.
346
     *
347
     * @param $ip
348
     *
349
     * @return bool
350
     */
351 7
    public function remove($ip)
352
    {
353 7
        $listed = $this->whichList($ip);
354
355 7
        if (!empty($listed)) {
356 7
            $this->delete($ip);
357
358 7
            return true;
359
        }
360
361 2
        $this->messages()->addMessage(sprintf('%s is not listed', $ip));
362
363 2
        return false;
364
    }
365
366
    /**
367
     * Add ip or range to array list.
368
     *
369
     * @param $whitelist
370
     * @param $ip
371
     *
372
     * @return array|mixed
373
     */
374 20
    private function addToArrayList($whitelist, $ip)
375
    {
376 20
        $data = $this->config()->get($list = $whitelist ? 'whitelist' : 'blacklist');
377
378 20
        $data[] = $ip;
379
380 20
        $this->config()->set($list, $data);
381
382 20
        return $data;
383
    }
384
385
    /**
386
     * Find an IP address in the data source.
387
     *
388
     * @param string $ip
389
     *
390
     * @return mixed
391
     */
392 47
    public function find($ip)
393
    {
394 47
        if ($this->cache()->has($ip)) {
395 18
            return $this->cache()->get($ip);
396
        }
397
398 47
        if ($model = $this->findIp($ip)) {
399 17
            $this->cache()->remember($model);
400
        }
401
402 47
        return $model;
403
    }
404
405
    /**
406
     * Find an IP address by country.
407
     *
408
     * @param $country
409
     *
410
     * @return mixed
411
     */
412 47
    public function findByCountry($country)
413
    {
414 47
        if ($this->config()->get('enable_country_search') && !is_null($country = $this->countries()->makeCountryFromString($country))) {
415 19
            return $this->find($country);
416
        }
417 28
    }
418
419
    /**
420
     * Find a Ip in the data source.
421
     *
422
     * @param string $ip
423
     *
424
     * @return void
425
     */
426 36
    public function addToProperList($whitelist, $ip)
427
    {
428 36
        $this->config()->get('use_database') ?
429 16
            $this->addToDatabaseList($whitelist, $ip) :
430 20
            $this->addToArrayList($whitelist, $ip);
431 36
    }
432
433
    /**
434
     * Delete ip address.
435
     *
436
     * @param $ip
437
     *
438
     * @return bool|void
439
     */
440 7
    public function delete($ip)
441
    {
442 7
        $this->config()->get('use_database') ?
443 4
            $this->removeFromDatabaseList($ip) :
444 3
            $this->removeFromArrayList($ip);
445 7
    }
446
447
    /**
448
     * Get all IP addresses.
449
     *
450
     * @return \Illuminate\Support\Collection
451
     */
452 48
    public function all()
453
    {
454 48
        $cacheTime = $this->config()->get('ip_list_cache_expire_time');
455
456 48
        if ($cacheTime > 0 && $list = $this->cache()->get(static::IP_ADDRESS_LIST_CACHE_NAME)) {
457
            return $list;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $list; (Illuminate\Contracts\Cache\Repository) is incompatible with the return type documented by PragmaRX\Firewall\Repositories\IpList::all of type Illuminate\Support\Collection.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
458
        }
459
460 48
        $list = $this->mergeLists(
461 48
            $this->getAllFromDatabase(),
462 48
            $this->toModels($this->getNonDatabaseIps())
463
        );
464
465 48
        if ($cacheTime > 0) {
466
            $this->cache()->put(
467
                static::IP_ADDRESS_LIST_CACHE_NAME,
468
                $list,
469
                $cacheTime
470
            );
471
        }
472
473 48
        return $list;
474
    }
475
476
    /**
477
     * Find ip address in all lists.
478
     *
479
     * @param $ip
480
     *
481
     * @return \Illuminate\Database\Eloquent\Model|null|static
482
     */
483 47
    private function findIp($ip)
484
    {
485 47
        if ($model = $this->nonDatabaseFind($ip)) {
486 17
            return $model;
487
        }
488
489 47
        if ($this->config()->get('use_database')) {
490 17
            return $this->model->where('ip_address', $ip)->first();
0 ignored issues
show
Documentation Bug introduced by
The method where does not exist on object<PragmaRX\Firewall...aravel\Models\Firewall>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
491
        }
492 30
    }
493
494
    /**
495
     * Find ip in non database lists.
496
     *
497
     * @param $ip
498
     *
499
     * @return \Illuminate\Database\Eloquent\Model
500
     */
501 47
    private function nonDatabaseFind($ip)
502
    {
503 47
        $ips = $this->getNonDatabaseIps();
504
505 47
        if ($ip = $this->ipArraySearch($ip, $ips)) {
506 17
            return $this->makeModel($ip);
507
        }
508 47
    }
509
510
    /**
511
     * Add ip or range to database.
512
     *
513
     * @param $whitelist
514
     * @param $ip
515
     *
516
     * @return \Illuminate\Database\Eloquent\Model
517
     */
518 16
    private function addToDatabaseList($whitelist, $ip)
519
    {
520 16
        $this->model->unguard();
521
522 16
        $model = $this->model->create([
0 ignored issues
show
Bug introduced by
The method create() does not exist on PragmaRX\Firewall\Vendor\Laravel\Models\Firewall. Did you maybe mean created()?

This check marks calls to methods that do not seem to exist on an object.

This is most likely the result of a method being renamed without all references to it being renamed likewise.

Loading history...
523 16
            'ip_address'  => $ip,
524 16
            'whitelisted' => $whitelist,
525
        ]);
526
527 16
        $this->cache()->remember($model);
528
529 16
        return $model;
530
    }
531
}
532