PredisRedisLock::clearLock() - Code Metrics - Inspection of "Add Predis timeout" - arvenil/ninja-mutex - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#31)

by Anthony

created 2017-11-10 11:10 UTC

PredisRedisLock::clearLock() A

↳ Parent: PredisRedisLock

Complexity

Conditions	2
Paths	2

Size

Total Lines	9
Code Lines	5

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	0
CRAP Score	6

Importance

Changes

Metric	Value
dl	0
loc	9
ccs	0
cts	0
cp	0
rs	9.6666
c	0
b	0
f	0
cc	2
eloc	5
nc	2
nop	1
crap	6

<?php
/**
 * This file is part of ninja-mutex.
 *
 * (C) Kamil Dziedzic <[email protected]>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */
namespace NinjaMutex\Lock;

use Predis;

/**
 * Lock implementor using Predis (client library for Redis)
 *
 * @author Kamil Dziedzic <[email protected]>
 */
class PredisRedisLock extends LockAbstract implements LockExpirationInterface
{
    /**
     * Predis connection
     *
     * @var Predis\Client
     */
    protected $client;

    /**
     * @var int Expiration time of the lock in seconds
     */
    protected $expiration = 0;

    /**
     * @param $client Predis\Client
     */
    public function __construct(Predis\Client $client)
    {
        parent::__construct();

        $this->client = $client;
    }

    /**
     * @param int $expiration Expiration time of the lock in seconds
     */
    public function setExpiration($expiration)
    {
        $this->expiration = $expiration;

        // Regenerate the lock information
        $this->lockInformation = $this->generateLockInformation();

    }

    /**
     * @inheritDoc
     */
    protected function generateLockInformation()
    {
        $params = parent::generateLockInformation();
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.
    }
}

        if ($this->expiration) {
            $params[] = time() + $this->expiration;
        }

        return $params;
    }

    /**
     * @param  string $name
     * @param  bool   $blocking
     * @return bool
     */
    protected function getLock($name, $blocking)
    {
        /**
         * Perform the process recommended by Redis for acquiring a lock, from here: https://redis.io/commands/setnx
         * We are "C4" in this example...
         *
         * 1. C4 sends SETNX lock.foo in order to acquire the lock (sets the value if it does not already exist).
         * 2. The crashed client C3 still holds it, so Redis will reply with 0 to C4.
         * 3. C4 sends GET lock.foo to check if the lock expired.
         *    If it is not, it will sleep for some time and retry from the start.
         * 4. Instead, if the lock is expired because the Unix time at lock.foo is older than the current Unix time,
         *    C4 tries to perform:
         *    GETSET lock.foo <current Unix timestamp + lock timeout + 1>
         *    Because of the GETSET semantic, C4 can check if the old value stored at key is still an expired timestamp
         *    If it is, the lock was acquired.
         * 5. If another client, for instance C5, was faster than C4 and acquired the lock with the GETSET operation,
         *    the C4 GETSET operation will return a non expired timestamp.
         *    C4 will simply restart from the first step. Note that even if C4 wrote they key and set the expiry time
         *    a few seconds in the future this is not a problem. C5's timeout will just be a few seconds later.
         */

        $lockValue = serialize($this->getLockInformation());

        if ($this->client->setnx($name, $lockValue)) {
            return true;
        }

        // Check if the existing lock has an expiry time. If it does and it has expired, delete the lock.
        if ($existingValue = $this->client->get($name)) {
            $existingValue = unserialize($existingValue);
            if (!empty($existingValue[3]) && $existingValue[3] <= time()) {
                // The existing lock has expired. We can delete it and take over.
                $newExistingValue = unserialize($this->client->getset($name, $lockValue));

                // GETSET atomically sets key to value and returns the old value that was stored at key.
                // If the old value from getset does not still contain an expired timestamp
                // another probably acquired the lock in the meantime.
                if ($newExistingValue[3] > time()) {
                    return false;
                }

                // Got him.
                return true;
            }
        }

        return false;
    }

    /**
     * Release lock
     *
     * @param  string $name name of lock
     * @return bool
     */
    public function releaseLock($name)
    {
        if (isset($this->locks[$name]) && $this->client->del([$name])) {
            unset($this->locks[$name]);

            return true;
        }

        return false;
    }

    /**
     * Check if lock is locked
     *
     * @param  string $name name of lock
     * @return bool
     */
    public function isLocked($name)
    {
        return null !== $this->client->get($name);
    }

    /**
     * Clear lock without releasing it
     * Do not use this method unless you know what you do
     *
     * @param  string $name name of lock
     * @return bool
     */
    public function clearLock($name)
    {
        if (!isset($this->locks[$name])) {
            return false;
        }

        unset($this->locks[$name]);
        return true;
    }
}


