Skip to content

Commit 6d5f10e

Browse files
PVince81PVince81
authored
Merge pull request #25332 from nextcloud/fix-removing-remote-shares-when-the-remote-server-is-unreachable
Fix removing remote shares when the remote server is unreachable
2 parents ec4474c + 93fb33d commit 6d5f10e

File tree

5 files changed

+309
-10
lines changed

5 files changed

+309
-10
lines changed

‎build/integration/features/bootstrap/FederationContext.php‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
*/
2929
use Behat\Behat\Context\Context;
3030
use Behat\Behat\Context\SnippetAcceptingContext;
31+
use Behat\Gherkin\Node\TableNode;
3132

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

@@ -39,6 +40,29 @@ class FederationContext implements Context, SnippetAcceptingContext {
3940
use AppConfiguration;
4041
use CommandLine;
4142

43+
/** @var string */
44+
private static $phpFederatedServerPid = '';
45+
46+
/** @var string */
47+
private $lastAcceptedRemoteShareId;
48+
49+
/**
50+
* @BeforeScenario
51+
* @AfterScenario
52+
*
53+
* The server is started also after the scenarios to ensure that it is
54+
* properly cleaned up if stopped.
55+
*/
56+
public function startFederatedServer() {
57+
if (self::$phpFederatedServerPid !== '') {
58+
return;
59+
}
60+
61+
$port = getenv('PORT_FED');
62+
63+
self::$phpFederatedServerPid = exec('php -S localhost:' . $port . ' -t ../../ >/dev/null & echo $!');
64+
}
65+
4266
/**
4367
* @BeforeScenario
4468
*/
@@ -93,6 +117,37 @@ public function federateGroupSharing($sharerUser, $sharerServer, $sharerPath, $s
93117
$this->usingServer($previous);
94118
}
95119

120+
/**
121+
* @Then remote share :count is returned with
122+
*
123+
* @param int $number
124+
* @param TableNode $body
125+
*/
126+
public function remoteShareXIsReturnedWith(int $number, TableNode $body) {
127+
$this->theHTTPStatusCodeShouldBe('200');
128+
$this->theOCSStatusCodeShouldBe('100');
129+
130+
if (!($body instanceof TableNode)) {
131+
return;
132+
}
133+
134+
$returnedShare = $this->getXmlResponse()->data[0];
135+
if ($returnedShare->element) {
136+
$returnedShare = $returnedShare->element[$number];
137+
}
138+
139+
$defaultExpectedFields = [
140+
'id' => 'A_NUMBER',
141+
'remote_id' => 'A_NUMBER',
142+
'accepted' => '1',
143+
];
144+
$expectedFields = array_merge($defaultExpectedFields, $body->getRowsHash());
145+
146+
foreach ($expectedFields as $field => $value) {
147+
$this->assertFieldIsInReturnedShare($field, $value, $returnedShare);
148+
}
149+
}
150+
96151
/**
97152
* @When /^User "([^"]*)" from server "(LOCAL|REMOTE)" accepts last pending share$/
98153
* @param string $user
@@ -109,6 +164,30 @@ public function acceptLastPendingShare($user, $server) {
109164
$this->theHTTPStatusCodeShouldBe('200');
110165
$this->theOCSStatusCodeShouldBe('100');
111166
$this->usingServer($previous);
167+
168+
$this->lastAcceptedRemoteShareId = $share_id;
169+
}
170+
171+
/**
172+
* @When /^user "([^"]*)" deletes last accepted remote share$/
173+
* @param string $user
174+
*/
175+
public function deleteLastAcceptedRemoteShare($user) {
176+
$this->asAn($user);
177+
$this->sendingToWith('DELETE', "/apps/files_sharing/api/v1/remote_shares/" . $this->lastAcceptedRemoteShareId, null);
178+
}
179+
180+
/**
181+
* @When /^remote server is stopped$/
182+
*/
183+
public function remoteServerIsStopped() {
184+
if (self::$phpFederatedServerPid === '') {
185+
return;
186+
}
187+
188+
exec('kill ' . self::$phpFederatedServerPid);
189+
190+
self::$phpFederatedServerPid = '';
112191
}
113192

114193
protected function resetAppConfigs() {

‎build/integration/federation_features/federated.feature‎

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,13 +278,230 @@ Feature: federated
278278

279279

280280

281+
Scenario: List federated share from another server not accepted yet
282+
Given Using server "LOCAL"
283+
And user "user0" exists
284+
Given Using server "REMOTE"
285+
And user "user1" exists
286+
# Rename file so it has a unique name in the target server (as the target
287+
# server may have its own /textfile0.txt" file)
288+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
289+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
290+
And Using server "LOCAL"
291+
When As an "user0"
292+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
293+
Then the list of returned shares has 0 shares
281294

295+
Scenario: List federated share from another server
296+
Given Using server "LOCAL"
297+
And user "user0" exists
298+
Given Using server "REMOTE"
299+
And user "user1" exists
300+
# Rename file so it has a unique name in the target server (as the target
301+
# server may have its own /textfile0.txt" file)
302+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
303+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
304+
And Using server "LOCAL"
305+
And User "user0" from server "LOCAL" accepts last pending share
306+
When As an "user0"
307+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
308+
Then the list of returned shares has 1 shares
309+
And remote share 0 is returned with
310+
| remote | http://localhost:8180/ |
311+
| name | /remote-share.txt |
312+
| owner | user1 |
313+
| user | user0 |
314+
| mountpoint | /remote-share.txt |
315+
| mimetype | text/plain |
316+
| mtime | A_NUMBER |
317+
| permissions | 27 |
318+
| type | file |
319+
| file_id | A_NUMBER |
282320

321+
Scenario: List federated share from another server no longer reachable
322+
Given Using server "LOCAL"
323+
And user "user0" exists
324+
Given Using server "REMOTE"
325+
And user "user1" exists
326+
# Rename file so it has a unique name in the target server (as the target
327+
# server may have its own /textfile0.txt" file)
328+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
329+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
330+
And Using server "LOCAL"
331+
And User "user0" from server "LOCAL" accepts last pending share
332+
And remote server is stopped
333+
When As an "user0"
334+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
335+
Then the list of returned shares has 1 shares
336+
And remote share 0 is returned with
337+
| remote | http://localhost:8180/ |
338+
| name | /remote-share.txt |
339+
| owner | user1 |
340+
| user | user0 |
341+
| mountpoint | /remote-share.txt |
283342

343+
Scenario: List federated share from another server no longer reachable after caching the file entry
344+
Given Using server "LOCAL"
345+
And user "user0" exists
346+
Given Using server "REMOTE"
347+
And user "user1" exists
348+
# Rename file so it has a unique name in the target server (as the target
349+
# server may have its own /textfile0.txt" file)
350+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
351+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
352+
And Using server "LOCAL"
353+
And User "user0" from server "LOCAL" accepts last pending share
354+
# Checking that the file exists caches the file entry, which causes an
355+
# exception to be thrown when getting the file info if the remote server is
356+
# unreachable.
357+
And as "user0" the file "/remote-share.txt" exists
358+
And remote server is stopped
359+
When As an "user0"
360+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
361+
Then the list of returned shares has 1 shares
362+
And remote share 0 is returned with
363+
| remote | http://localhost:8180/ |
364+
| name | /remote-share.txt |
365+
| owner | user1 |
366+
| user | user0 |
367+
| mountpoint | /remote-share.txt |
284368

285369

286370

371+
Scenario: Delete federated share with another server
372+
Given Using server "LOCAL"
373+
And user "user0" exists
374+
Given Using server "REMOTE"
375+
And user "user1" exists
376+
# Rename file so it has a unique name in the target server (as the target
377+
# server may have its own /textfile0.txt" file)
378+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
379+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
380+
And As an "user1"
381+
And sending "GET" to "/apps/files_sharing/api/v1/shares"
382+
And the list of returned shares has 1 shares
383+
And Using server "LOCAL"
384+
And User "user0" from server "LOCAL" accepts last pending share
385+
And as "user0" the file "/remote-share.txt" exists
386+
And As an "user0"
387+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
388+
And the list of returned shares has 1 shares
389+
And Using server "REMOTE"
390+
When As an "user1"
391+
And Deleting last share
392+
Then the OCS status code should be "100"
393+
And the HTTP status code should be "200"
394+
And As an "user1"
395+
And sending "GET" to "/apps/files_sharing/api/v1/shares"
396+
And the list of returned shares has 0 shares
397+
And Using server "LOCAL"
398+
And as "user0" the file "/remote-share.txt" does not exist
399+
And As an "user0"
400+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
401+
And the list of returned shares has 0 shares
287402

403+
Scenario: Delete federated share from another server
404+
Given Using server "LOCAL"
405+
And user "user0" exists
406+
Given Using server "REMOTE"
407+
And user "user1" exists
408+
# Rename file so it has a unique name in the target server (as the target
409+
# server may have its own /textfile0.txt" file)
410+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
411+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
412+
And As an "user1"
413+
And sending "GET" to "/apps/files_sharing/api/v1/shares"
414+
And the list of returned shares has 1 shares
415+
And Using server "LOCAL"
416+
And User "user0" from server "LOCAL" accepts last pending share
417+
And as "user0" the file "/remote-share.txt" exists
418+
And As an "user0"
419+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
420+
And the list of returned shares has 1 shares
421+
When user "user0" deletes last accepted remote share
422+
Then the OCS status code should be "100"
423+
And the HTTP status code should be "200"
424+
And as "user0" the file "/remote-share.txt" does not exist
425+
And As an "user0"
426+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
427+
And the list of returned shares has 0 shares
428+
And Using server "REMOTE"
429+
And As an "user1"
430+
And sending "GET" to "/apps/files_sharing/api/v1/shares"
431+
And the list of returned shares has 0 shares
288432

433+
Scenario: Delete federated share from another server no longer reachable
434+
Given Using server "LOCAL"
435+
And user "user0" exists
436+
Given Using server "REMOTE"
437+
And user "user1" exists
438+
# Rename file so it has a unique name in the target server (as the target
439+
# server may have its own /textfile0.txt" file)
440+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
441+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
442+
And Using server "LOCAL"
443+
And User "user0" from server "LOCAL" accepts last pending share
444+
And as "user0" the file "/remote-share.txt" exists
445+
And As an "user0"
446+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
447+
And the list of returned shares has 1 shares
448+
And remote server is stopped
449+
When user "user0" deletes last accepted remote share
450+
Then the OCS status code should be "100"
451+
And the HTTP status code should be "200"
452+
And as "user0" the file "/remote-share.txt" does not exist
453+
And As an "user0"
454+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
455+
And the list of returned shares has 0 shares
289456

457+
Scenario: Delete federated share file from another server
458+
Given Using server "LOCAL"
459+
And user "user0" exists
460+
Given Using server "REMOTE"
461+
And user "user1" exists
462+
# Rename file so it has a unique name in the target server (as the target
463+
# server may have its own /textfile0.txt" file)
464+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
465+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
466+
And As an "user1"
467+
And sending "GET" to "/apps/files_sharing/api/v1/shares"
468+
And the list of returned shares has 1 shares
469+
And Using server "LOCAL"
470+
And User "user0" from server "LOCAL" accepts last pending share
471+
And as "user0" the file "/remote-share.txt" exists
472+
And As an "user0"
473+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
474+
And the list of returned shares has 1 shares
475+
When User "user0" deletes file "/remote-share.txt"
476+
Then the HTTP status code should be "204"
477+
And as "user0" the file "/remote-share.txt" does not exist
478+
And As an "user0"
479+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
480+
And the list of returned shares has 0 shares
481+
And Using server "REMOTE"
482+
And As an "user1"
483+
And sending "GET" to "/apps/files_sharing/api/v1/shares"
484+
And the list of returned shares has 0 shares
290485

486+
Scenario: Delete federated share file from another server no longer reachable
487+
Given Using server "LOCAL"
488+
And user "user0" exists
489+
Given Using server "REMOTE"
490+
And user "user1" exists
491+
# Rename file so it has a unique name in the target server (as the target
492+
# server may have its own /textfile0.txt" file)
493+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
494+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
495+
And Using server "LOCAL"
496+
And User "user0" from server "LOCAL" accepts last pending share
497+
And as "user0" the file "/remote-share.txt" exists
498+
And As an "user0"
499+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
500+
And the list of returned shares has 1 shares
501+
And remote server is stopped
502+
When User "user0" deletes file "/remote-share.txt"
503+
Then the HTTP status code should be "204"
504+
And as "user0" the file "/remote-share.txt" does not exist
505+
And As an "user0"
506+
And sending "GET" to "/apps/files_sharing/api/v1/remote_shares"
507+
And the list of returned shares has 0 shares

‎build/integration/run.sh‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,10 @@ php -S localhost:$PORT -t ../.. &
3838
PHPPID=$!
3939
echo $PHPPID
4040

41+
# The federated server is started and stopped by the tests themselves
4142
PORT_FED=$((8180 + $EXECUTOR_NUMBER))
4243
echo $PORT_FED
43-
php -S localhost:$PORT_FED -t ../.. &
44-
PHPPID_FED=$!
45-
echo $PHPPID_FED
44+
export PORT_FED
4645

4746
export TEST_SERVER_URL="http://localhost:$PORT/ocs/"
4847
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
6564
RESULT=$?
6665

6766
kill $PHPPID
68-
kill $PHPPID_FED
6967

7068
if [ "$INSTALLED" == "true" ]; then
7169

‎build/psalm-baseline.xml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3833,7 +3833,7 @@
38333833
</NoInterfaceProperties>
38343834
</file>
38353835
<file src="lib/private/Files/Storage/Wrapper/Availability.php">
3836-
<InvalidNullableReturnType occurrences="34">
3836+
<InvalidNullableReturnType occurrences="33">
38373837
<code>copy</code>
38383838
<code>copyFromStorage</code>
38393839
<code>file_exists</code>
@@ -3850,7 +3850,6 @@
38503850
<code>getMimeType</code>
38513851
<code>getOwner</code>
38523852
<code>getPermissions</code>
3853-
<code>hasUpdated</code>
38543853
<code>hash</code>
38553854
<code>isCreatable</code>
38563855
<code>isDeletable</code>

0 commit comments

Comments
 (0)