diff --git a/lib/Controller/LockController.php b/lib/Controller/LockController.php
index 9a4b1b86..c07d159e 100644
--- a/lib/Controller/LockController.php
+++ b/lib/Controller/LockController.php
@@ -103,11 +103,14 @@ public function unlocking(string $fileId, int $lockType = ILock::TYPE_USER): Dat
$response->setStatus(Http::STATUS_PRECONDITION_FAILED);
return $response;
} catch (UnauthorizedUnlockException) {
- $lock = $this->lockService->getLockFromFileId((int)$fileId);
- $response = new DataResponse();
- $response->setStatus(Http::STATUS_LOCKED);
- $response->setData($lock->jsonSerialize());
- return $response;
+ try {
+ $lock = $this->lockService->getLockFromFileId((int)$fileId);
+ } catch (LockNotFoundException) {
+ $response = new DataResponse();
+ $response->setStatus(Http::STATUS_PRECONDITION_FAILED);
+ return $response;
+ }
+ return new DataResponse($lock, Http::STATUS_LOCKED);
} catch (Exception $e) {
return $this->fail($e);
}
@@ -120,21 +123,17 @@ public function setOCSVersion($version): void {
private function buildOCSResponse(string $format, DataResponse $data): V1Response|V2Response {
$message = null;
- if ($data->getStatus() === Http::STATUS_LOCKED) {
- $lock = new FileLock();
- $lock->import($data->getData());
- $this->lockService->injectMetadata($lock);
- $message = $this->l10n->t('File is currently locked by %s', [$lock->getDisplayName()]);
+ $containedData = $data->getData();
+ if ($data->getStatus() === Http::STATUS_LOCKED && $containedData instanceof FileLock) {
+ $this->lockService->injectMetadata($containedData);
+ $message = $this->l10n->t('File is currently locked by %s', [$containedData->getDisplayName() ?? $containedData->getOwner()]);
}
if ($data->getStatus() === Http::STATUS_PRECONDITION_FAILED) {
- /** @var FileLock $lock */
- $lock = $data->getData();
$message = $this->l10n->t('File is not locked');
}
- $containedData = $data->getData();
if ($containedData instanceof FileLock) {
- $data->setData($data->getData()->jsonSerialize());
+ $data->setData($containedData->jsonSerialize());
}
if ($this->ocsVersion === 1) {
diff --git a/lib/Model/FileLock.php b/lib/Model/FileLock.php
index 6a180215..98612f97 100644
--- a/lib/Model/FileLock.php
+++ b/lib/Model/FileLock.php
@@ -203,16 +203,19 @@ public function importFromDatabase(array $data): self {
return $this;
}
+ /**
+ * Import the shape produced by jsonSerialize() (also accepts database column names).
+ */
public function import(array $data): void {
- $this->setId((int)$data['id']);
- $this->setUri($data['uri'] ?? '');
- $this->setUserId($data['user_id']);
- $this->setFileId((int)$data['file_id']);
- $this->setToken($data['token'] ?? '');
- $this->setCreation((int)$data['creation']);
- $this->setLockType((int)$data['type']);
- $this->setTimeout((int)$data['ttl']);
- $this->setDisplayName($data['owner'] ?? '');
+ $this->setId((int)($data['id'] ?? 0));
+ $this->setUri((string)($data['uri'] ?? ''));
+ $this->setUserId((string)($data['userId'] ?? $data['user_id'] ?? ''));
+ $this->setFileId((int)($data['fileId'] ?? $data['file_id'] ?? 0));
+ $this->setToken((string)($data['token'] ?? ''));
+ $this->setCreation((int)($data['creation'] ?? 0));
+ $this->setLockType((int)($data['type'] ?? ILock::TYPE_USER));
+ $this->setTimeout((int)($data['ttl'] ?? 0));
+ $this->setDisplayName((string)($data['displayName'] ?? $data['owner'] ?? ''));
}
#[\Override]
diff --git a/lib/Service/LockService.php b/lib/Service/LockService.php
index 80ac5393..46706c12 100644
--- a/lib/Service/LockService.php
+++ b/lib/Service/LockService.php
@@ -56,6 +56,11 @@ public function __construct(
) {
}
+ public function clearCache(): void {
+ $this->lockCache = [];
+ $this->remoteLockCache = [];
+ }
+
public function getLockForNodeId(int $nodeId, ?Node $node = null): FileLock|false {
if (array_key_exists($nodeId, $this->lockCache) && $this->lockCache[$nodeId] !== false) {
return $this->lockCache[$nodeId];
@@ -415,7 +420,7 @@ public function getRemoteLockFromDav(int $nodeId, ?Node $node = null): ?FileLock
$fileLock = new FileLock();
$fileLock->import([
'fileId' => $nodeId,
- 'owner' => (string)($storage->getPropfindPropertyValue($path, Application::DAV_PROPERTY_LOCK_OWNER_DISPLAYNAME) ?? ''),
+ 'displayName' => (string)($storage->getPropfindPropertyValue($path, Application::DAV_PROPERTY_LOCK_OWNER_DISPLAYNAME) ?? ''),
'type' => (int)($storage->getPropfindPropertyValue($path, Application::DAV_PROPERTY_LOCK_OWNER_TYPE) ?? 0),
'creation' => (int)($storage->getPropfindPropertyValue($path, Application::DAV_PROPERTY_LOCK_TIME) ?? 0),
'ttl' => (int)($storage->getPropfindPropertyValue($path, Application::DAV_PROPERTY_LOCK_TIMEOUT) ?? 0),
diff --git a/tests/Feature/CommandTest.php b/tests/Feature/CommandTest.php
new file mode 100644
index 00000000..696e030c
--- /dev/null
+++ b/tests/Feature/CommandTest.php
@@ -0,0 +1,53 @@
+loginAndGetUserFolder(self::USER1)->newFile('cli.txt', 'AAA');
+ $id = $file->getId();
+ $tester = $this->tester();
+
+ $tester->execute(['file_id' => (string)$id, '--status' => true]);
+ self::assertStringContainsString('not locked', $tester->getDisplay());
+
+ self::assertSame(0, $tester->execute(['file_id' => (string)$id, 'user_id' => self::USER1]));
+ self::assertSame(1, $this->lockRowCount($id));
+
+ $tester->execute(['file_id' => (string)$id, '--status' => true]);
+ self::assertStringContainsString('locked by ' . self::USER1, $tester->getDisplay());
+ }
+
+ public function testUnlock(): void {
+ $file = $this->loginAndGetUserFolder(self::USER1)->newFile('cli-unlock.txt', 'AAA');
+ $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1));
+
+ self::assertSame(0, $this->tester()->execute(['file_id' => (string)$file->getId(), '--unlock' => true]));
+ self::assertSame(0, $this->lockRowCount($file->getId()));
+ }
+}
diff --git a/tests/Feature/ControllableTimeFactory.php b/tests/Feature/ControllableTimeFactory.php
new file mode 100644
index 00000000..8a40ecb7
--- /dev/null
+++ b/tests/Feature/ControllableTimeFactory.php
@@ -0,0 +1,39 @@
+time ?? time();
+ }
+
+ #[\Override]
+ public function getDateTime(string $time = 'now', ?\DateTimeZone $timezone = null): \DateTime {
+ if ($time === 'now' && $this->time !== null) {
+ return (new \DateTime('@' . $this->time))->setTimezone($timezone ?? new \DateTimeZone('UTC'));
+ }
+ return parent::getDateTime($time, $timezone);
+ }
+
+ #[\Override]
+ public function now(): \DateTimeImmutable {
+ return new \DateTimeImmutable('@' . $this->getTime());
+ }
+}
diff --git a/tests/Feature/LockFeatureTest.php b/tests/Feature/LockFeatureTest.php
index 9fced4c4..0fb0954f 100644
--- a/tests/Feature/LockFeatureTest.php
+++ b/tests/Feature/LockFeatureTest.php
@@ -456,6 +456,44 @@ public function testUnlockStaleClientLock(): void {
$this->assertCount(0, $locks);
}
+ /**
+ * The display name of a federated lock comes from the remote as free text.
+ * It is not a local user id and must not be treated as one.
+ */
+ public function testRemoteLockKeepsTheRemoteDisplayName(): void {
+ $this->loginAsUser(self::TEST_USER1);
+
+ $storage = $this->createMock(\OCA\Files_Sharing\External\Storage::class);
+ $storage->method('instanceOfStorage')->willReturnCallback(
+ static fn (string $class): bool => $class === \OC\Files\Storage\DAV::class
+ );
+ $storage->method('getPropfindPropertyValue')->willReturnCallback(
+ static fn (string $path, string $property): mixed => match ($property) {
+ Application::DAV_PROPERTY_LOCK => '1',
+ Application::DAV_PROPERTY_LOCK_OWNER_DISPLAYNAME => 'Alice Remote',
+ Application::DAV_PROPERTY_LOCK_OWNER_TYPE => (string)ILock::TYPE_USER,
+ default => null,
+ }
+ );
+ $storage->method('getRemote')->willReturn('https://cloud.example.org/remote.php/dav');
+
+ $node = $this->createMock(\OCP\Files\Node::class);
+ $node->method('getStorage')->willReturn($storage);
+ $node->method('getInternalPath')->willReturn('files/remote-locked.txt');
+
+ $service = \OCP\Server::get(LockService::class);
+ $lock = $service->getRemoteLockFromDav(424242, $node);
+
+ $this->assertNotNull($lock);
+ $this->assertSame('Alice Remote@cloud.example.org', $lock->getDisplayName());
+ $this->assertSame('', $lock->getOwner(), 'a remote display name is not a local user id');
+ $this->assertSame(
+ 'Alice Remote@cloud.example.org',
+ $service->injectMetadata($lock)->getDisplayName(),
+ 'the propfind path runs every lock through injectMetadata'
+ );
+ }
+
private function loginAndGetUserFolder(string $userId) {
$this->loginAsUser($userId);
return $this->rootFolder->getUserFolder($userId);
diff --git a/tests/Feature/LockTestCase.php b/tests/Feature/LockTestCase.php
new file mode 100644
index 00000000..e12ab681
--- /dev/null
+++ b/tests/Feature/LockTestCase.php
@@ -0,0 +1,160 @@
+createUser($user, $user);
+ }
+ \OCP\Server::get(IUserManager::class)->registerBackend($backend);
+ }
+
+ protected function setUp(): void {
+ parent::setUp();
+ $this->time = null;
+ $this->lockManager = \OCP\Server::get(ILockManager::class);
+ $this->rootFolder = \OCP\Server::get(IRootFolder::class);
+ $this->timeFactory = new ControllableTimeFactory();
+ $this->overwriteService(ITimeFactory::class, $this->timeFactory);
+ $this->clearLocks();
+ $this->setLockTimeoutMinutes(-1);
+ \OC_Hook::$thrownExceptions = [];
+ }
+
+ protected function tearDown(): void {
+ $this->clearLocks();
+ foreach ([self::USER1, self::USER2, self::USER3] as $user) {
+ try {
+ $this->loginAsUser($user);
+ foreach ($this->rootFolder->getUserFolder($user)->getDirectoryListing() as $node) {
+ try {
+ $node->delete();
+ } catch (\Throwable) {
+ }
+ }
+ if (class_exists(\OCA\Files_Trashbin\Trashbin::class)) {
+ \OCA\Files_Trashbin\Trashbin::deleteAll();
+ }
+ } catch (\Throwable) {
+ }
+ }
+ parent::tearDown();
+ }
+
+ protected function lockService(): LockService {
+ return \OCP\Server::get(LockService::class);
+ }
+
+ protected function clearLocks(): void {
+ \OCP\Server::get(IDBConnection::class)->executeStatement('DELETE FROM `*PREFIX*files_lock`');
+ $this->lockService()->clearCache();
+ }
+
+ protected function setLockTimeoutMinutes(int $minutes): void {
+ \OCP\Server::get(IConfig::class)->setAppValue(Application::APP_ID, ConfigLexicon::LOCK_TIMEOUT, (string)$minutes);
+ }
+
+ protected function loginAndGetUserFolder(string $userId): Folder {
+ $this->loginAsUser($userId);
+ $this->lockService()->clearCache();
+ return $this->rootFolder->getUserFolder($userId);
+ }
+
+ protected function shareWith(\OCP\Files\Node $node, string $owner, string $user, int $permissions = 19): IShare {
+ $shareManager = \OCP\Server::get(IShareManager::class);
+ $share = $shareManager->newShare();
+ $share->setNode($node)
+ ->setSharedBy($owner)
+ ->setSharedWith($user)
+ ->setShareType(IShare::TYPE_USER)
+ ->setPermissions($permissions);
+ $share = $shareManager->createShare($share);
+ $share->setStatus(IShare::STATUS_ACCEPTED);
+ $shareManager->updateShare($share);
+ return $share;
+ }
+
+ /**
+ * Move the clock to an absolute moment, which lets a test model two processes
+ * whose clock reads happen in a different order than their database writes.
+ */
+ protected function atTime(int $timestamp): void {
+ $this->time = $timestamp;
+ $this->timeFactory->time = $timestamp;
+ $this->lockService()->clearCache();
+ }
+
+ protected function toTheFuture(int $seconds): void {
+ if ($this->time === null) {
+ $this->time = time();
+ }
+ $this->time += $seconds;
+ $this->timeFactory->time = $this->time;
+ $this->lockService()->clearCache();
+ }
+
+ protected function lockRowCount(int $fileId): int {
+ return (int)\OCP\Server::get(IDBConnection::class)->executeQuery(
+ 'SELECT COUNT(*) FROM `*PREFIX*files_lock` WHERE `file_id` = ?', [$fileId]
+ )->fetchOne();
+ }
+
+ protected function storedLock(int $fileId): ?FileLock {
+ $locks = \OCP\Server::get(LocksRequest::class)->getFromFileIds([$fileId]);
+ return $locks[0] ?? null;
+ }
+
+ /**
+ * Create a file as USER1 and share it with USER2 (and optionally USER3).
+ */
+ protected function sharedFile(string $name, int $permissions = 19, ?int $permissionsUser3 = null): File {
+ $file = $this->loginAndGetUserFolder(self::USER1)->newFile($name, 'AAA');
+ $this->shareWith($file, self::USER1, self::USER2, $permissions);
+ if ($permissionsUser3 !== null) {
+ $this->shareWith($file, self::USER1, self::USER3, $permissionsUser3);
+ }
+ return $file;
+ }
+}
diff --git a/tests/Feature/OcsControllerTest.php b/tests/Feature/OcsControllerTest.php
new file mode 100644
index 00000000..b112ecfe
--- /dev/null
+++ b/tests/Feature/OcsControllerTest.php
@@ -0,0 +1,84 @@
+setOCSVersion(2);
+ return $controller;
+ }
+
+ /**
+ * @return array{int, array|string} rendered status and decoded body (array for json, string for xml)
+ */
+ private function render(DataResponse $response, string $format = 'json'): array {
+ $rendered = $this->controller()->buildResponse($response, $format);
+ self::assertInstanceOf(BaseResponse::class, $rendered);
+ $body = $rendered->render();
+ if ($format === 'json') {
+ return [$rendered->getStatus(), json_decode($body, true, 512, JSON_THROW_ON_ERROR)];
+ }
+ return [$rendered->getStatus(), $body];
+ }
+
+ public function testLockUnlockRoundTrip(): void {
+ $file = $this->loginAndGetUserFolder(self::USER1)->newFile('ocs.txt', 'AAA');
+ [$status, $body] = $this->render($this->controller()->locking((string)$file->getId()));
+ self::assertSame(Http::STATUS_OK, $status);
+ self::assertSame(self::USER1, $body['ocs']['data']['userId']);
+ self::assertSame(ILock::TYPE_USER, $body['ocs']['data']['type']);
+ self::assertStringStartsWith('files_lock/', $body['ocs']['data']['token']);
+
+ [$status] = $this->render($this->controller()->unlocking((string)$file->getId()));
+ self::assertSame(Http::STATUS_OK, $status);
+ self::assertSame(0, $this->lockRowCount($file->getId()));
+
+ [$status, $body] = $this->render($this->controller()->unlocking((string)$file->getId()));
+ self::assertSame(Http::STATUS_PRECONDITION_FAILED, $status);
+ self::assertSame('File is not locked', $body['ocs']['meta']['message']);
+ }
+
+ public function testConflictIsAStructured423(): void {
+ $file = $this->sharedFile('conflict.txt');
+ $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1));
+ $this->loginAndGetUserFolder(self::USER2);
+
+ foreach (['json', 'xml'] as $format) {
+ [$status, $body] = $this->render($this->controller()->locking((string)$file->getId()), $format);
+ self::assertSame(Http::STATUS_LOCKED, $status, $format);
+ if ($format === 'json') {
+ self::assertSame(self::USER1, $body['ocs']['data']['userId']);
+ self::assertSame($file->getId(), $body['ocs']['data']['fileId']);
+ self::assertStringContainsString('locked by', $body['ocs']['meta']['message']);
+ } else {
+ self::assertStringContainsString('' . self::USER1 . '', $body);
+ self::assertStringContainsString('423', $body);
+ }
+ }
+
+ [$status, $body] = $this->render($this->controller()->unlocking((string)$file->getId()));
+ self::assertSame(Http::STATUS_LOCKED, $status, 'a recipient may not release the owner lock');
+ self::assertSame(self::USER1, $body['ocs']['data']['userId']);
+ self::assertSame(1, $this->lockRowCount($file->getId()));
+ }
+}
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
index f1414fbe..062a5fd0 100644
--- a/tests/bootstrap.php
+++ b/tests/bootstrap.php
@@ -17,4 +17,6 @@
require_once __DIR__ . '/../../../lib/base.php';
require_once __DIR__ . '/../../../tests/autoload.php';
+\OC::$composerAutoloader->addPsr4('OCA\\FilesLock\\Tests\\', __DIR__ . '/', true);
+
Server::get(IAppManager::class)->loadApp('files_lock');