Skip to content

Commit

Permalink
Merge pull request #39044 from nextcloud/more-empty-mount-checking
Browse files Browse the repository at this point in the history
Fix root mounts not being setup in some cases
  • Loading branch information
icewind1991 authored Oct 24, 2023
2 parents 970ac3d + 1eb3293 commit efe68d0
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 6 deletions.
4 changes: 4 additions & 0 deletions apps/files_sharing/lib/External/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,10 @@ public function removeShare($mountPoint): bool {
$this->logger->error('Mount point to remove share not found', ['mountPoint' => $mountPoint]);
return false;
}
if (!$mountPointObj instanceof Mount) {
$this->logger->error('Mount point to remove share is not an external share, share probably doesn\'t exist', ['mountPoint' => $mountPoint]);
return false;
}
$id = $mountPointObj->getStorage()->getCache()->getId('');

$mountPoint = $this->stripPath($mountPoint);
Expand Down
13 changes: 10 additions & 3 deletions apps/files_sharing/tests/External/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@
namespace OCA\Files_Sharing\Tests\External;

use OC\Federation\CloudIdManager;
use OC\Files\Mount\MountPoint;
use OC\Files\SetupManagerFactory;
use OC\Files\Storage\StorageFactory;
use OC\Files\Storage\Temporary;
use OCA\Files_Sharing\External\Manager;
use OCA\Files_Sharing\External\MountProvider;
use OCA\Files_Sharing\Tests\TestCase;
Expand Down Expand Up @@ -191,13 +193,18 @@ private function createManagerForUser($userId) {
}

private function setupMounts() {
$this->mountManager->clear();
$this->clearMounts();
$mounts = $this->testMountProvider->getMountsForUser($this->user, new StorageFactory());
foreach ($mounts as $mount) {
$this->mountManager->addMount($mount);
}
}

private function clearMounts() {
$this->mountManager->clear();
$this->mountManager->addMount(new MountPoint(Temporary::class, '', []));
}

public function testAddUserShare() {
$this->doTestAddShare([
'remote' => 'http://localhost',
Expand Down Expand Up @@ -235,7 +242,7 @@ public function doTestAddShare($shareData1, $isGroup = false) {
if ($isGroup) {
$this->manager->expects($this->never())->method('tryOCMEndPoint');
} else {
$this->manager->expects($this->any())->method('tryOCMEndPoint')
$this->manager->method('tryOCMEndPoint')
->withConsecutive(
['http://localhost', 'token1', '2342', 'accept'],
['http://localhost', 'token3', '2342', 'decline'],
Expand Down Expand Up @@ -415,7 +422,7 @@ public function doTestAddShare($shareData1, $isGroup = false) {

$this->assertEmpty(self::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted');

$this->mountManager->clear();
$this->clearMounts();
self::invokePrivate($this->manager, 'setupMounts');
$this->assertNotMount($shareData1['name']);
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
Expand Down
5 changes: 5 additions & 0 deletions lib/private/Files/Config/MountProviderCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ public function getRootMounts(): array {
$mounts = array_reduce($mounts, function (array $mounts, array $providerMounts) {
return array_merge($mounts, $providerMounts);
}, []);

if (count($mounts) === 0) {
throw new \Exception("No root mounts provided by any provider");
}

return $mounts;
}

Expand Down
11 changes: 10 additions & 1 deletion lib/private/Files/Mount/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ public function find(string $path): IMountPoint {
return $this->pathCache[$path];
}



if (count($this->mounts) === 0) {
$this->setupManager->setupRoot();
if (count($this->mounts) === 0) {
throw new \Exception("No mounts even after explicitly setting up the root mounts");
}
}

$current = $path;
while (true) {
$mountPoint = $current . '/';
Expand All @@ -117,7 +126,7 @@ public function find(string $path): IMountPoint {
}
}

throw new NotFoundException("No mount for path " . $path . " existing mounts: " . implode(",", array_keys($this->mounts)));
throw new NotFoundException("No mount for path " . $path . " existing mounts (" . count($this->mounts) ."): " . implode(",", array_keys($this->mounts)));
}

/**
Expand Down
5 changes: 3 additions & 2 deletions lib/private/Files/SetupManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,13 @@ public function setupRoot(): void {
if ($this->rootSetup) {
return;
}

$this->setupBuiltinWrappers();

$this->rootSetup = true;

$this->eventLogger->start('fs:setup:root', 'Setup root filesystem');

$this->setupBuiltinWrappers();

$rootMounts = $this->mountProviderCollection->getRootMounts();
foreach ($rootMounts as $rootMountProvider) {
$this->mountManager->addMount($rootMountProvider);
Expand Down

0 comments on commit efe68d0

Please sign in to comment.