Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/Symfony/Component/Lock/BlockingStoreInterface.php
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!($this->store instanceof StoreInterface) -> !$this->store instanceof StoreInterface

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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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}
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing @deprecated

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

broken notice, __METHOD__ gives FQCN::method. Parentheses are missing also, and we usually don't add the and will be removed ... part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean the paretheses in %s() ?

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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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}
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

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__));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing parentheses

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
Original file line number Diff line number Diff line change
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__));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

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
Original file line number Diff line number Diff line change
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

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
Original file line number Diff line number Diff line change
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;
}
}
Loading