1		<?php
2		/**
3		* This file is part of ninja-mutex.
4		*
5		* (C) Kamil Dziedzic <[email protected]>
6		*
7		* For the full copyright and license information, please view the LICENSE
8		* file that was distributed with this source code.
9		*/
10		namespace NinjaMutex\Lock;
11
12		use Predis;
13
14		/**
15		* Lock implementor using Predis (client library for Redis)
16		*
17		* @author Kamil Dziedzic <[email protected]>
18		*/
19		class PredisRedisLock extends LockAbstract implements LockExpirationInterface
20		{
21		/**
22		* Predis connection
23		*
24		* @var Predis\Client
25		*/
26		protected $client;
27
28		/**
29		* @var int Expiration time of the lock in seconds
30		*/
31		protected $expiration = 0;
32
33		/**
34		* @param $client Predis\Client
35		*/
36		public function __construct(Predis\Client $client)
37		{
38		parent::__construct();
39
40		$this->client = $client;
41		}
42
43	33	/**
44		* @param int $expiration Expiration time of the lock in seconds
45	33	*/
46	4	public function setExpiration($expiration)
47		{
48		$this->expiration = $expiration;
49	33
50		// Regenerate the lock information
51		$this->lockInformation = $this->generateLockInformation();
		0 ignored issues – show Bug introduced 2017-11-10 11:16 UTC by Report Bug Copy Issue Report The property `lockInformation` does not seem to exist. Did you mean `lockInformationProvider`? An attempt at access to an undefined property has been detected. This may either be a typographical error or the property has been renamed but there are still references to its old name. If you really want to allow access to undefined properties, you can define magic methods to allow access. See the php core documentation on Overloading. Loading history...
52		}
53
54		/**
55		* @inheritDoc
56		*/
57		protected function generateLockInformation()
58	33	{
59		$params = parent::generateLockInformation();
		0 ignored issues – show Bug introduced 2017-11-10 11:16 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `NinjaMutex\Lock\LockAbstract` as the method `generateLockInformation()` does only exist in the following sub-classes of `NinjaMutex\Lock\LockAbstract`: `NinjaMutex\Lock\PredisRedisLock`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
60	33
61	33	if ($this->expiration) {
62		$params[] = time() + $this->expiration;
63	33	}
64
65		return $params;
66	5	}
67
68		/**
69		* @param string $name
70		* @param bool $blocking
71		* @return bool
72		*/
73		protected function getLock($name, $blocking)
74		{
75	15	/**
76		* Perform the process recommended by Redis for acquiring a lock, from here: https://redis.io/commands/setnx
77	15	* We are "C4" in this example...
78		*
79		* 1. C4 sends SETNX lock.foo in order to acquire the lock (sets the value if it does not already exist).
80		* 2. The crashed client C3 still holds it, so Redis will reply with 0 to C4.
81		* 3. C4 sends GET lock.foo to check if the lock expired.
82		* If it is not, it will sleep for some time and retry from the start.
83		* 4. Instead, if the lock is expired because the Unix time at lock.foo is older than the current Unix time,
84		* C4 tries to perform:
85		* GETSET lock.foo <current Unix timestamp + lock timeout + 1>
86		* Because of the GETSET semantic, C4 can check if the old value stored at key is still an expired timestamp
87		* If it is, the lock was acquired.
88		* 5. If another client, for instance C5, was faster than C4 and acquired the lock with the GETSET operation,
89		* the C4 GETSET operation will return a non expired timestamp.
90		* C4 will simply restart from the first step. Note that even if C4 wrote they key and set the expiry time
91		* a few seconds in the future this is not a problem. C5's timeout will just be a few seconds later.
92		*/
93
94		$lockValue = serialize($this->getLockInformation());
95
96		if ($this->client->setnx($name, $lockValue)) {
97		return true;
98		}
99
100		// Check if the existing lock has an expiry time. If it does and it has expired, delete the lock.
101		if ($existingValue = $this->client->get($name)) {
102		$existingValue = unserialize($existingValue);
103		if (!empty($existingValue[3]) && $existingValue[3] <= time()) {
104		// The existing lock has expired. We can delete it and take over.
105		$newExistingValue = unserialize($this->client->getset($name, $lockValue));
106
107		// GETSET atomically sets key to value and returns the old value that was stored at key.
108		// If the old value from getset does not still contain an expired timestamp
109		// another probably acquired the lock in the meantime.
110		if ($newExistingValue[3] > time()) {
111		return false;
112		}
113
114		// Got him.
115		return true;
116		}
117		}
118
119		return false;
120		}
121
122		/**
123		* Release lock
124		*
125		* @param string $name name of lock
126		* @return bool
127		*/
128		public function releaseLock($name)
129		{
130		if (isset($this->locks[$name]) && $this->client->del([$name])) {
131		unset($this->locks[$name]);
132
133		return true;
134		}
135
136		return false;
137		}
138
139		/**
140		* Check if lock is locked
141		*
142		* @param string $name name of lock
143		* @return bool
144		*/
145		public function isLocked($name)
146		{
147		return null !== $this->client->get($name);
148		}
149
150		/**
151		* Clear lock without releasing it
152		* Do not use this method unless you know what you do
153		*
154		* @param string $name name of lock
155		* @return bool
156		*/
157		public function clearLock($name)
158		{
159		if (!isset($this->locks[$name])) {
160		return false;
161		}
162
163		unset($this->locks[$name]);
164		return true;
165		}
166		}
167

arvenil / ninja-mutex

Pull Request — master (#31)

PredisRedisLock::clearLock() A

Complexity

Size

Duplication

Code Coverage

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like