Merge pull request #29565 from nextcloud/backport/25332/stable22

[stable22] Fix removing remote shares when the remote server is unreachable
nextcloud · Mar 10, 2022 · 06600f6 · 06600f6
2 parents 78b370b + 8206b38
commit 06600f6
Show file tree

Hide file tree

Showing 5 changed files with 309 additions and 10 deletions.
diff --git a/build/integration/features/bootstrap/FederationContext.php b/build/integration/features/bootstrap/FederationContext.php
@@ -28,6 +28,7 @@
  */
 use Behat\Behat\Context\Context;
 use Behat\Behat\Context\SnippetAcceptingContext;
+use Behat\Gherkin\Node\TableNode;
 
 require __DIR__ . '/../../vendor/autoload.php';
 
@@ -39,6 +40,29 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	use AppConfiguration;
 	use CommandLine;
 
+	/** @var string */
+	private static $phpFederatedServerPid = '';
+
+	/** @var string */
+	private $lastAcceptedRemoteShareId;
+
+	/**
+	 * @BeforeScenario
+	 * @AfterScenario
+	 *
+	 * The server is started also after the scenarios to ensure that it is
+	 * properly cleaned up if stopped.
+	 */
+	public function startFederatedServer() {
+		if (self::$phpFederatedServerPid !== '') {
+			return;
+		}
+
+		$port = getenv('PORT_FED');
+
+		self::$phpFederatedServerPid = exec('php -S localhost:' . $port . ' -t ../../ >/dev/null & echo $!');
+	}
+
 	/**
 	 * @BeforeScenario
 	 */
@@ -93,6 +117,37 @@ public function federateGroupSharing($sharerUser, $sharerServer, $sharerPath, $s
 		$this->usingServer($previous);
 	}
 
