Unify lock service methods to use the LockScope and propagate ETag on…

… lock change

Signed-off-by: Julius Härtl <[email protected]>

lib/Command/Lock.php

@@ -42,6 +42,8 @@
 use OCA\FilesLock\Service\FileService;
 use OCA\FilesLock\Service\LockService;
 use OCP\Files\InvalidPathException;
+use OCP\Files\Lock\ILock;
+use OCP\Files\Lock\LockScope;
 use OCP\Files\NotFoundException;
 use OCP\IUserManager;
 use Symfony\Component\Console\Exception\InvalidArgumentException;
@@ -201,7 +203,9 @@ private function lockFile(InputInterface $input, OutputInterface $output, int $f
 		$file = $this->fileService->getFileFromId($user->getUID(), $fileId);
 
 		$output->writeln('<info>locking ' . $file->getName() . ' to ' . $userId . '</info>');
-		$this->lockService->lockFileAsUser($file, $user);
+		$this->lockService->lock(new LockScope(
+			$file, ILock::TYPE_USER, $userId
+		));
 	}
 
 
@@ -220,7 +224,7 @@ private function unlockFile(InputInterface $input, OutputInterface $output, int
 
 		$output->writeln('<info>unlocking File #' . $fileId);
 		try {
-			$this->lockService->unlockFile($fileId, '', true);
+			$this->lockService->unlockFile($fileId, $input->getArgument('user_id'), true);
 		} catch (LockNotFoundException $e) {
 		}
 

lib/Controller/LockController.php

@@ -38,6 +38,8 @@
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
+use OCP\Files\Lock\ILock;
+use OCP\Files\Lock\LockScope;
 use OCP\IRequest;
 use OCP\IUserSession;
 
@@ -94,7 +96,9 @@ public function locking(string $fileId): DataResponse {
 			$user = $this->userSession->getUser();
 			$file = $this->fileService->getFileFromId($user->getUID(), (int)$fileId);
 
-			$lock = $this->lockService->lockFileAsUser($file, $user);
+			$lock = $this->lockService->lock(new LockScope(
+				$file, ILock::TYPE_USER, $user->getUID()
+			));
 
 			return new DataResponse($lock, Http::STATUS_OK);
 		} catch (Exception $e) {

lib/Cron/Unlock.php

@@ -30,52 +30,24 @@
 namespace OCA\FilesLock\Cron;
 
 
-use OC\BackgroundJob\TimedJob;
-use OCA\FilesLock\AppInfo\Application;
+use OCP\BackgroundJob\TimedJob;
 use OCA\FilesLock\Service\LockService;
-use OCP\AppFramework\QueryException;
 
-
-/**
- * Class Unlock
- *
- * @package OCA\FilesLock\Cron
- */
 class Unlock extends TimedJob {
 
+	private LockService $lockService;
 
-	/** @var LockService */
-	private $lockService;
-
+	public function __construct(LockService $lockService) {
+		$this->lockService = $lockService;
 
-	/**
-	 * Unlock constructor.
-	 */
-	public function __construct() {
-//		$this->setInterval(12 * 60); // 12 minutes
 		$this->setInterval(1);
 	}
 
-
-	/**
-	 * @param mixed $argument
-	 *
-	 * @throws QueryException
-	 */
-	protected function run($argument) {
-		$app = new Application();
-		$c = $app->getContainer();
-
-		$this->lockService = $c->query(LockService::class);
-
+	protected function run($argument): void {
 		$this->manageTimeoutLock();
 	}
 
-
-	/**
-	 *
-	 */
-	private function manageTimeoutLock() {
+	private function manageTimeoutLock(): void {
 		$this->lockService->removeLocks($this->lockService->getDeprecatedLocks());
 	}
 

lib/DAV/LockPlugin.php

@@ -13,6 +13,7 @@
 use OCA\FilesLock\Service\LockService;
 use OCP\Files\InvalidPathException;
 use OCP\Files\Lock\ILock;
+use OCP\Files\Lock\LockScope;
 use OCP\Files\Lock\OwnerLockedException;
 use OCP\Files\NotFoundException;
 use OCP\IUserManager;
@@ -163,7 +164,9 @@ public function httpLock(RequestInterface $request, ResponseInterface $response)
 
 			try {
 				$file = $this->fileService->getFileFromAbsoluteUri($this->server->getRequestUri());
-				$lockInfo = $this->lockService->lockFileAsUser($file, $this->userSession->getUser());
+				$lockInfo = $this->lockService->lock(new LockScope(
+					$file, ILock::TYPE_USER, $this->userSession->getUser()->getUID()
+				));
 				$response->setStatus(200);
 				$response->setBody(
 					$this->server->xml->write(
@@ -193,7 +196,9 @@ public function httpUnlock(RequestInterface $request, ResponseInterface $respons
 
 			try {
 				$file = $this->fileService->getFileFromAbsoluteUri($this->server->getRequestUri());
-				$this->lockService->unlockFile($file->getId(), $this->userSession->getUser()->getUID());
+				$this->lockService->unlock(new LockScope(
+					$file, ILock::TYPE_USER, $this->userSession->getUser()->getUID()
+				));
 				$response->setStatus(200);
 				$response->setBody(
 					$this->server->xml->write(

lib/LockProvider.php

@@ -6,6 +6,7 @@
 use OCP\Files\Lock\ILock;
 use OCP\Files\Lock\ILockProvider;
 use OCP\Files\Lock\LockScope;
+use OCP\Files\Lock\OwnerLockedException;
 use OCP\PreConditionNotMetException;
 
 class LockProvider implements ILockProvider {
@@ -24,17 +25,16 @@ public function getLocks(int $fileId): array {
 		return [];
 	}
 
+	/**
+	 * @inheritdoc
+	 */
 	public function lock(LockScope $lockInfo): ILock {
-		if ($lockInfo->getType() === ILock::TYPE_USER) {
-			return $this->lockService->lockFileByUserId($lockInfo->getNode(), $lockInfo->getOwner());
-		}
-		if ($lockInfo->getType() === ILock::TYPE_APP) {
-			return $this->lockService->lockFileAsApp($lockInfo->getNode(), $lockInfo->getOwner());
-		}
-
-		throw new PreConditionNotMetException('Invalid type');
+		return $this->lockService->lock($lockInfo);
 	}
 
+	/**
+	 * @inheritdoc
+	 */
 	public function unlock(LockScope $lockInfo) {
 		try {
 			$this->lockService->getLockFromFileId($lockInfo->getNode()->getId());

lib/Model/FileLock.php

@@ -35,6 +35,7 @@
 use JsonSerializable;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\Lock\ILock;
+use OCP\Files\Lock\LockScope;
 use Sabre\DAV\Locks\LockInfo;
 
 /**
@@ -84,6 +85,14 @@ public function __construct(int $timeout = 1800) {
 		$this->creation = \OC::$server->get(ITimeFactory::class)->getTime();
 	}
 
+	public static function fromLockScope(LockScope $lockScope, int $timeout): FileLock {
+		$lock = new FileLock($timeout);
+		$lock->setUserId($lockScope->getOwner());
+		$lock->setLockType($lockScope->getType());
+		$lock->setFileId($lockScope->getNode()->getId());
+		return $lock;
+	}
+
 
 	/**
 	 * @return int

lib/Service/LockService.php

@@ -33,7 +33,6 @@
 use Exception;
 use OCA\FilesLock\Db\LocksRequest;
 use OCA\FilesLock\Exceptions\LockNotFoundException;
-use OCA\FilesLock\Exceptions\NotFileException;
 use OCA\FilesLock\Exceptions\UnauthorizedUnlockException;
 use OCA\FilesLock\Model\FileLock;
 use OCA\FilesLock\Tools\Traits\TLogger;
@@ -42,17 +41,13 @@
 use OCP\DirectEditing\IManager;
 use OCP\DirectEditing\RegisterDirectEditorEvent;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\Files\FileInfo;
 use OCP\Files\InvalidPathException;
 use OCP\Files\Lock\ILock;
 use OCP\Files\Lock\LockScope;
 use OCP\Files\Lock\OwnerLockedException;
-use OCP\Files\Node;
 use OCP\Files\NotFoundException;
 use OCP\IL10N;
-use OCP\IUser;
 use OCP\IUserManager;
-use OCP\PreConditionNotMetException;
 
 
 /**
@@ -130,90 +125,33 @@ public function getLockForNodeId(int $nodeId) {
 		return $this->lockCache[$nodeId];
 	}
 
-
-	/**
-	 * @throws OwnerLockedException
-	 */
-	public function lock(FileLock $lock): FileLock {
+	public function lock(LockScope $lockScope): FileLock {
 		try {
-			$known = $this->getLockFromFileId($lock->getFileId());
-			if ($known->getLockType() === $lock->getLockType() && $known->getOwner() === $lock->getOwner()) {
+			$known = $this->getLockFromFileId($lockScope->getNode()->getId());
+
+			// Extend lock expiry if matching
+			if (
+				$known->getLockType() === $lockScope->getType() &&
+				$known->getOwner() === $lockScope->getOwner()
+			) {
 				$known->setTimeout(
 					$known->getTimeout() - $known->getETA() + $this->configService->getTimeoutSeconds()
 				);
 				$this->notice('extending existing lock', false, ['fileLock' => $known]);
 				$this->locksRequest->update($known);
 				return $known;
 			}
+
 			throw new OwnerLockedException($known);
 		} catch (LockNotFoundException $e) {
+			$lock = FileLock::fromLockScope($lockScope, $this->configService->getTimeoutSeconds());
 			$this->generateToken($lock);
 			$lock->setCreation(time());
 			$this->notice('locking file', false, ['fileLock' => $lock]);
 			$this->locksRequest->save($lock);
+			$this->propagateEtag($lockScope);
+			return $lock;
 		}
-		return $lock;
-	}
-
-
-	/**
-	 * @param Node $file
-	 * @param IUser $user
-	 *
-	 * @return FileLock
-	 * @throws InvalidPathException
-	 * @throws NotFileException
-	 * @throws NotFoundException
-	 * @throws OwnerLockedException
-	 */
-	public function lockFileAsUser(Node $file, IUser $user): FileLock {
-		if ($file->getType() !== FileInfo::TYPE_FILE) {
-			throw new NotFileException('Must be a file, seems to be a folder.');
-		}
-
-		$lock = new FileLock($this->configService->getTimeoutSeconds());
-		$lock->setUserId($user->getUID());
-		$lock->setFileId($file->getId());
-
-		return $this->lock($lock);
-	}
-
-	/**
-	 * @throws InvalidPathException
-	 * @throws NotFileException
-	 * @throws NotFoundException
-	 * @throws PreConditionNotMetException
-	 * @throws OwnerLockedException
-	 */
-	public function lockFileByUserId(Node $file, string $userId): FileLock {
-		$user = $this->userManager->get($userId);
-		if ($user) {
-			return $this->lockFileAsUser($file, $user);
-		}
-
-		throw new PreConditionNotMetException('No user found' . $userId);
-	}
-
-
-	/**
-	 * @throws InvalidPathException
-	 * @throws NotFileException
-	 * @throws NotFoundException
-	 * @throws OwnerLockedException
-	 */
-	public function lockFileAsApp(Node $file, string $appId): FileLock {
-		if ($file->getType() !== FileInfo::TYPE_FILE) {
-			throw new NotFileException('Must be a file, seems to be a folder.');
-		}
-
-		$lock = new FileLock($this->configService->getTimeoutSeconds());
-		$lock->setLockType(ILock::TYPE_APP);
-		$lock->setUserId($appId);
-		$lock->setFileId($file->getId());
-
-		$this->lock($lock);
-
-		return $lock;
 	}
 
 	public function getAppName(string $appId): ?string {
@@ -250,7 +188,7 @@ public function unlock(LockScope $lock, bool $force = false): FileLock {
 		}
 
 		$this->locksRequest->delete($known);
-
+		$this->propagateEtag($lock);
 		return $known;
 	}
 
@@ -262,12 +200,24 @@ public function unlock(LockScope $lock, bool $force = false): FileLock {
 	 * @throws UnauthorizedUnlockException
 	 */
 	public function unlockFile(int $fileId, string $userId, bool $force = false): FileLock {
+		$lock = $this->getLockForNodeId($fileId);
+		if (!$lock) {
+			throw new LockNotFoundException();
+		}
+
+		$type = ILock::TYPE_USER;
+		if ($force) {
+			$userId = $lock->getOwner();
+			$type = $lock->getLockType();
+		}
+
 		$node = $this->fileService->getFileFromId($userId, $fileId);
 		$lock = new LockScope(
 			$node,
-			ILock::TYPE_USER,
+			$type,
 			$userId,
 		);
+		$this->propagateEtag($lock);
 		return $this->unlock($lock, $force);
 	}
 
@@ -281,7 +231,7 @@ public function getDeprecatedLocks(): array {
 			$this->notice(
 				'ConfigService::LOCK_TIMEOUT is not numerical, using default', true, ['current' => $timeout]
 			);
-			$timeout = $this->configService->defaults[ConfigService::LOCK_TIMEOUT];
+			$timeout = (int)$this->configService->defaults[ConfigService::LOCK_TIMEOUT];
 		}
 
 		try {
@@ -341,5 +291,11 @@ function (FileLock $lock) {
 
 		$this->locksRequest->removeIds($ids);
 	}
+
+	private function propagateEtag(LockScope $lockScope): void {
+		$node = $lockScope->getNode();
+		$node->getStorage()->getUpdater()->propagate($node->getPath(), $node->getMTime());
+	}
+
 }