Skip to content

Commit

Permalink
feature #32198 [Lock] Split "StoreInterface" into multiple interfaces…
Browse files Browse the repository at this point in the history
… with less responsability (Simperfit)

This PR was squashed before being merged into the 4.4 branch (closes #32198).

Discussion
----------

[Lock] Split "StoreInterface" into multiple interfaces with less responsability

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | Contribute to #28694 <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | TODO <!-- required for new features -->

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

We are removing the StoreInterface in order to split into multiple interface, it will help reduce de responsability of the StoreInterface.

Firstly, since not all stores needs to have all the methods of the StoreInterface, we can split this like this in order to limit the methods that are needed for each store.

Secondly, we add supportsX methods in order to avoid throwing exception when a store does not supports a feature it's easier an instance of the special interface or not, and it can return true/false on the support method.

**Really big thanks to** @jderusse for working with me on this, 1-2 hours of talking together, and another 1-2 hours of pre-review :). now giving it to the whole community !

*some time has been sponsored by* @izisolutions

Commits
-------

91fcbea [Lock] Split \"StoreInterface\" into multiple interfaces with less responsability
  • Loading branch information
fabpot committed Jul 8, 2019
2 parents 350ec6c + 91fcbea commit 608d428
Show file tree
Hide file tree
Showing 16 changed files with 291 additions and 78 deletions.
36 changes: 36 additions & 0 deletions src/Symfony/Component/Lock/BlockingStoreInterface.php
@@ -0,0 +1,36 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Lock;

use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Exception\NotSupportedException;

/**
* @author Hamza Amrouche <hamza.simperfit@gmail.com>
*/
interface BlockingStoreInterface
{
/**
* Waits until a key becomes free, then stores the resource.
*
* If the store does not support this feature it should throw a NotSupportedException.
*
* @throws LockConflictedException
* @throws NotSupportedException
*/
public function waitAndSave(Key $key);

/**
* Checks if the store can wait until a key becomes free before storing the resource.
*/
public function supportsWaitAndSave(): bool;
}
5 changes: 3 additions & 2 deletions src/Symfony/Component/Lock/CHANGELOG.md
Expand Up @@ -4,8 +4,9 @@ CHANGELOG
4.4.0
-----

* added InvalidTtlException

* added InvalidTtlException
* deprecated `Symfony\Component\Lock\StoreInterface` in favor of `Symfony\Component\Lock\BlockingStoreInterface` and `Symfony\Component\Lock\PersistStoreInterface`

4.2.0
-----

Expand Down
14 changes: 9 additions & 5 deletions src/Symfony/Component/Lock/Lock.php
Expand Up @@ -19,6 +19,7 @@
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Exception\LockExpiredException;
use Symfony\Component\Lock\Exception\LockReleasingException;
use Symfony\Component\Lock\Exception\NotSupportedException;

