Merge pull request #82 from nextcloud/bugfix/noid/skip-viewer-app-lock

Avoid checking viewer id if not relevant
nextcloud · Jul 6, 2022 · e5dd182 · e5dd182
2 parents 03b70b8 + 373130d
commit e5dd182
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 6 deletions.
diff --git a/lib/Storage/LockWrapper.php b/lib/Storage/LockWrapper.php
@@ -30,6 +30,7 @@
 use OCP\Files\InvalidPathException;
 use OCP\Files\Lock\ILock;
 use OCP\Files\Lock\ILockManager;
+use OCP\Files\Lock\NoLockProviderException;
 use OCP\Files\NotFoundException;
 use OCP\IUserSession;
 use OCP\Lock\LockedException;
@@ -123,21 +124,18 @@ protected function isLocked(string $ownerId, string $path, string $viewerId, &$l
 			}
 
 			$lock = $this->lockService->getLockFromFileId($file->getId());
-			// TODO: double check empty viewer id condition
-			if ($viewerId === '') {
-				return true;
-			}
 
-			$lockScope = $this->lockManager->getLockInScope();
 			if ($lock->getType() === ILock::TYPE_USER && $lock->getOwner() !== $viewerId) {
 				return true;
 			}
+
 			if ($lock->getType() === ILock::TYPE_APP) {
+				$lockScope = $this->lockManager->getLockInScope();
 				if (!$lockScope || $lockScope->getType() !== $lock->getType() || $lockScope->getOwner() !== $lock->getOwner()) {
 					return true;
 				}
 			}
-		} catch (LockNotFoundException | InvalidPathException | NotFoundException $e) {
+		} catch (NoLockProviderException | LockNotFoundException | InvalidPathException | NotFoundException $e) {
 		}
 
 		return false;

diff --git a/tests/Feature/LockFeatureTest.php b/tests/Feature/LockFeatureTest.php
@@ -291,6 +291,32 @@ public function testLockDifferentApps() {
 		});
 	}
 
+	public function testLockDifferentAppsPublic() {
+		self::logout();
+		$file = $this->rootFolder->getUserFolder(self::TEST_USER1)
+			->newFile('testfile_public', 'AAA');
+		$scope = new LockContext($file, ILock::TYPE_APP, 'collaborative_app');
+		$this->lockManager->lock($scope);
+
+		$this->lockManager->runInScope($scope, function () use ($file) {
+			self::assertEquals('collaborative_app', $this->lockManager->getLockInScope()->getOwner());
+			$file->putContent('EEE');
+			self::assertEquals('EEE', $file->getContent());
+		});
+
+		$otherAppScope = new LockContext($file, ILock::TYPE_APP, 'other_app');
+		$this->lockManager->runInScope($otherAppScope, function () use ($file) {
+			self::assertEquals('other_app', $this->lockManager->getLockInScope()->getOwner());
+			try {
+				$file->putContent('BBB');
+				$this->fail('Expected to throw a ManuallyLockedException');
+			} catch (ManuallyLockedException $e) {
+				self::assertInstanceOf(ManuallyLockedException::class, $e);
+				self::assertEquals('EEE', $file->getContent());
+			}
+		});
+	}
+
 	private function loginAndGetUserFolder(string $userId) {
 		$this->loginAsUser($userId);
 		return $this->rootFolder->getUserFolder($userId);
@@ -317,6 +343,10 @@ public function tearDown(): void {
 		parent::tearDown();
 		$folder = $this->rootFolder->getUserFolder(self::TEST_USER1);
 		$folder->delete('testfile');
+		$folder->delete('etag_test');
 		$folder->delete('testfile2');
+		$folder->delete('testfile3');
+		$folder->delete('testfile-infinite');
+		$folder->delete('testfile_public');
 	}
 }