Merge pull request #7897 from nextcloud/strict_lockingproviders

Made locking providers strict

apps/testing/lib/Locking/FakeDBLockingProvider.php

@@ -51,7 +51,7 @@ class FakeDBLockingProvider extends \OC\Lock\DBLockingProvider {
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	public function releaseLock($path, $type) {
+	public function releaseLock(string $path, int $type) {
 		// we DONT keep shared locks till the end of the request
 		if ($type === self::LOCK_SHARED) {
 			$this->db->executeUpdate(

lib/private/Lock/AbstractLockingProvider.php

 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -29,6 +30,7 @@ use OCP\Lock\ILockingProvider;
  * to release any left over 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
 
 	protected $acquiredLocks = [
@@ -43,7 +45,7 @@ abstract class AbstractLockingProvider implements ILockingProvider {
 	 * @param int $type
 	 * @return bool
 	 */
-	protected function hasAcquiredLock($path, $type) {
+	protected function hasAcquiredLock(string $path, int $type): bool {
 		if ($type === self::LOCK_SHARED) {
 			return isset($this->acquiredLocks['shared'][$path]) && $this->acquiredLocks['shared'][$path] > 0;
 		} else {
@@ -57,7 +59,7 @@ abstract class AbstractLockingProvider implements ILockingProvider {
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	protected function markAcquire($path, $type) {
+	protected function markAcquire(string $path, int $type) {
 		if ($type === self::LOCK_SHARED) {
 			if (!isset($this->acquiredLocks['shared'][$path])) {
 				$this->acquiredLocks['shared'][$path] = 0;
@@ -74,7 +76,7 @@ abstract class AbstractLockingProvider implements ILockingProvider {
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	protected function markRelease($path, $type) {
+	protected function markRelease(string $path, int $type) {
 		if ($type === self::LOCK_SHARED) {
 			if (isset($this->acquiredLocks['shared'][$path]) and $this->acquiredLocks['shared'][$path] > 0) {
 				$this->acquiredLocks['shared'][$path]--;
@@ -93,7 +95,7 @@ abstract class AbstractLockingProvider implements ILockingProvider {
 	 * @param string $path
 	 * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	protected function markChange($path, $targetType) {
+	protected function markChange(string $path, int $targetType) {
 		if ($targetType === self::LOCK_SHARED) {
 			unset($this->acquiredLocks['exclusive'][$path]);
 			if (!isset($this->acquiredLocks['shared'][$path])) {
@@ -121,7 +123,7 @@ abstract class AbstractLockingProvider implements ILockingProvider {
 		}
 	}
 
-	protected function getOwnSharedLockCount($path) {
+	protected function getOwnSharedLockCount(string $path) {
 		return isset($this->acquiredLocks['shared'][$path]) ? $this->acquiredLocks['shared'][$path] : 0;
 	}
 }

lib/private/Lock/DBLockingProvider.php

@@ -61,7 +61,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param string $path
 	 * @return bool
 	 */
-	protected function isLocallyLocked($path) {
+	protected function isLocallyLocked(string $path): bool {
 		return isset($this->sharedLocks[$path]) && $this->sharedLocks[$path];
 	}
 
@@ -71,7 +71,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	protected function markAcquire($path, $type) {
+	protected function markAcquire(string $path, int $type) {
 		parent::markAcquire($path, $type);
 		if ($type === self::LOCK_SHARED) {
 			$this->sharedLocks[$path] = true;
@@ -84,7 +84,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param string $path
 	 * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	protected function markChange($path, $targetType) {
+	protected function markChange(string $path, int $targetType) {
 		parent::markChange($path, $targetType);
 		if ($targetType === self::LOCK_SHARED) {
 			$this->sharedLocks[$path] = true;
@@ -99,7 +99,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param \OCP\AppFramework\Utility\ITimeFactory $timeFactory
 	 * @param int $ttl
 	 */
-	public function __construct(IDBConnection $connection, ILogger $logger, ITimeFactory $timeFactory, $ttl = 3600) {
+	public function __construct(IDBConnection $connection, ILogger $logger, ITimeFactory $timeFactory, int $ttl = 3600) {
 		$this->connection = $connection;
 		$this->logger = $logger;
 		$this->timeFactory = $timeFactory;
@@ -114,7 +114,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @return int number of inserted rows
 	 */
 
-	protected function initLockField($path, $lock = 0) {
+	protected function initLockField(string $path, int $lock = 0): int {
 		$expire = $this->getExpireTime();
 		return $this->connection->insertIfNotExist('*PREFIX*file_locks', ['key' => $path, 'lock' => $lock, 'ttl' => $expire], ['key']);
 	}
@@ -122,7 +122,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	/**
 	 * @return int
 	 */
-	protected function getExpireTime() {
+	protected function getExpireTime(): int {
 		return $this->timeFactory->getTime() + $this->ttl;
 	}
 
@@ -131,7 +131,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @return bool
 	 */
-	public function isLocked($path, $type) {
+	public function isLocked(string $path, int $type): bool {
 		if ($this->hasAcquiredLock($path, $type)) {
 			return true;
 		}
@@ -157,7 +157,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @throws \OCP\Lock\LockedException
 	 */
-	public function acquireLock($path, $type) {
+	public function acquireLock(string $path, int $type) {
 		$expire = $this->getExpireTime();
 		if ($type === self::LOCK_SHARED) {
 			if (!$this->isLocallyLocked($path)) {
@@ -194,7 +194,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	public function releaseLock($path, $type) {
+	public function releaseLock(string $path, int $type) {
 		$this->markRelease($path, $type);
 
 		// we keep shared locks till the end of the request so we can re-use them
@@ -213,7 +213,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @throws \OCP\Lock\LockedException
 	 */
-	public function changeLock($path, $targetType) {
+	public function changeLock(string $path, int $targetType) {
 		$expire = $this->getExpireTime();
 		if ($targetType === self::LOCK_SHARED) {
 			$result = $this->connection->executeUpdate(

lib/private/Lock/MemcacheLockingProvider.php

 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -36,12 +37,12 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 * @param \OCP\IMemcache $memcache
 	 * @param int $ttl
 	 */
-	public function __construct(IMemcache $memcache, $ttl = 3600) {
+	public function __construct(IMemcache $memcache, int $ttl = 3600) {
 		$this->memcache = $memcache;
 		$this->ttl = $ttl;
 	}
 
-	private function setTTL($path) {
+	private function setTTL(string $path) {
 		if ($this->memcache instanceof IMemcacheTTL) {
 			$this->memcache->setTTL($path, $this->ttl);
 		}
@@ -52,7 +53,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @return bool
 	 */
-	public function isLocked($path, $type) {
+	public function isLocked(string $path, int $type): bool {
 		$lockValue = $this->memcache->get($path);
 		if ($type === self::LOCK_SHARED) {
 			return $lockValue > 0;
@@ -68,7 +69,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @throws \OCP\Lock\LockedException
 	 */
-	public function acquireLock($path, $type) {
+	public function acquireLock(string $path, int $type) {
 		if ($type === self::LOCK_SHARED) {
 			if (!$this->memcache->inc($path)) {
 				throw new LockedException($path);
@@ -87,7 +88,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 */
-	public function releaseLock($path, $type) {
+	public function releaseLock(string $path, int $type) {
 		if ($type === self::LOCK_SHARED) {
 			if ($this->getOwnSharedLockCount($path) === 1) {
 				$removed = $this->memcache->cad($path, 1); // if we're the only one having a shared lock we can remove it in one go
@@ -111,7 +112,7 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @throws \OCP\Lock\LockedException
 	 */
-	public function changeLock($path, $targetType) {
+	public function changeLock(string $path, int $targetType) {
 		if ($targetType === self::LOCK_SHARED) {
 			if (!$this->memcache->cas($path, 'exclusive', 1)) {
 				throw new LockedException($path);

lib/private/Lock/NoopLockingProvider.php

 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -32,24 +33,24 @@ use OCP\Lock\ILockingProvider;
  */
 class NoopLockingProvider implements ILockingProvider {
 
-    /**
-     * {@inheritdoc}
-     */
-	public function isLocked($path, $type) {
+	/**
+	 * {@inheritdoc}
+	 */
+	public function isLocked(string $path, int $type): bool {
 		return false;
 	}
 
-    /**
-     * {@inheritdoc}
-     */
-	public function acquireLock($path, $type) {
+	/**
+	 * {@inheritdoc}
+	 */
+	public function acquireLock(string $path, int $type) {
 		// do nothing
 	}
 
 	/**
-     * {@inheritdoc}
+	 * {@inheritdoc}
 	 */
-	public function releaseLock($path, $type) {
+	public function releaseLock(string $path, int $type) {
 		// do nothing
 	}
 
@@ -63,7 +64,7 @@ class NoopLockingProvider implements ILockingProvider {
 	/**
 	 * {@inheritdoc}
 	 */
-	public function changeLock($path, $targetType) {
+	public function changeLock(string $path, int $targetType) {
 		// do nothing
 	}
 }

lib/public/Lock/ILockingProvider.php

 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -45,7 +46,7 @@ interface ILockingProvider {
 	 * @return bool
 	 * @since 8.1.0
 	 */
-	public function isLocked($path, $type);
+	public function isLocked(string $path, int $type): bool;
 
 	/**
 	 * @param string $path
@@ -53,14 +54,14 @@ interface ILockingProvider {
 	 * @throws \OCP\Lock\LockedException
 	 * @since 8.1.0
 	 */
-	public function acquireLock($path, $type);
+	public function acquireLock(string $path, int $type);
 
 	/**
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @since 8.1.0
 	 */
-	public function releaseLock($path, $type);
+	public function releaseLock(string $path, int $type);
 
 	/**
 	 * Change the type of an existing lock
@@ -70,7 +71,7 @@ interface ILockingProvider {
 	 * @throws \OCP\Lock\LockedException
 	 * @since 8.1.0
 	 */
-	public function changeLock($path, $targetType);
+	public function changeLock(string $path, int $targetType);
 
 	/**
 	 * release all lock acquired by this instance

lib/public/Lock/LockedException.php

 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -48,7 +49,7 @@ class LockedException extends \Exception {
 	 *
 	 * @since 8.1.0
 	 */
-	public function __construct($path, \Exception $previous = null) {
+	public function __construct(string $path, \Exception $previous = null) {
 		parent::__construct('"' . $path . '" is locked', 0, $previous);
 		$this->path = $path;
 	}
@@ -57,7 +58,7 @@ class LockedException extends \Exception {
 	 * @return string
 	 * @since 8.1.0
 	 */
-	public function getPath() {
+	public function getPath(): string {
 		return $this->path;
 	}
 }