test(wopi): add coverage for locking operations

.github/workflows/integration.yml

@@ -357,7 +357,7 @@ jobs:
 
     if: always()
 
-    name: phpunit-oci-summary
+    name: phpunit-integration-summary
 
     steps:
       - name: Summary status

lib/Controller/WopiController.php

@@ -879,6 +879,21 @@ private function normalizePath(string $path, ?string $parent = null): string {
 		return $parent . $path;
 	}
 
+	/**
+	 * Acquire a server-side lock for the current WOPI file.
+	 *
+	 * This method uses the file lock manager to enforce file-level exclusivity.
+	 * The client-provided WOPI lock token is currently ignored.
+	 *
+	 * If the file is already manually locked by the same editor user, the request
+	 * is still allowed so WOPI write flows can continue.
+	 *
+	 * Returns:
+	 * - 200 on success
+	 * - 400 if the lock provider is unavailable or the lock precondition fails
+	 * - 423 if another user owns a conflicting lock
+	 * - 500 for unexpected failures
+	 */
 	private function lock(Wopi $wopi, string $lock): JSONResponse {
 		try {
 			$lock = $this->lockManager->lock(new LockContext(
@@ -890,19 +905,36 @@ private function lock(Wopi $wopi, string $lock): JSONResponse {
 		} catch (NoLockProviderException|PreConditionNotMetException) {
 			return new JSONResponse([], Http::STATUS_BAD_REQUEST);
 		} catch (OwnerLockedException $e) {
-			// If a file is manually locked by a user we want to all this user to still perform a WOPI lock and write
+			// The file is already manually locked by this editor, so allow the WOPI lock request to continue.
 			if ($e->getLock()->getType() === ILock::TYPE_USER && $e->getLock()->getOwner() === $wopi->getEditorUid()) {
 				return new JSONResponse();
 			}
 
+			// Another user owns the lock, so reject this WOPI lock request.
 			return new JSONResponse([], Http::STATUS_LOCKED);
 		} catch (\Exception) {
 			return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 	}
 
+	/**
+	 * Release the server-side lock for the current WOPI file.
+	 *
+	 * This method uses the file lock manager to release the application lock
+	 * associated with the target file. The client-provided WOPI lock token is
+	 * currently ignored.
+	 *
+	 * If the file is already manually locked by the same editor user, the request
+	 * is still allowed so WOPI write flows can continue.
+	 *
+	 * Returns:
+	 * - 200 on success
+	 * - 400 if the lock provider is unavailable or the lock precondition fails
+	 * - 500 for unexpected failures
+	 */
 	private function unlock(Wopi $wopi, string $lock): JSONResponse {
 		try {
+			// Try to release the application lock for the current file.
 			$this->lockManager->unlock(new LockContext(
 				$this->getFileForWopiToken($wopi),
 				ILock::TYPE_APP,
@@ -911,18 +943,37 @@ private function unlock(Wopi $wopi, string $lock): JSONResponse {
 			return new JSONResponse();
 		} catch (NoLockProviderException|PreConditionNotMetException) {
 			$locks = $this->lockManager->getLocks($wopi->getFileid());
+			// Allow the request to succeed if manually locked unless the conflictinglock belongs to another editor.
 			$canWriteThroughLock = count($locks) > 0 && $locks[0]->getType() === ILock::TYPE_USER && $locks[0]->getOwner() !== $wopi->getEditorUid() ? false : true;
 			if ($canWriteThroughLock) {
 				return new JSONResponse();
 			}
+
+			// A conflicting lock is present, so this unlock request cannot be satisfied.
 			return new JSONResponse([], Http::STATUS_BAD_REQUEST);
 		} catch (\Exception) {
 			return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 	}
 
+	/**
+	 * Refresh the server-side lock for the current WOPI file.
+	 *
+	 * This method attempts to acquire the file lock again through the lock manager
+	 * rather than updating a stored WOPI lock token. The client-provided WOPI lock token
+	 * is currently ignored.
+	 *
+	 * Note that Collabora just calls LOCK again to do a refresh; this is currently unused.
+	 *
+	 * Returns:
+	 * - 200 on success
+	 * - 400 if the lock provider is unavailable or the lock precondition fails
+	 * - 423 if another user owns a conflicting lock
+	 * - 500 for unexpected failures
+	 */
 	private function refreshLock(Wopi $wopi, string $lock): JSONResponse {
 		try {
+			// Refresh the application lock by acquiring it again through the server-side lock manager.
 			$lock = $this->lockManager->lock(new LockContext(
 				$this->getFileForWopiToken($wopi),
 				ILock::TYPE_APP,
@@ -932,14 +983,26 @@ private function refreshLock(Wopi $wopi, string $lock): JSONResponse {
 		} catch (NoLockProviderException|PreConditionNotMetException) {
 			return new JSONResponse([], Http::STATUS_BAD_REQUEST);
 		} catch (OwnerLockedException) {
+			// Another user owns the lock, so the refresh cannot proceed.
 			return new JSONResponse([], Http::STATUS_LOCKED);
 		} catch (\Exception) {
 			return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 	}
 
+	/**
+	 * Inspect the current lock state for the WOPI file.
+	 *
+	 * This method queries the file lock manager for the target file, but this implementation
+	 * is effectively a no-op currently: it does not return the lock information to the caller
+	 * nor do anything with the result. The client-provided WOPI lock token is also ignored.
+	 *
+	 * Note that Collabora currently never calls GET_LOCK; this is currently unused.
+	 */
 	private function getLock(Wopi $wopi, string $lock): JSONResponse {
+		// Query the current lock state from the server-side file lock manager.
 		$locks = $this->lockManager->getLocks($wopi->getFileid());
+		// The queried lock state is not returned to the caller.
 		return new JSONResponse();
 	}
 

tests/features/bootstrap/WopiContext.php

@@ -31,6 +31,8 @@ class WopiContext implements Context {
 	private $fileIds = [];
 	private $wopiToken;
 	private $checkFileInfoResult;
+	private $wopiLock;
+	private $lockResponseBody;
 
 	public function __construct() {
 		$this->downloadedFile = tempnam(sys_get_temp_dir(), 'downloadedFile');
@@ -354,4 +356,95 @@ public function performGuestCheckFileInfoRequests($count) {
 			$this->response = $last;
 		}
 	}
+
+	/**
+	 * @When /^Collabora locks the file with id "([^"]*)"$/
+	 */
+	public function collaboraLocksTheFileWithId(string $lockId): void {
+		$this->wopiLock = $lockId;
+		$this->collaboraPostOverride('LOCK', [
+			'X-WOPI-Lock' => $lockId,
+		]);
+	}
+
+	/**
+	 * @When /^Collabora refreshes the lock with id "([^"]*)"$/
+	 */
+	public function collaboraRefreshesTheLockWithId(string $lockId): void {
+		$this->wopiLock = $lockId;
+		$this->collaboraPostOverride('REFRESH_LOCK', [
+			'X-WOPI-Lock' => $lockId,
+		]);
+	}
+
+	/**
+	 * @When /^Collabora unlocks the file with id "([^"]*)"$/
+	 */
+	public function collaboraUnlocksTheFileWithId(string $lockId): void {
+		$this->collaboraPostOverride('UNLOCK', [
+			'X-WOPI-Lock' => $lockId,
+		]);
+	}
+
+	/**
+	 * @When /^Collabora gets the current lock$/
+	 */
+	public function collaboraGetsTheCurrentLock(): void {
+		$this->collaboraPostOverride('GET_LOCK');
+		$this->lockResponseBody = (string)$this->response->getBody();
+	}
+
+	/**
+	 * @Then /^the WOPI lock should be "([^"]*)"$/
+	 */
+	public function theWopiLockShouldBe(string $expected): void {
+		Assert::assertEquals($expected, trim($this->lockResponseBody ?? ''));
+	}
+
+	/**
+	 * @Then /^the WOPI lock response should be empty$/
+	 */
+	public function theWopiLockResponseShouldBeEmpty(): void {
+		Assert::assertSame('', trim($this->lockResponseBody ?? ''));
+	}
+
+	/**
+	 * @When /^Collabora unlocks the file with wrong id "([^"]*)"$/
+	 */
+	public function collaboraUnlocksTheFileWithWrongId(string $lockId): void {
+		$this->collaboraPostOverride('UNLOCK', [
+			'X-WOPI-Lock' => $lockId,
+		]);
+	}
+
+	/**
+	 * @Then /^the WOPI HTTP status code should be one of "([^"]*)"$/
+	 */
+	public function theWopiHttpStatusCodeShouldBeOneOf(string $codes): void {
+		$validCodes = array_map('trim', explode(',', $codes));
+		Assert::assertContains((string)$this->response->getStatusCode(), $validCodes);
+	}
+
+	private function collaboraPostOverride(string $override, array $headers = [], ?string $body = null): void {
+		$client = new Client();
+		$options = [
+			'headers' => array_merge([
+				'X-COOL-WOPI-Timestamp' => $this->checkFileInfoResult['LastModifiedTime'] ?? null,
+				'X-WOPI-Override' => $override,
+			], $headers),
+		];
+
+		if ($body !== null) {
+			$options['body'] = $body;
+		}
+
+		try {
+			$this->response = $client->post(
+				$this->getWopiEndpointBaseUrl() . 'index.php/apps/richdocuments/wopi/files/' . $this->fileId . '?access_token=' . $this->wopiToken,
+				$options
+			);
+		} catch (\GuzzleHttp\Exception\ClientException $e) {
+			$this->response = $e->getResponse();
+		}
+	}
 }

tests/features/wopi.feature

@@ -378,6 +378,49 @@ Feature: WOPI
     And as "user1" the file "/renamed_file.odt" does not exist
 
 
+  Scenario: Lock, refresh, get and unlock a file
+    Given as user "user1"
+    And User "user1" uploads file "./../emptyTemplates/template.odt" to "/locked.odt"
+    Then User "user1" opens "/locked.odt"
+    And Collabora fetches checkFileInfo
+
+    When Collabora locks the file with id "lock-123"
+    Then the WOPI HTTP status code should be "200"
+
+    # GET_LOCK not current implemented/supported
+    #When Collabora gets the current lock
+    #Then the WOPI lock should be "lock-123"
+
+    When Collabora refreshes the lock with id "lock-123"
+    Then the WOPI HTTP status code should be "200"
+
+    #When Collabora gets the current lock
+    #Then the WOPI lock should be "lock-123"
+
+    When Collabora unlocks the file with id "lock-123"
+    Then the WOPI HTTP status code should be "200"
+
+    #When Collabora gets the current lock
+    #Then the WOPI lock response should be empty
+
+  Scenario: Unlock with wrong lock token fails
+    Given as user "user1"
+    And User "user1" uploads file "./../emptyTemplates/template.odt" to "/wrong-unlock.odt"
+    Then User "user1" opens "/wrong-unlock.odt"
+    And Collabora fetches checkFileInfo
+
+    When Collabora locks the file with id "lock-123"
+    Then the WOPI HTTP status code should be "200"
+
+    When Collabora unlocks the file with wrong id "lock-456"
+    # UNLOCK currently ignores client provided lock identifier
+    #Then the WOPI HTTP status code should be "400"
+    Then the WOPI HTTP status code should be "200"
+
+    #When Collabora gets the current lock
+    #Then the WOPI lock should be "lock-123"
+
+
   Scenario: Public share cannot request a specific saved version
     Given as user "user1"
     And User "user1" uploads file "./../emptyTemplates/template.odt" to "/file.odt"
@@ -398,4 +441,4 @@ Feature: WOPI
     Then Using web as guest
     And a guest opens the share link
     When I perform "11" guest checkFileInfo requests
-    Then the WOPI HTTP status code should be "429"
+    Then the WOPI HTTP status code should be "429"

tests/run-integration.sh

@@ -47,6 +47,9 @@ PHPPIDA=$!
 PHP_CLI_SERVER_WORKERS=10 php -S localhost:$PORT_SERVERB -t $OC_PATH &
 PHPPIDB=$!
 
+if [ "$SCENARIO_TO_RUN" == "features/wopi.feature" ]; then
+	$OCC app:enable --force files_lock
+fi
 
 $OCC config:app:set richdocuments wopi_url --value="http://localhost:9980"
 $OCC config:app:set richdocuments public_wopi_url --value="http://localhost:9980"