CronService::__construct()   A
last analyzed

Complexity

Conditions 1
Paths 1

Size

Total Lines 5
Code Lines 3

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 5
rs 9.4285
c 0
b 0
f 0
cc 1
eloc 3
nc 1
nop 2
1
<?php
2
3
namespace T4web\Cron\Service;
4
5
use Cron\Cron;
6
use T4web\Cron\Exception\TimeoutException;
7
8
class CronService
9
{
10
    /**
11
     * @var integer
12
     */
13
    protected $timeout = null;
14
15
    /**
16
     * @var Cron
17
     */
18
    protected $cron = null;
19
20
    /**
21
     * @var int
22
     */
23
    protected $startTime = null;
24
25
    public function __construct($timeout, Cron $cron)
26
    {
27
        $this->timeout = $timeout;
28
        $this->cron = $cron;
29
    }
30
31
    public function run()
32
    {
33
        $this->startTime = time();
34
35
        $this->cron->run();
36
        $this->wait();
37
        $this->throwErrorIfTimeout();
38
    }
39
40
    protected function wait()
41
    {
42
        do {
43
            sleep(1);
44
        } while ($this->cron->isRunning() && !$this->checkTimeout());
45
    }
46
47
    protected function checkTimeout()
48
    {
49
        if (is_null($this->timeout)) {
50
            return false;
51
        }
52
53
        if ($this->timeout > (time() - $this->startTime)) {
54
            return false;
55
        }
56
57
        return true;
58
    }
59
60
    protected function throwErrorIfTimeout()
61
    {
62
        if ($this->checkTimeout()) {
63
            throw new TimeoutException($this->assembleErrorString());
64
        }
65
    }
66
67
    protected function assembleErrorString()
68
    {
69
        $string = 'Jobs: ' . PHP_EOL;
70
        $i = 1;
71
        foreach ($this->cron->getExecutor()->getRunningJobs() as $job) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Cron\Executor\ExecutorInterface as the method getRunningJobs() does only exist in the following implementations of said interface: T4web\Cron\Executor\Executor.

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...
72
            $string .= $i . '. ' . $job->getProcess()->getCommandLine() . PHP_EOL;
73
            $i++;
74
        }
75
76
        return $string . ' have taken over ' . $this->timeout . ' seconds to execute.';
77
    }
78
}
79