diff --git a/build/integration/features/bootstrap/FederationContext.php b/build/integration/features/bootstrap/FederationContext.php index 2754cf668e61f..423708adc1053 100644 --- 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 index 17ec6b4b43e22..8b627e55a429b 100644 --- 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 index 4808ab58ef5f9..a20398e8ee9ba 100755 --- 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 index e420ab4b734c5..22c2732bf2945 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3884,7 +3884,7 @@ - + copy copyFromStorage file_exists @@ -3901,7 +3901,6 @@ getMimeType getOwner getPermissions - hasUpdated hash isCreatable isDeletable diff --git a/lib/private/Files/Storage/Wrapper/Availability.php b/lib/private/Files/Storage/Wrapper/Availability.php index b6d1ba2178ba0..910ea3697571a 100644 --- a/lib/private/Files/Storage/Wrapper/Availability.php +++ b/lib/private/Files/Storage/Wrapper/Availability.php @@ -379,11 +379,15 @@ public function getLocalFile($path) { /** {@inheritdoc} */ public function hasUpdated($path, $time) { - $this->checkAvailability(); + if (!$this->isAvailable()) { + return false; + } try { return parent::hasUpdated($path, $time); } catch (StorageNotAvailableException $e) { - $this->setUnavailable($e); + // set unavailable but don't rethrow + $this->setUnavailable(null); + return false; } } @@ -449,7 +453,7 @@ public function getMetaData($path) { /** * @throws StorageNotAvailableException */ - protected function setUnavailable(StorageNotAvailableException $e) { + protected function setUnavailable(?StorageNotAvailableException $e): void { $delay = self::RECHECK_TTL_SEC; if ($e instanceof StorageAuthException) { $delay = max( @@ -459,7 +463,9 @@ protected function setUnavailable(StorageNotAvailableException $e) { ); } $this->getStorageCache()->setAvailability(false, $delay); - throw $e; + if ($e !== null) { + throw $e; + } }