Cleanup lock related code

- Port to QueryBuilder
- Improve the doc a bit
- Add type hinting

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

diff --git a/apps/testing/lib/Locking/FakeDBLockingProvider.php b/apps/testing/lib/Locking/FakeDBLockingProvider.php
index 5f8ea3994779ac7d42274a81b675e7ef4ca54e20..2556ba29a642794ced6e38d7271a563ba94ceaa3 100644 (file)
--- 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 f4899bbb2b2c96b51066f9b8d7733c54bd568ebb..6e8289db12eea9eb4d6af298340d5a622545287b 100644 (file)
--- 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 5eb03ad856b8ac3d5cfd788700a5c84985776753..fb8af8ac55bd041e1d01bcf0864b79dda06d4d19 100644 (file)
--- a/lib/private/Lock/DBLockingProvider.php
+++ b/lib/private/Lock/DBLockingProvider.php
@@ -9,6 +9,7 @@
  * @author Ole Ostergaard <ole.c.ostergaard@gmail.com>
  * @author Robin Appelman <robin@icewind.nl>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
+ * @author Carl Schwan <carl@carlschwan.eu>
  *
  * @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 008a7875d7e1ade0eb46ce9d8f73a4c49ce8fe8b..d4eebd7c3022175e6be8ee722fdf564dc436f9e5 100644 (file)
--- 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 95ae8cf0cdaf82993b4cfdb8ebb6cc824f0c71c6..ff56932a8944321104fa6f4c189d5d51fa3947fe 100644 (file)
--- 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 e9d673d3746edaa005f2cf21dffa061864878c90..2010f49f108bda846f1d1edfc4bb664a9ccf5bed 100644 (file)
--- 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 3dc7c73b6ebb0e6dfefa7e7452fada2fe6117ff6..a2015d42f474dd30511ddebf09be11eee1848cee 100644 (file)
--- 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 d45c379ff0876e7b3a8bf576c060d2100a3a2a60..860e8d73cf0831604f7bcf9a7b3d1bfe5d8adf3d 100644 (file)
--- 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 973337edb4ee1a43558ab180994fc942166767cc..349a175b566e23632421c2b7a3acef01a89ac8cf 100644 (file)
--- 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() {