/**
* Lock is the default implementation of the LockInterface.
Expand All @@ -36,12 +37,12 @@ final class Lock implements LockInterface, LoggerAwareInterface
private $dirty = false;

/**
* @param Key $key Resource to lock
* @param StoreInterface $store Store used to handle lock persistence
* @param float|null $ttl Maximum expected lock duration in seconds
* @param bool $autoRelease Whether to automatically release the lock or not when the lock instance is destroyed
* @param Key $key Resource to lock
* @param PersistStoreInterface $store Store used to handle lock persistence
* @param float|null $ttl Maximum expected lock duration in seconds
* @param bool $autoRelease Whether to automatically release the lock or not when the lock instance is destroyed
*/
public function __construct(Key $key, StoreInterface $store, float $ttl = null, bool $autoRelease = true)
public function __construct(Key $key, PersistStoreInterface $store, float $ttl = null, bool $autoRelease = true)
{
$this->store = $store;
$this->key = $key;
Expand Down Expand Up @@ -70,6 +71,9 @@ public function acquire($blocking = false)
{
try {
if ($blocking) {
if (!($this->store instanceof StoreInterface) && !($this->store instanceof BlockingStoreInterface && $this->store->supportsWaitAndSave())) {
throw new NotSupportedException(sprintf('The store "%s" does not support blocking locks.', \get_class($this->store)));
}
$this->store->waitAndSave($this->key);
} else {
$this->store->save($this->key);
Expand Down
53 changes: 53 additions & 0 deletions src/Symfony/Component/Lock/PersistStoreInterface.php
@@ -0,0 +1,53 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Lock;

use Symfony\Component\Lock\Exception\LockAcquiringException;
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Exception\LockReleasingException;

/**
* @author Jérémy Derussé <jeremy@derusse.com>
*/
interface PersistStoreInterface
{
/**
* Stores the resource if it's not locked by someone else.
*
* @throws LockAcquiringException
* @throws LockConflictedException
*/
public function save(Key $key);

/**
* Removes a resource from the storage.
*
* @throws LockReleasingException
*/
public function delete(Key $key);

/**
* Returns whether or not the resource exists in the storage.
*
* @return bool
*/
public function exists(Key $key);

/**
* Extends the TTL of a resource.
*
* @param float $ttl amount of seconds to keep the lock in the store
*
* @throws LockConflictedException
*/
public function putOffExpiration(Key $key, $ttl);
}
25 changes: 19 additions & 6 deletions src/Symfony/Component/Lock/Store/CombinedStore.php
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Exception\NotSupportedException;
use Symfony\Component\Lock\Key;
use Symfony\Component\Lock\PersistStoreInterface;
use Symfony\Component\Lock\StoreInterface;
use Symfony\Component\Lock\Strategy\StrategyInterface;

Expand All @@ -26,27 +27,27 @@
*
* @author Jérémy Derussé <jeremy@derusse.com>
*/
class CombinedStore implements StoreInterface, LoggerAwareInterface
class CombinedStore implements StoreInterface, PersistStoreInterface, LoggerAwareInterface
{
use LoggerAwareTrait;
use ExpiringStoreTrait;

/** @var StoreInterface[] */
/** @var PersistStoreInterface[] */
private $stores;
/** @var StrategyInterface */
private $strategy;

/**
* @param StoreInterface[] $stores The list of synchronized stores
* @param StrategyInterface $strategy
* @param PersistStoreInterface[] $stores The list of synchronized stores
* @param StrategyInterface $strategy
*
* @throws InvalidArgumentException
*/
public function __construct(array $stores, StrategyInterface $strategy)
{
foreach ($stores as $store) {
if (!$store instanceof StoreInterface) {
throw new InvalidArgumentException(sprintf('The store must implement "%s". Got "%s".', StoreInterface::class, \get_class($store)));
if (!$store instanceof PersistStoreInterface) {
throw new InvalidArgumentException(sprintf('The store must implement "%s". Got "%s".', PersistStoreInterface::class, \get_class($store)));
}
}

Expand Down Expand Up @@ -92,8 +93,12 @@ public function save(Key $key)
throw new LockConflictedException();
}

/**
* {@inheritdoc}
*/
public function waitAndSave(Key $key)
{
@trigger_error(sprintf('%s::%s has been deprecated since Symfony 4.4 and will be removed in Symfony 5.0.', \get_class($this), __METHOD__), E_USER_DEPRECATED);
throw new NotSupportedException(sprintf('The store "%s" does not supports blocking locks.', \get_class($this)));
}

Expand Down Expand Up @@ -181,4 +186,12 @@ public function exists(Key $key)

return false;
}

/**
* {@inheritdoc}
*/
public function supportsWaitAndSave(): bool
{
return false;
}
}
12 changes: 11 additions & 1 deletion src/Symfony/Component/Lock/Store/FlockStore.php
Expand Up @@ -11,10 +11,12 @@

namespace Symfony\Component\Lock\Store;

use Symfony\Component\Lock\BlockingStoreInterface;
use Symfony\Component\Lock\Exception\InvalidArgumentException;
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Exception\LockStorageException;
use Symfony\Component\Lock\Key;
use Symfony\Component\Lock\PersistStoreInterface;
use Symfony\Component\Lock\StoreInterface;

/**
Expand All @@ -27,7 +29,7 @@
* @author Romain Neutron <imprec@gmail.com>
* @author Nicolas Grekas <p@tchwork.com>
*/
class FlockStore implements StoreInterface
class FlockStore implements StoreInterface, BlockingStoreInterface, PersistStoreInterface
{
private $lockPath;

Expand Down Expand Up @@ -64,6 +66,14 @@ public function waitAndSave(Key $key)
$this->lock($key, true);
}

/**
* {@inheritdoc}
*/
public function supportsWaitAndSave(): bool
{
return true;
}

private function lock(Key $key, $blocking)
{
// The lock is maybe already acquired.
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/Lock/Store/MemcachedStore.php
Expand Up @@ -15,14 +15,15 @@
use Symfony\Component\Lock\Exception\InvalidTtlException;
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Key;
use Symfony\Component\Lock\PersistStoreInterface;
use Symfony\Component\Lock\StoreInterface;

/**
* MemcachedStore is a StoreInterface implementation using Memcached as store engine.
*
* @author Jérémy Derussé <jeremy@derusse.com>
*/
class MemcachedStore implements StoreInterface
class MemcachedStore implements StoreInterface, PersistStoreInterface
{
use ExpiringStoreTrait;

Expand Down Expand Up @@ -69,8 +70,12 @@ public function save(Key $key)
$this->checkNotExpired($key);
}

/**
* {@inheritdoc}
*/
public function waitAndSave(Key $key)
{
@trigger_error(sprintf('%s has been deprecated since Symfony 4.4 and will be removed in Symfony 5.0.', __METHOD__));
throw new InvalidArgumentException(sprintf('The store "%s" does not supports blocking locks.', \get_class($this)));
}

Expand Down
4 changes: 3 additions & 1 deletion src/Symfony/Component/Lock/Store/PdoStore.php
Expand Up @@ -19,6 +19,7 @@
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Exception\NotSupportedException;
use Symfony\Component\Lock\Key;
use Symfony\Component\Lock\PersistStoreInterface;
use Symfony\Component\Lock\StoreInterface;

/**
Expand All @@ -34,7 +35,7 @@
*
* @author Jérémy Derussé <jeremy@derusse.com>
*/
class PdoStore implements StoreInterface
class PdoStore implements StoreInterface, PersistStoreInterface
{
use ExpiringStoreTrait;

Expand Down Expand Up @@ -145,6 +146,7 @@ public function save(Key $key)
*/
public function waitAndSave(Key $key)
{
@trigger_error(sprintf('%s has been deprecated since Symfony 4.4 and will be removed in Symfony 5.0.', __METHOD__));
throw new NotSupportedException(sprintf('The store "%s" does not supports blocking locks.', __METHOD__));
}

Expand Down
4 changes: 3 additions & 1 deletion src/Symfony/Component/Lock/Store/RedisStore.php
Expand Up @@ -17,14 +17,15 @@
use Symfony\Component\Lock\Exception\InvalidTtlException;
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Key;
use Symfony\Component\Lock\PersistStoreInterface;
use Symfony\Component\Lock\StoreInterface;

/**
* RedisStore is a StoreInterface implementation using Redis as store engine.
*
* @author Jérémy Derussé <jeremy@derusse.com>
*/
class RedisStore implements StoreInterface
class RedisStore implements StoreInterface, PersistStoreInterface
{
use ExpiringStoreTrait;

Expand Down Expand Up @@ -77,6 +78,7 @@ public function save(Key $key)
*/
public function waitAndSave(Key $key)
{
@trigger_error(sprintf('%s::%s has been deprecated since Symfony 4.4 and will be removed in Symfony 5.0.', \get_class($this), __METHOD__), E_USER_DEPRECATED);
throw new InvalidArgumentException(sprintf('The store "%s" does not supports blocking locks.', \get_class($this)));
}

Expand Down
20 changes: 15 additions & 5 deletions src/Symfony/Component/Lock/Store/RetryTillSaveStore.php
Expand Up @@ -14,8 +14,10 @@
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\NullLogger;
use Symfony\Component\Lock\BlockingStoreInterface;
use Symfony\Component\Lock\Exception\LockConflictedException;
use Symfony\Component\Lock\Key;
use Symfony\Component\Lock\PersistStoreInterface;
use Symfony\Component\Lock\StoreInterface;

/**
Expand All @@ -24,7 +26,7 @@
*
* @author Jérémy Derussé <jeremy@derusse.com>
*/
class RetryTillSaveStore implements StoreInterface, LoggerAwareInterface
class RetryTillSaveStore implements PersistStoreInterface, BlockingStoreInterface, StoreInterface, LoggerAwareInterface
{
use LoggerAwareTrait;

Expand All @@ -33,11 +35,11 @@ class RetryTillSaveStore implements StoreInterface, LoggerAwareInterface
private $retryCount;

/**
* @param StoreInterface $decorated The decorated StoreInterface
* @param int $retrySleep Duration in ms between 2 retry
* @param int $retryCount Maximum amount of retry
* @param PersistStoreInterface $decorated The decorated StoreInterface
* @param int $retrySleep Duration in ms between 2 retry
* @param int $retryCount Maximum amount of retry
*/
public function __construct(StoreInterface $decorated, int $retrySleep = 100, int $retryCount = PHP_INT_MAX)
public function __construct(PersistStoreInterface $decorated, int $retrySleep = 100, int $retryCount = PHP_INT_MAX)
{
$this->decorated = $decorated;
$this->retrySleep = $retrySleep;
Expand Down Expand Up @@ -99,4 +101,12 @@ public function exists(Key $key)
{
return $this->decorated->exists($key);
}

/**
* {@inheritdoc}
*/
public function supportsWaitAndSave(): bool
{
return true;
}
}

0 comments on commit 608d428

Please sign in to comment.