Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable22] Fix removing remote shares when the remote server is unreachable #29565

Merged
merged 6 commits into from
Mar 10, 2022
Merged

[stable22] Fix removing remote shares when the remote server is unreachable #29565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
92e1072
Add integration tests for deleting federated shares
danxuliu Jan 22, 2021
60b8116
Add integration tests for listing federated shares
danxuliu Apr 8, 2021
6e45663
Add integration tests for federated shares from unavailable servers
danxuliu Jan 22, 2021
2421282
Return false in hasUpdated when storage is not available
PVince81 Oct 21, 2021
3f08a6d
Update psalm baseline
PVince81 Oct 22, 2021
8206b38
Update lib/private/Files/Storage/Wrapper/Availability.php add void
PVince81 Nov 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions build/integration/features/bootstrap/FederationContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
*/
use Behat\Behat\Context\Context;
use Behat\Behat\Context\SnippetAcceptingContext;
use Behat\Gherkin\Node\TableNode;

require __DIR__ . '/../../vendor/autoload.php';

Expand All @@ -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
*/
Expand Down Expand Up @@ -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
Expand All @@ -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() {
Expand Down
217 changes: 217 additions & 0 deletions build/integration/federation_features/federated.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 2 additions & 4 deletions build/integration/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/"
Expand All @@ -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

Expand Down
3 changes: 1 addition & 2 deletions build/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3884,7 +3884,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>
Expand All @@ -3901,7 +3901,6 @@
<code>getMimeType</code>
<code>getOwner</code>
<code>getPermissions</code>
<code>hasUpdated</code>
<code>hash</code>
<code>isCreatable</code>
<code>isDeletable</code>
Expand Down
Loading