From fcae6a68c347e2913cc29f45648be37789f09c29 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 22 Apr 2022 15:00:20 +0200 Subject: [PATCH] Cleanup lock related code - Port to QueryBuilder - Improve the doc a bit - Add type hinting Signed-off-by: Carl Schwan --- .../lib/Locking/FakeDBLockingProvider.php | 17 +- lib/private/Lock/AbstractLockingProvider.php | 53 ++--- lib/private/Lock/DBLockingProvider.php | 223 ++++++++---------- lib/private/Lock/MemcacheLockingProvider.php | 41 +--- lib/private/Lock/NoopLockingProvider.php | 8 +- lib/private/Server.php | 1 - lib/public/Lock/ILockingProvider.php | 36 ++- tests/lib/Lock/DBLockingProviderTest.php | 3 +- .../Lock/NonCachingDBLockingProviderTest.php | 3 +- 9 files changed, 142 insertions(+), 243 deletions(-) diff --git a/apps/testing/lib/Locking/FakeDBLockingProvider.php b/apps/testing/lib/Locking/FakeDBLockingProvider.php index 5f8ea399477..2556ba29a64 100644 --- a/apps/testing/lib/Locking/FakeDBLockingProvider.php +++ b/apps/testing/lib/Locking/FakeDBLockingProvider.php @@ -26,32 +26,27 @@ namespace OCA\Testing\Locking; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IDBConnection; use Psr\Log\LoggerInterface; +use OC\Lock\DBLockingProvider; -class FakeDBLockingProvider extends \OC\Lock\DBLockingProvider { +class FakeDBLockingProvider extends DBLockingProvider { // Lock for 10 hours just to be sure public const TTL = 36000; /** * Need a new child, because parent::connection is private instead of protected... - * @var IDBConnection */ - protected $db; + protected IDBConnection $db; public function __construct( IDBConnection $connection, - LoggerInterface $logger, ITimeFactory $timeFactory ) { - parent::__construct($connection, $logger, $timeFactory); + parent::__construct($connection, $timeFactory); $this->db = $connection; } - - /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - */ - public function releaseLock(string $path, int $type) { + /** @inheritDoc */ + public function releaseLock(string $path, int $type): void { // we DONT keep shared locks till the end of the request if ($type === self::LOCK_SHARED) { $this->db->executeUpdate( diff --git a/lib/private/Lock/AbstractLockingProvider.php b/lib/private/Lock/AbstractLockingProvider.php index f4899bbb2b2..6e8289db12e 100644 --- a/lib/private/Lock/AbstractLockingProvider.php +++ b/lib/private/Lock/AbstractLockingProvider.php @@ -30,24 +30,18 @@ use OCP\Lock\ILockingProvider; /** * Base locking provider that keeps track of locks acquired during the current request - * to release any left over locks at the end of the request + * to release any leftover locks at the end of the request */ abstract class AbstractLockingProvider implements ILockingProvider { - /** @var int $ttl */ - protected $ttl; // how long until we clear stray locks in seconds + /** how long until we clear stray locks in seconds */ + protected int $ttl; protected $acquiredLocks = [ 'shared' => [], 'exclusive' => [] ]; - /** - * Check if we've locally acquired a lock - * - * @param string $path - * @param int $type - * @return bool - */ + /** @inheritDoc */ protected function hasAcquiredLock(string $path, int $type): bool { if ($type === self::LOCK_SHARED) { return isset($this->acquiredLocks['shared'][$path]) && $this->acquiredLocks['shared'][$path] > 0; @@ -56,14 +50,9 @@ abstract class AbstractLockingProvider implements ILockingProvider { } } - /** - * Mark a locally acquired lock - * - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - */ - protected function markAcquire(string $path, int $type) { - if ($type === self::LOCK_SHARED) { + /** @inheritDoc */ + protected function markAcquire(string $path, int $targetType): void { + if ($targetType === self::LOCK_SHARED) { if (!isset($this->acquiredLocks['shared'][$path])) { $this->acquiredLocks['shared'][$path] = 0; } @@ -73,13 +62,8 @@ abstract class AbstractLockingProvider implements ILockingProvider { } } - /** - * Mark a release of a locally acquired lock - * - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - */ - protected function markRelease(string $path, int $type) { + /** @inheritDoc */ + protected function markRelease(string $path, int $type): void { if ($type === self::LOCK_SHARED) { if (isset($this->acquiredLocks['shared'][$path]) and $this->acquiredLocks['shared'][$path] > 0) { $this->acquiredLocks['shared'][$path]--; @@ -92,13 +76,8 @@ abstract class AbstractLockingProvider implements ILockingProvider { } } - /** - * Change the type of an existing tracked lock - * - * @param string $path - * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE - */ - protected function markChange(string $path, int $targetType) { + /** @inheritDoc */ + protected function markChange(string $path, int $targetType): void { if ($targetType === self::LOCK_SHARED) { unset($this->acquiredLocks['exclusive'][$path]); if (!isset($this->acquiredLocks['shared'][$path])) { @@ -111,10 +90,8 @@ abstract class AbstractLockingProvider implements ILockingProvider { } } - /** - * release all lock acquired by this instance which were marked using the mark* methods - */ - public function releaseAll() { + /** @inheritDoc */ + public function releaseAll(): void { foreach ($this->acquiredLocks['shared'] as $path => $count) { for ($i = 0; $i < $count; $i++) { $this->releaseLock($path, self::LOCK_SHARED); @@ -126,7 +103,7 @@ abstract class AbstractLockingProvider implements ILockingProvider { } } - protected function getOwnSharedLockCount(string $path) { - return isset($this->acquiredLocks['shared'][$path]) ? $this->acquiredLocks['shared'][$path] : 0; + protected function getOwnSharedLockCount(string $path): int { + return $this->acquiredLocks['shared'][$path] ?? 0; } } diff --git a/lib/private/Lock/DBLockingProvider.php b/lib/private/Lock/DBLockingProvider.php index 5eb03ad856b..fb8af8ac55b 100644 --- a/lib/private/Lock/DBLockingProvider.php +++ b/lib/private/Lock/DBLockingProvider.php @@ -9,6 +9,7 @@ * @author Ole Ostergaard * @author Robin Appelman * @author Roeland Jago Douma + * @author Carl Schwan * * @license AGPL-3.0 * @@ -33,51 +34,40 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; -use Psr\Log\LoggerInterface; /** * Locking provider that stores the locks in the database */ class DBLockingProvider extends AbstractLockingProvider { - /** - * @var \OCP\IDBConnection - */ - private $connection; - - private LoggerInterface $logger; - - /** - * @var \OCP\AppFramework\Utility\ITimeFactory - */ - private $timeFactory; - - private $sharedLocks = []; + private IDBConnection $connection; + private ITimeFactory $timeFactory; + private array $sharedLocks = []; + private bool $cacheSharedLocks; - /** - * @var bool - */ - private $cacheSharedLocks; + public function __construct( + IDBConnection $connection, + ITimeFactory $timeFactory, + int $ttl = 3600, + bool $cacheSharedLocks = true + ) { + $this->connection = $connection; + $this->timeFactory = $timeFactory; + $this->ttl = $ttl; + $this->cacheSharedLocks = $cacheSharedLocks; + } /** * Check if we have an open shared lock for a path - * - * @param string $path - * @return bool */ protected function isLocallyLocked(string $path): bool { return isset($this->sharedLocks[$path]) && $this->sharedLocks[$path]; } - /** - * Mark a locally acquired lock - * - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - */ - protected function markAcquire(string $path, int $type) { - parent::markAcquire($path, $type); + /** @inheritDoc */ + protected function markAcquire(string $path, int $targetType): void { + parent::markAcquire($path, $targetType); if ($this->cacheSharedLocks) { - if ($type === self::LOCK_SHARED) { + if ($targetType === self::LOCK_SHARED) { $this->sharedLocks[$path] = true; } } @@ -85,11 +75,8 @@ class DBLockingProvider extends AbstractLockingProvider { /** * Change the type of an existing tracked lock - * - * @param string $path - * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE */ - protected function markChange(string $path, int $targetType) { + protected function markChange(string $path, int $targetType): void { parent::markChange($path, $targetType); if ($this->cacheSharedLocks) { if ($targetType === self::LOCK_SHARED) { @@ -100,29 +87,8 @@ class DBLockingProvider extends AbstractLockingProvider { } } - /** - * @param bool $cacheSharedLocks - */ - public function __construct( - IDBConnection $connection, - LoggerInterface $logger, - ITimeFactory $timeFactory, - int $ttl = 3600, - $cacheSharedLocks = true - ) { - $this->connection = $connection; - $this->logger = $logger; - $this->timeFactory = $timeFactory; - $this->ttl = $ttl; - $this->cacheSharedLocks = $cacheSharedLocks; - } - /** * Insert a file locking row if it does not exists. - * - * @param string $path - * @param int $lock - * @return int number of inserted rows */ protected function initLockField(string $path, int $lock = 0): int { $expire = $this->getExpireTime(); @@ -133,25 +99,21 @@ class DBLockingProvider extends AbstractLockingProvider { ]); } - /** - * @return int - */ protected function getExpireTime(): int { return $this->timeFactory->getTime() + $this->ttl; } - /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @return bool - */ + /** @inheritDoc */ public function isLocked(string $path, int $type): bool { if ($this->hasAcquiredLock($path, $type)) { return true; } - $query = $this->connection->prepare('SELECT `lock` from `*PREFIX*file_locks` WHERE `key` = ?'); - $query->execute([$path]); - $lockValue = (int)$query->fetchOne(); + $query = $this->connection->getQueryBuilder(); + $query->select('lock') + ->from('file_locks') + ->where($query->expr()->eq('key', $query->createNamedParameter($path))); + $result = $query->executeQuery(); + $lockValue = (int)$result->fetchOne(); if ($type === self::LOCK_SHARED) { if ($this->isLocallyLocked($path)) { // if we have a shared lock we kept open locally but it's released we always have at least 1 shared lock in the db @@ -166,21 +128,20 @@ class DBLockingProvider extends AbstractLockingProvider { } } - /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @throws \OCP\Lock\LockedException - */ - public function acquireLock(string $path, int $type, string $readablePath = null) { + /** @inheritDoc */ + public function acquireLock(string $path, int $type, ?string $readablePath = null): void { $expire = $this->getExpireTime(); if ($type === self::LOCK_SHARED) { if (!$this->isLocallyLocked($path)) { $result = $this->initLockField($path, 1); if ($result <= 0) { - $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*file_locks` SET `lock` = `lock` + 1, `ttl` = ? WHERE `key` = ? AND `lock` >= 0', - [$expire, $path] - ); + $query = $this->connection->getQueryBuilder(); + $query->update('file_locks') + ->set('lock', $query->func()->add('lock', $query->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->set('ttl', $query->createNamedParameter($expire)) + ->where($query->expr()->eq('key', $query->createNamedParameter($path))) + ->andWhere($query->expr()->gte('lock', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + $result = $query->executeStatement(); } } else { $result = 1; @@ -192,10 +153,13 @@ class DBLockingProvider extends AbstractLockingProvider { } $result = $this->initLockField($path, -1); if ($result <= 0) { - $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*file_locks` SET `lock` = -1, `ttl` = ? WHERE `key` = ? AND `lock` = ?', - [$expire, $path, $existing] - ); + $query = $this->connection->getQueryBuilder(); + $query->update('file_locks') + ->set('lock', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT)) + ->set('ttl', $query->createNamedParameter($expire, IQueryBuilder::PARAM_INT)) + ->where($query->expr()->eq('key', $query->createNamedParameter($path))) + ->andWhere($query->expr()->eq('lock', $query->createNamedParameter($existing))); + $result = $query->executeStatement(); } } if ($result !== 1) { @@ -204,52 +168,53 @@ class DBLockingProvider extends AbstractLockingProvider { $this->markAcquire($path, $type); } - /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - */ - public function releaseLock(string $path, int $type) { + /** @inheritDoc */ + public function releaseLock(string $path, int $type): void { $this->markRelease($path, $type); // we keep shared locks till the end of the request so we can re-use them if ($type === self::LOCK_EXCLUSIVE) { - $this->connection->executeUpdate( - 'UPDATE `*PREFIX*file_locks` SET `lock` = 0 WHERE `key` = ? AND `lock` = -1', - [$path] - ); + $qb = $this->connection->getQueryBuilder(); + $qb->update('file_locks') + ->set('lock', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('key', $qb->createNamedParameter($path))) + ->andWhere($qb->expr()->eq('lock', $qb->createNamedParameter(-1, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); } elseif (!$this->cacheSharedLocks) { - $query = $this->connection->getQueryBuilder(); - $query->update('file_locks') - ->set('lock', $query->func()->subtract('lock', $query->createNamedParameter(1))) - ->where($query->expr()->eq('key', $query->createNamedParameter($path))) - ->andWhere($query->expr()->gt('lock', $query->createNamedParameter(0))); - $query->execute(); + $qb = $this->connection->getQueryBuilder(); + $qb->update('file_locks') + ->set('lock', $qb->func()->subtract('lock', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->where($qb->expr()->eq('key', $qb->createNamedParameter($path))) + ->andWhere($qb->expr()->gt('lock', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); } } - /** - * Change the type of an existing lock - * - * @param string $path - * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @throws \OCP\Lock\LockedException - */ - public function changeLock(string $path, int $targetType) { + /** @inheritDoc */ + public function changeLock(string $path, int $targetType): void { $expire = $this->getExpireTime(); if ($targetType === self::LOCK_SHARED) { - $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*file_locks` SET `lock` = 1, `ttl` = ? WHERE `key` = ? AND `lock` = -1', - [$expire, $path] - ); + $qb = $this->connection->getQueryBuilder(); + $result = $qb->update('file_locks') + ->set('lock', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT)) + ->set('ttl', $qb->createNamedParameter($expire, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->andX( + $qb->expr()->eq('key', $qb->createNamedParameter($path)), + $qb->expr()->eq('lock', $qb->createNamedParameter(-1, IQueryBuilder::PARAM_INT)) + ))->executeStatement(); } else { // since we only keep one shared lock in the db we need to check if we have more then one shared lock locally manually if (isset($this->acquiredLocks['shared'][$path]) && $this->acquiredLocks['shared'][$path] > 1) { throw new LockedException($path); } - $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*file_locks` SET `lock` = -1, `ttl` = ? WHERE `key` = ? AND `lock` = 1', - [$expire, $path] - ); + $qb = $this->connection->getQueryBuilder(); + $result = $qb->update('file_locks') + ->set('lock', $qb->createNamedParameter(-1, IQueryBuilder::PARAM_INT)) + ->set('ttl', $qb->createNamedParameter($expire, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->andX( + $qb->expr()->eq('key', $qb->createNamedParameter($path)), + $qb->expr()->eq('lock', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT)) + ))->executeStatement(); } if ($result !== 1) { throw new LockedException($path); @@ -257,16 +222,14 @@ class DBLockingProvider extends AbstractLockingProvider { $this->markChange($path, $targetType); } - /** - * cleanup empty locks - */ - public function cleanExpiredLocks() { + /** @inheritDoc */ + public function cleanExpiredLocks(): void { $expire = $this->timeFactory->getTime(); try { - $this->connection->executeUpdate( - 'DELETE FROM `*PREFIX*file_locks` WHERE `ttl` < ?', - [$expire] - ); + $qb = $this->connection->getQueryBuilder(); + $qb->delete('file_locks') + ->where($qb->expr()->lt('ttl', $qb->createNamedParameter($expire, IQueryBuilder::PARAM_INT))) + ->executeStatement(); } catch (\Exception $e) { // If the table is missing, the clean up was successful if ($this->connection->tableExists('file_locks')) { @@ -275,10 +238,8 @@ class DBLockingProvider extends AbstractLockingProvider { } } - /** - * release all lock acquired by this instance which were marked using the mark* methods - */ - public function releaseAll() { + /** @inheritDoc */ + public function releaseAll(): void { parent::releaseAll(); if (!$this->cacheSharedLocks) { @@ -292,15 +253,15 @@ class DBLockingProvider extends AbstractLockingProvider { $chunkedPaths = array_chunk($lockedPaths, 100); - foreach ($chunkedPaths as $chunk) { - $builder = $this->connection->getQueryBuilder(); - - $query = $builder->update('file_locks') - ->set('lock', $builder->func()->subtract('lock', $builder->expr()->literal(1))) - ->where($builder->expr()->in('key', $builder->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY))) - ->andWhere($builder->expr()->gt('lock', new Literal(0))); + $qb = $this->connection->getQueryBuilder(); + $qb->update('file_locks') + ->set('lock', $qb->func()->subtract('lock', $qb->expr()->literal(1))) + ->where($qb->expr()->in('key', $qb->createParameter('chunk'))) + ->andWhere($qb->expr()->gt('lock', new Literal(0))); - $query->execute(); + foreach ($chunkedPaths as $chunk) { + $qb->setParameter('chunk', $chunk, IQueryBuilder::PARAM_STR_ARRAY); + $qb->executeStatement(); } } } diff --git a/lib/private/Lock/MemcacheLockingProvider.php b/lib/private/Lock/MemcacheLockingProvider.php index 008a7875d7e..d4eebd7c302 100644 --- a/lib/private/Lock/MemcacheLockingProvider.php +++ b/lib/private/Lock/MemcacheLockingProvider.php @@ -32,31 +32,19 @@ use OCP\IMemcacheTTL; use OCP\Lock\LockedException; class MemcacheLockingProvider extends AbstractLockingProvider { - /** - * @var \OCP\IMemcache - */ - private $memcache; + private IMemcache $memcache; - /** - * @param \OCP\IMemcache $memcache - * @param int $ttl - */ public function __construct(IMemcache $memcache, int $ttl = 3600) { $this->memcache = $memcache; $this->ttl = $ttl; } - private function setTTL(string $path) { + private function setTTL(string $path): void { if ($this->memcache instanceof IMemcacheTTL) { $this->memcache->setTTL($path, $this->ttl); } } - /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @return bool - */ public function isLocked(string $path, int $type): bool { $lockValue = $this->memcache->get($path); if ($type === self::LOCK_SHARED) { @@ -68,13 +56,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider { } } - /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @param string $readablePath human readable path to use in error messages - * @throws \OCP\Lock\LockedException - */ - public function acquireLock(string $path, int $type, string $readablePath = null) { + public function acquireLock(string $path, int $type, ?string $readablePath = null): void { if ($type === self::LOCK_SHARED) { if (!$this->memcache->inc($path)) { throw new LockedException($path, null, $this->getExistingLockForException($path), $readablePath); @@ -89,11 +71,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider { $this->markAcquire($path, $type); } - /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - */ - public function releaseLock(string $path, int $type) { + public function releaseLock(string $path, int $type): void { if ($type === self::LOCK_SHARED) { $ownSharedLockCount = $this->getOwnSharedLockCount($path); $newValue = 0; @@ -120,14 +98,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider { $this->markRelease($path, $type); } - /** - * Change the type of an existing lock - * - * @param string $path - * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @throws \OCP\Lock\LockedException - */ - public function changeLock(string $path, int $targetType) { + public function changeLock(string $path, int $targetType): void { if ($targetType === self::LOCK_SHARED) { if (!$this->memcache->cas($path, 'exclusive', 1)) { throw new LockedException($path, null, $this->getExistingLockForException($path)); @@ -142,7 +113,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider { $this->markChange($path, $targetType); } - private function getExistingLockForException($path) { + private function getExistingLockForException(string $path): string { $existing = $this->memcache->get($path); if (!$existing) { return 'none'; diff --git a/lib/private/Lock/NoopLockingProvider.php b/lib/private/Lock/NoopLockingProvider.php index 95ae8cf0cda..ff56932a894 100644 --- a/lib/private/Lock/NoopLockingProvider.php +++ b/lib/private/Lock/NoopLockingProvider.php @@ -45,28 +45,28 @@ class NoopLockingProvider implements ILockingProvider { /** * {@inheritdoc} */ - public function acquireLock(string $path, int $type, string $readablePath = null) { + public function acquireLock(string $path, int $type, ?string $readablePath = null): void { // do nothing } /** * {@inheritdoc} */ - public function releaseLock(string $path, int $type) { + public function releaseLock(string $path, int $type): void { // do nothing } /**1 * {@inheritdoc} */ - public function releaseAll() { + public function releaseAll(): void { // do nothing } /** * {@inheritdoc} */ - public function changeLock(string $path, int $targetType) { + public function changeLock(string $path, int $targetType): void { // do nothing } } diff --git a/lib/private/Server.php b/lib/private/Server.php index e9d673d3746..2010f49f108 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1109,7 +1109,6 @@ class Server extends ServerContainer implements IServerContainer { } return new DBLockingProvider( $c->get(IDBConnection::class), - $c->get(LoggerInterface::class), new TimeFactory(), $ttl, !\OC::$CLI diff --git a/lib/public/Lock/ILockingProvider.php b/lib/public/Lock/ILockingProvider.php index 3dc7c73b6eb..a2015d42f47 100644 --- a/lib/public/Lock/ILockingProvider.php +++ b/lib/public/Lock/ILockingProvider.php @@ -28,7 +28,10 @@ declare(strict_types=1); namespace OCP\Lock; /** - * Interface ILockingProvider + * This interface allows locking and unlocking filesystem paths + * + * This interface should be used directly and not implemented by an application. + * The implementation is provided by the server. * * @since 8.1.0 */ @@ -43,42 +46,37 @@ interface ILockingProvider { public const LOCK_EXCLUSIVE = 2; /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @return bool + * @psalm-param self::LOCK_SHARED|self::LOCK_EXCLUSIVE $type * @since 8.1.0 */ public function isLocked(string $path, int $type): bool; /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @param string $readablePath human readable path to use in error messages, since 20.0.0 - * @throws \OCP\Lock\LockedException + * @psalm-param self::LOCK_SHARED|self::LOCK_EXCLUSIVE $type + * @param ?string $readablePath A human-readable path to use in error messages, since 20.0.0 + * @throws LockedException * @since 8.1.0 */ - public function acquireLock(string $path, int $type, string $readablePath = null); + public function acquireLock(string $path, int $type, ?string $readablePath = null): void; /** - * @param string $path - * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + * @psalm-param self::LOCK_SHARED|self::LOCK_EXCLUSIVE $type * @since 8.1.0 */ - public function releaseLock(string $path, int $type); + public function releaseLock(string $path, int $type): void; /** - * Change the type of an existing lock + * Change the target type of an existing lock * - * @param string $path - * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE - * @throws \OCP\Lock\LockedException + * @psalm-param self::LOCK_SHARED|self::LOCK_EXCLUSIVE $targetType + * @throws LockedException * @since 8.1.0 */ - public function changeLock(string $path, int $targetType); + public function changeLock(string $path, int $targetType): void; /** - * release all lock acquired by this instance + * Release all lock acquired by this instance * @since 8.1.0 */ - public function releaseAll(); + public function releaseAll(): void; } diff --git a/tests/lib/Lock/DBLockingProviderTest.php b/tests/lib/Lock/DBLockingProviderTest.php index d45c379ff08..860e8d73cf0 100644 --- a/tests/lib/Lock/DBLockingProviderTest.php +++ b/tests/lib/Lock/DBLockingProviderTest.php @@ -23,7 +23,6 @@ namespace Test\Lock; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Lock\ILockingProvider; -use Psr\Log\LoggerInterface; /** * Class DBLockingProvider @@ -66,7 +65,7 @@ class DBLockingProviderTest extends LockingProvider { */ protected function getInstance() { $this->connection = \OC::$server->getDatabaseConnection(); - return new \OC\Lock\DBLockingProvider($this->connection, \OC::$server->get(LoggerInterface::class), $this->timeFactory, 3600); + return new \OC\Lock\DBLockingProvider($this->connection, $this->timeFactory, 3600); } protected function tearDown(): void { diff --git a/tests/lib/Lock/NonCachingDBLockingProviderTest.php b/tests/lib/Lock/NonCachingDBLockingProviderTest.php index 973337edb4e..349a175b566 100644 --- a/tests/lib/Lock/NonCachingDBLockingProviderTest.php +++ b/tests/lib/Lock/NonCachingDBLockingProviderTest.php @@ -22,7 +22,6 @@ namespace Test\Lock; use OCP\Lock\ILockingProvider; -use Psr\Log\LoggerInterface; /** * @group DB @@ -35,7 +34,7 @@ class NonCachingDBLockingProviderTest extends DBLockingProviderTest { */ protected function getInstance() { $this->connection = \OC::$server->getDatabaseConnection(); - return new \OC\Lock\DBLockingProvider($this->connection, \OC::$server->get(LoggerInterface::class), $this->timeFactory, 3600, false); + return new \OC\Lock\DBLockingProvider($this->connection, $this->timeFactory, 3600, false); } public function testDoubleShared() { -- 2.39.5