+	/**
+	 * @Then remote share :count is returned with
+	 *
+	 * @param int $number
+	 * @param TableNode $body
+	 */
+	public function remoteShareXIsReturnedWith(int $number, TableNode $body) {
+		$this->theHTTPStatusCodeShouldBe('200');
+		$this->theOCSStatusCodeShouldBe('100');
+
+		if (!($body instanceof TableNode)) {
+			return;
+		}
+
+		$returnedShare = $this->getXmlResponse()->data[0];
+		if ($returnedShare->element) {
+			$returnedShare = $returnedShare->element[$number];
+		}
+
+		$defaultExpectedFields = [
+			'id' => 'A_NUMBER',
+			'remote_id' => 'A_NUMBER',
+			'accepted' => '1',
+		];
+		$expectedFields = array_merge($defaultExpectedFields, $body->getRowsHash());
+
+		foreach ($expectedFields as $field => $value) {
+			$this->assertFieldIsInReturnedShare($field, $value, $returnedShare);
+		}
+	}
+
 	/**
 	 * @When /^User "([^"]*)" from server "(LOCAL|REMOTE)" accepts last pending share$/
 	 * @param string $user
@@ -109,6 +164,30 @@ public function acceptLastPendingShare($user, $server) {
 		$this->theHTTPStatusCodeShouldBe('200');
 		$this->theOCSStatusCodeShouldBe('100');
 		$this->usingServer($previous);
+
+		$this->lastAcceptedRemoteShareId = $share_id;
+	}
+
+	/**
+	 * @When /^user "([^"]*)" deletes last accepted remote share$/
+	 * @param string $user
+	 */
+	public function deleteLastAcceptedRemoteShare($user) {
+		$this->asAn($user);
+		$this->sendingToWith('DELETE', "/apps/files_sharing/api/v1/remote_shares/" . $this->lastAcceptedRemoteShareId, null);
+	}
+
+	/**
+	 * @When /^remote server is stopped$/
+	 */
+	public function remoteServerIsStopped() {
+		if (self::$phpFederatedServerPid === '') {
+			return;
+		}
+
+		exec('kill ' . self::$phpFederatedServerPid);
+
+		self::$phpFederatedServerPid = '';
 	}
 
 	protected function resetAppConfigs() {

diff --git a/build/integration/federation_features/federated.feature b/build/integration/federation_features/federated.feature
@@ -278,13 +278,230 @@ Feature: federated
 
 
 
+	Scenario: List federated share from another server not accepted yet
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And Using server "LOCAL"
+		When As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		Then the list of returned shares has 0 shares
 
+	Scenario: List federated share from another server
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		When As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		Then the list of returned shares has 1 shares
+		And remote share 0 is returned with
+			| remote      | http://localhost:8180/ |
+			| name        | /remote-share.txt |
+			| owner       | user1 |
+			| user        | user0 |
+			| mountpoint  | /remote-share.txt |
+			| mimetype    | text/plain |
+			| mtime        | A_NUMBER |
+			| permissions | 27 |
+			| type        | file |
+			| file_id     | A_NUMBER |
 
+	Scenario: List federated share from another server no longer reachable
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		And remote server is stopped
+		When As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		Then the list of returned shares has 1 shares
+		And remote share 0 is returned with
+			| remote      | http://localhost:8180/ |
+			| name        | /remote-share.txt |
+			| owner       | user1 |
+			| user        | user0 |
+			| mountpoint  | /remote-share.txt |
 
+	Scenario: List federated share from another server no longer reachable after caching the file entry
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		# Checking that the file exists caches the file entry, which causes an
+		# exception to be thrown when getting the file info if the remote server is
+		# unreachable.
+		And as "user0" the file "/remote-share.txt" exists
+		And remote server is stopped
+		When As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		Then the list of returned shares has 1 shares
+		And remote share 0 is returned with
+			| remote      | http://localhost:8180/ |
+			| name        | /remote-share.txt |
+			| owner       | user1 |
+			| user        | user0 |
+			| mountpoint  | /remote-share.txt |
 
 
 
+	Scenario: Delete federated share with another server
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And As an "user1"
+		And sending "GET" to "/apps/files_sharing/api/v1/shares"
+		And the list of returned shares has 1 shares
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		And as "user0" the file "/remote-share.txt" exists
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 1 shares
+		And Using server "REMOTE"
+		When As an "user1"
+		And Deleting last share
+		Then the OCS status code should be "100"
+		And the HTTP status code should be "200"
+		And As an "user1"
+		And sending "GET" to "/apps/files_sharing/api/v1/shares"
+		And the list of returned shares has 0 shares
+		And Using server "LOCAL"
+		And as "user0" the file "/remote-share.txt" does not exist
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 0 shares
 
+	Scenario: Delete federated share from another server
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And As an "user1"
+		And sending "GET" to "/apps/files_sharing/api/v1/shares"
+		And the list of returned shares has 1 shares
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		And as "user0" the file "/remote-share.txt" exists
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 1 shares
+		When user "user0" deletes last accepted remote share
+		Then the OCS status code should be "100"
+		And the HTTP status code should be "200"
+		And as "user0" the file "/remote-share.txt" does not exist
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 0 shares
+		And Using server "REMOTE"
+		And As an "user1"
+		And sending "GET" to "/apps/files_sharing/api/v1/shares"
+		And the list of returned shares has 0 shares
 
+	Scenario: Delete federated share from another server no longer reachable
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		And as "user0" the file "/remote-share.txt" exists
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 1 shares
+		And remote server is stopped
+		When user "user0" deletes last accepted remote share
+		Then the OCS status code should be "100"
+		And the HTTP status code should be "200"
+		And as "user0" the file "/remote-share.txt" does not exist
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 0 shares
 
+	Scenario: Delete federated share file from another server
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And As an "user1"
+		And sending "GET" to "/apps/files_sharing/api/v1/shares"
+		And the list of returned shares has 1 shares
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		And as "user0" the file "/remote-share.txt" exists
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 1 shares
+		When User "user0" deletes file "/remote-share.txt"
+		Then the HTTP status code should be "204"
+		And as "user0" the file "/remote-share.txt" does not exist
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 0 shares
+		And Using server "REMOTE"
+		And As an "user1"
+		And sending "GET" to "/apps/files_sharing/api/v1/shares"
+		And the list of returned shares has 0 shares
 
+	Scenario: Delete federated share file from another server no longer reachable
+		Given Using server "LOCAL"
+		And user "user0" exists
+		Given Using server "REMOTE"
+		And user "user1" exists
+		# Rename file so it has a unique name in the target server (as the target
+		# server may have its own /textfile0.txt" file)
+		And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+		And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+		And Using server "LOCAL"
+		And User "user0" from server "LOCAL" accepts last pending share
+		And as "user0" the file "/remote-share.txt" exists
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 1 shares
+		And remote server is stopped
+		When User "user0" deletes file "/remote-share.txt"
+		Then the HTTP status code should be "204"
+		And as "user0" the file "/remote-share.txt" does not exist
+		And As an "user0"
+		And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
+		And the list of returned shares has 0 shares
diff --git a/build/integration/run.sh b/build/integration/run.sh
@@ -38,11 +38,10 @@ php -S localhost:$PORT -t ../.. &
 PHPPID=$!
 echo $PHPPID
 
+# The federated server is started and stopped by the tests themselves
 PORT_FED=$((8180 + $EXECUTOR_NUMBER))
 echo $PORT_FED
-php -S localhost:$PORT_FED -t ../.. &
-PHPPID_FED=$!
-echo $PHPPID_FED
+export PORT_FED
 
 export TEST_SERVER_URL="http://localhost:$PORT/ocs/"
 export TEST_SERVER_FED_URL="http://localhost:$PORT_FED/ocs/"
@@ -65,7 +64,6 @@ vendor/bin/behat --strict -f junit -f pretty $TAGS $SCENARIO_TO_RUN
 RESULT=$?
 
 kill $PHPPID
-kill $PHPPID_FED
 
 if [ "$INSTALLED" == "true" ]; then
 

diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
@@ -3943,7 +3943,7 @@
     </NoInterfaceProperties>
   </file>
   <file src="lib/private/Files/Storage/Wrapper/Availability.php">
-    <InvalidNullableReturnType occurrences="34">
+    <InvalidNullableReturnType occurrences="33">
       <code>copy</code>
       <code>copyFromStorage</code>
       <code>file_exists</code>
@@ -3960,7 +3960,6 @@
       <code>getMimeType</code>
       <code>getOwner</code>
       <code>getPermissions</code>
-      <code>hasUpdated</code>
       <code>hash</code>
       <code>isCreatable</code>
       <code>isDeletable</code>