Skip to content

feat(external-storage): Implement IPartialMountPoint #57497

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

Draft
CarlSchwan wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat(external-storage): Implement IPartialMountPoint #57497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
110 changes: 94 additions & 16 deletions apps/files_external/lib/Config/ConfigAdapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
use OCA\Files_External\MountConfig;
use OCA\Files_External\Service\UserGlobalStoragesService;
use OCA\Files_External\Service\UserStoragesService;
use OCP\AppFramework\QueryException;
use OCP\Files\Config\IAuthoritativeMountProvider;
use OCP\Files\Config\IMountProvider;
use OCP\Files\Config\IPartialMountProvider;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\ObjectStore\IObjectStore;
use OCP\Files\Storage\IConstructableStorage;
Expand All @@ -27,13 +27,15 @@
use OCP\Files\StorageNotAvailableException;
use OCP\IUser;
use OCP\Server;
use Override;
use Psr\Clock\ClockInterface;
use Psr\Container\ContainerExceptionInterface;
use Psr\Log\LoggerInterface;

/**
* Make the old files_external config work with the new public mount config api
*/
class ConfigAdapter implements IMountProvider, IAuthoritativeMountProvider {
class ConfigAdapter implements IMountProvider, IAuthoritativeMountProvider, IPartialMountProvider {
public function __construct(
private UserStoragesService $userStoragesService,
private UserGlobalStoragesService $userGlobalStoragesService,
Expand All @@ -57,7 +59,7 @@
/**
* Process storage ready for mounting
*
* @throws QueryException
* @throws ContainerExceptionInterface
*/
private function prepareStorageConfig(StorageConfig &$storage, IUser $user): void {
foreach ($storage->getBackendOptions() as $option => $value) {
Expand All @@ -81,8 +83,6 @@

/**
* Construct the storage implementation
*
* @param StorageConfig $storageConfig
*/
private function constructStorage(StorageConfig $storageConfig): IStorage {
$class = $storageConfig->getBackend()->getStorageClass();
Expand All @@ -99,17 +99,12 @@
}

/**
* Get all mountpoints applicable for the user
*
* @return IMountPoint[]
* @param list<StorageConfig> $storageConfigs
* @return array
* @throws ContainerExceptionInterface
*/
public function getMountsForUser(IUser $user, IStorageFactory $loader) {
$this->userStoragesService->setUser($user);
$this->userGlobalStoragesService->setUser($user);

$storageConfigs = $this->userGlobalStoragesService->getAllStoragesForUser();

$storages = array_map(function (StorageConfig $storageConfig) use ($user) {
private function getAvailableStorages(array $storageConfigs, IUser $user): array {
$storages = array_map(function (StorageConfig $storageConfig) use ($user): IStorage {
try {
return $this->constructStorageForUser($user, $storageConfig);
} catch (\Exception $e) {
Expand All @@ -123,7 +118,7 @@
return $storage->getId();
}, $storages));

$availableStorages = array_map(function (IStorage $storage, StorageConfig $storageConfig): IStorage {
return array_map(function (IStorage $storage, StorageConfig $storageConfig): IStorage {
try {
$availability = $storage->getAvailability();
if (!$availability['available'] && !Availability::shouldRecheck($availability)) {
Expand All @@ -137,6 +132,19 @@
}
return $storage;
}, $storages, $storageConfigs);
}

/**
* Get all mountpoints applicable for the user
*
* @return IMountPoint[]
*/
public function getMountsForUser(IUser $user, IStorageFactory $loader): array {
$this->userStoragesService->setUser($user);
$this->userGlobalStoragesService->setUser($user);

$storageConfigs = $this->userGlobalStoragesService->getAllStoragesForUser();
$availableStorages = $this->getAvailableStorages($storageConfigs, $user);

$mounts = array_map(function (StorageConfig $storageConfig, IStorage $storage) use ($user, $loader) {
$storage->setOwner($user->getUID());
Expand Down Expand Up @@ -173,4 +181,74 @@

return $mounts;
}

#[Override]
public function getMountsForPath(string $path, bool $forChildren, array $mountProviderArgs, IStorageFactory $loader): array {

Check failure on line 186 in apps/files_external/lib/Config/ConfigAdapter.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

ParamNameMismatch 

apps/files_external/lib/Config/ConfigAdapter.php:186:42: ParamNameMismatch: Argument 1 of OCA\Files_External\Config\ConfigAdapter::getMountsForPath has wrong name $path, expecting $setupPathHint as defined by OCP\Files\Config\IPartialMountProvider::getMountsForPath (see https://psalm.dev/230)
if (empty($mountProviderArgs)) {

Check failure on line 187 in apps/files_external/lib/Config/ConfigAdapter.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

TypeDoesNotContainType 

apps/files_external/lib/Config/ConfigAdapter.php:187:7: TypeDoesNotContainType: Type non-empty-list<OCP\Files\Config\MountProviderArgs> for $mountProviderArgs is never falsy (see https://psalm.dev/056)

Check failure on line 187 in apps/files_external/lib/Config/ConfigAdapter.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

TypeDoesNotContainType 

apps/files_external/lib/Config/ConfigAdapter.php:187:7: TypeDoesNotContainType: Operand of type false is always falsy (see https://psalm.dev/056)
return [];
}

$userId = null;
$user = null;
foreach ($mountProviderArgs as $mountProviderArg) {
if ($userId === null) {
$user = $mountProviderArg->mountInfo->getUser();
$userId = $user->getUID();
} elseif ($userId !== $mountProviderArg->mountInfo->getUser()->getUID()) {
throw new \LogicException('Mounts must belong to the same user!');
}
}
Comment on lines +191 to +200
Copy link
Contributor

@artonge artonge Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if we shouldn't move that check:

server/lib/private/Files/Config/MountProviderCollection.php

Lines 91 to 116 in e83c890

public function getUserMountsFromProviderByPath(
string $providerClass,
string $path,
bool $forChildren,
array $mountProviderArgs,
): array {
$provider = $this->providers[$providerClass] ?? null;
if ($provider === null) {
return [];
}
if (!is_a($providerClass, IPartialMountProvider::class, true)) {
throw new \LogicException(
'Mount provider does not support partial mounts'
);
}
/** @var IPartialMountProvider $provider */
return $provider->getMountsForPath(
$path,
$forChildren,
$mountProviderArgs,
$this->loader,
);
}

Not sure why Salvatore didn't.


if (!$forChildren) {
// override path with mount point when fetching without children
$path = $mountProviderArgs[0]->mountInfo->getMountPoint(); // TODO: not sure what this is doing, is this getting the parent path?
}

$this->userStoragesService->setUser($user);
$this->userGlobalStoragesService->setUser($user);

$storageConfigs = $this->userGlobalStoragesService->getAllStoragesForUserWithPath($path, $forChildren);
$availableStorages = $this->getAvailableStorages($storageConfigs, $user);

$mounts = [];

$i = 0;
foreach ($storageConfigs as $storageConfig) {
$storage = $availableStorages[$i];
$i++;
$storage->setOwner($user->getUID());
$mountPoint = '/' . $user->getUID() . '/files' . $storageConfig->getMountPoint();
if ($storageConfig->getType() === StorageConfig::MOUNT_TYPE_PERSONAL) {
$mounts[$mountPoint] = new PersonalMount(
$this->userStoragesService,
$storageConfig,
$storageConfig->getId(),
new KnownMtime([
'storage' => $storage,
'clock' => $this->clock,
]),
$mountPoint,
null,
$loader,
$storageConfig->getMountOptions(),
$storageConfig->getId()
);
} else {
$mounts[$mountPoint] = new SystemMountPoint(
$storageConfig,
$storage,
$mountPoint,
null,
$loader,
$storageConfig->getMountOptions(),
$storageConfig->getId()
);
}
}

$this->userStoragesService->resetUser();
$this->userGlobalStoragesService->resetUser();

return $mounts;
}
}
Loading
Loading