Skip to content

Commit 272d614

Browse files
icewind1991icewind1991
committed
fix: improve handling updated storages
Signed-off-by: Robin Appelman <[email protected]>
1 parent be3bbf2 commit 272d614

File tree

7 files changed

+313
-77
lines changed

7 files changed

+313
-77
lines changed

‎apps/files_external/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
'OCA\\Files_External\\Event\\StorageCreatedEvent' => $baseDir . '/../lib/Event/StorageCreatedEvent.php',
4141
'OCA\\Files_External\\Event\\StorageDeletedEvent' => $baseDir . '/../lib/Event/StorageDeletedEvent.php',
4242
'OCA\\Files_External\\Event\\StorageUpdatedEvent' => $baseDir . '/../lib/Event/StorageUpdatedEvent.php',
43+
'OCA\\Files_External\\Lib\\ApplicableHelper' => $baseDir . '/../lib/Lib/ApplicableHelper.php',
4344
'OCA\\Files_External\\Lib\\Auth\\AmazonS3\\AccessKey' => $baseDir . '/../lib/Lib/Auth/AmazonS3/AccessKey.php',
4445
'OCA\\Files_External\\Lib\\Auth\\AuthMechanism' => $baseDir . '/../lib/Lib/Auth/AuthMechanism.php',
4546
'OCA\\Files_External\\Lib\\Auth\\Builtin' => $baseDir . '/../lib/Lib/Auth/Builtin.php',

‎apps/files_external/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class ComposerStaticInitFiles_External
5555
'OCA\\Files_External\\Event\\StorageCreatedEvent' => __DIR__ . '/..' . '/../lib/Event/StorageCreatedEvent.php',
5656
'OCA\\Files_External\\Event\\StorageDeletedEvent' => __DIR__ . '/..' . '/../lib/Event/StorageDeletedEvent.php',
5757
'OCA\\Files_External\\Event\\StorageUpdatedEvent' => __DIR__ . '/..' . '/../lib/Event/StorageUpdatedEvent.php',
58+
'OCA\\Files_External\\Lib\\ApplicableHelper' => __DIR__ . '/..' . '/../lib/Lib/ApplicableHelper.php',
5859
'OCA\\Files_External\\Lib\\Auth\\AmazonS3\\AccessKey' => __DIR__ . '/..' . '/../lib/Lib/Auth/AmazonS3/AccessKey.php',
5960
'OCA\\Files_External\\Lib\\Auth\\AuthMechanism' => __DIR__ . '/..' . '/../lib/Lib/Auth/AuthMechanism.php',
6061
'OCA\\Files_External\\Lib\\Auth\\Builtin' => __DIR__ . '/..' . '/../lib/Lib/Auth/Builtin.php',

‎apps/files_external/lib/Lib/ApplicableHelper.php‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Files_External\Lib;
10+
11+
use OC\User\LazyUser;
12+
use OCP\IGroupManager;
13+
use OCP\IUser;
14+
use OCP\IUserManager;
15+
16+
class ApplicableHelper {
17+
public function __construct(
18+
private readonly IUserManager $userManager,
19+
private readonly IGroupManager $groupManager,
20+
) {
21+
}
22+
23+
/**
24+
* Get all users that have access to a storage
25+
*
26+
* @return \Iterator<string, IUser>
27+
*/
28+
public function getUsersForStorage(StorageConfig $storage): \Iterator {
29+
$yielded = [];
30+
if (count($storage->getApplicableUsers()) + count($storage->getApplicableGroups()) === 0) {
31+
yield from $this->userManager->getSeenUsers();
32+
}
33+
foreach ($storage->getApplicableUsers() as $userId) {
34+
$yielded[$userId] = true;
35+
yield $userId => new LazyUser($userId, $this->userManager);
36+
}
37+
foreach ($storage->getApplicableGroups() as $groupId) {
38+
$group = $this->groupManager->get($groupId);
39+
if ($group !== null) {
40+
foreach ($group->getUsers() as $user) {
41+
if (!isset($yielded[$user->getUID()])) {
42+
$yielded[$user->getUID()] = true;
43+
yield $user->getUID() => $user;
44+
}
45+
}
46+
}
47+
}
48+
}
49+
50+
public function isApplicableForUser(StorageConfig $storage, IUser $user): bool {
51+
if (count($storage->getApplicableUsers()) + count($storage->getApplicableGroups()) === 0) {
52+
return true;
53+
}
54+
if (in_array($user->getUID(), $storage->getApplicableUsers())) {
55+
return true;
56+
}
57+
$groupIds = $this->groupManager->getUserGroupIds($user);
58+
foreach ($groupIds as $groupId) {
59+
if (in_array($groupId, $storage->getApplicableGroups())) {
60+
return true;
61+
}
62+
}
63+
return false;
64+
}
65+
66+
/**
67+
* Return all users that are applicable for storage $a, but not for $b
68+
*
69+
* @return \Iterator<IUser>
70+
*/
71+
public function diffApplicable(StorageConfig $a, StorageConfig $b): \Iterator {
72+
$aIsAll = count($a->getApplicableUsers()) + count($a->getApplicableGroups()) === 0;
73+
$bIsAll = count($b->getApplicableUsers()) + count($b->getApplicableGroups()) === 0;
74+
if ($bIsAll) {
75+
return;
76+
}
77+
78+
if ($aIsAll) {
79+
foreach ($this->getUsersForStorage($a) as $user) {
80+
if (!$this->isApplicableForUser($b, $user)) {
81+
yield $user;
82+
}
83+
}
84+
} else {
85+
$yielded = [];
86+
foreach ($a->getApplicableGroups() as $groupId) {
87+
if (!in_array($groupId, $b->getApplicableGroups())) {
88+
$group = $this->groupManager->get($groupId);
89+
if ($group) {
90+
foreach ($group->getUsers() as $user) {
91+
if (!$this->isApplicableForUser($b, $user)) {
92+
if (!isset($yielded[$user->getUID()])) {
93+
$yielded[$user->getUID()] = true;
94+
yield $user;
95+
}
96+
}
97+
}
98+
}
99+
}
100+
}
101+
foreach ($a->getApplicableUsers() as $userId) {
102+
if (!in_array($userId, $b->getApplicableUsers())) {
103+
$user = $this->userManager->get($userId);
104+
if ($user && !$this->isApplicableForUser($b, $user)) {
105+
if (!isset($yielded[$user->getUID()])) {
106+
$yielded[$user->getUID()] = true;
107+
yield $user;
108+
}
109+
}
110+
}
111+
}
112+
}
113+
}
114+
}

‎apps/files_external/lib/Lib/StorageConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ public function getMountPointForUser(IUser $user): string {
441441
return '/' . $user->getUID() . '/files/' . trim($this->mountPoint, '/') . '/';
442442
}
443443

444-
public function __clone(): void {
444+
public function __clone() {
445445
$this->backend = clone $this->backend;
446446
$this->authMechanism = clone $this->authMechanism;
447447
}

‎apps/files_external/lib/Service/DBConfigService.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
/**
1717
* Stores the mount config in the database
1818
*
19-
* @psalm-type StorageConfigData = array{type: int, priority: int, applicable: list<array{type: mixed, value: mixed}>, config: array, options: array}
19+
* @psalm-type ApplicableConfig = array{type: int, value: string}
20+
* @psalm-type StorageConfigData = array{type: int, priority: int, applicable: list<ApplicableConfig>, config: array, options: array, ...<string, mixed>}
2021
*/
2122
class DBConfigService {
2223
public const MOUNT_TYPE_ADMIN = 1;
@@ -451,9 +452,9 @@ private function getMountsFromQuery(IQueryBuilder $query): array {
451452
* @param string $table
452453
* @param string[] $fields
453454
* @param int[] $mountIds
454-
* @return array [$mountId => [['field1' => $value1, ...], ...], ...]
455+
* @return array<int, list<array>> [$mountId => [['field1' => $value1, ...], ...], ...]
455456
*/
456-
private function selectForMounts($table, array $fields, array $mountIds) {
457+
private function selectForMounts(string $table, array $fields, array $mountIds): array {
457458
if (count($mountIds) === 0) {
458459
return [];
459460
}
@@ -485,9 +486,9 @@ private function selectForMounts($table, array $fields, array $mountIds) {
485486

486487
/**
487488
* @param int[] $mountIds
488-
* @return array [$id => [['type' => $type, 'value' => $value], ...], ...]
489+
* @return array<int, list<ApplicableConfig>> [$id => [['type' => $type, 'value' => $value], ...], ...]
489490
*/
490-
public function getApplicableForMounts($mountIds) {
491+
public function getApplicableForMounts(array $mountIds): array {
491492
return $this->selectForMounts('external_applicable', ['type', 'value'], $mountIds);
492493
}
493494

‎apps/files_external/lib/Service/MountCacheService.php‎

Lines changed: 20 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@
1010

1111
use OC\Files\Cache\CacheEntry;
1212
use OC\Files\Storage\FailedStorage;
13-
use OC\User\LazyUser;
1413
use OCA\Files_External\Config\ConfigAdapter;
1514
use OCA\Files_External\Event\StorageCreatedEvent;
1615
use OCA\Files_External\Event\StorageDeletedEvent;
1716
use OCA\Files_External\Event\StorageUpdatedEvent;
17+
use OCA\Files_External\Lib\ApplicableHelper;
1818
use OCA\Files_External\Lib\StorageConfig;
1919
use OCP\Cache\CappedMemoryCache;
2020
use OCP\EventDispatcher\Event;
@@ -25,9 +25,7 @@
2525
use OCP\Group\Events\UserAddedEvent;
2626
use OCP\Group\Events\UserRemovedEvent;
2727
use OCP\IGroup;
28-
use OCP\IGroupManager;
2928
use OCP\IUser;
30-
use OCP\IUserManager;
3129
use OCP\User\Events\PostLoginEvent;
3230
use OCP\User\Events\UserCreatedEvent;
3331

@@ -42,9 +40,8 @@ class MountCacheService implements IEventListener {
4240
public function __construct(
4341
private readonly IUserMountCache $userMountCache,
4442
private readonly ConfigAdapter $configAdapter,
45-
private readonly IUserManager $userManager,
46-
private readonly IGroupManager $groupManager,
4743
private readonly GlobalStoragesService $storagesService,
44+
private readonly ApplicableHelper $applicableHelper,
4845
) {
4946
$this->storageRootCache = new CappedMemoryCache();
5047
}
@@ -76,63 +73,24 @@ public function handle(Event $event): void {
7673
}
7774
}
7875

79-
80-
/**
81-
* Get all users that have access to a storage, either directly or through a group
82-
*
83-
* @param StorageConfig $storage
84-
* @return \Iterator<string, IUser>
85-
*/
86-
private function getUsersForStorage(StorageConfig $storage): \Iterator {
87-
$yielded = [];
88-
if (count($storage->getApplicableUsers()) + count($storage->getApplicableGroups()) === 0) {
89-
yield from $this->userManager->getSeenUsers();
90-
}
91-
foreach ($storage->getApplicableUsers() as $userId) {
92-
$yielded[$userId] = true;
93-
yield $userId => new LazyUser($userId, $this->userManager);
94-
}
95-
foreach ($storage->getApplicableGroups() as $groupId) {
96-
$group = $this->groupManager->get($groupId);
97-
if ($group !== null) {
98-
foreach ($group->searchUsers('') as $user) {
99-
if (!isset($yielded[$user->getUID()])) {
100-
$yielded[$user->getUID()] = true;
101-
yield $user->getUID() => $user;
102-
}
103-
}
104-
}
105-
}
106-
}
107-
10876
public function handleDeletedStorage(StorageConfig $storage): void {
109-
foreach ($this->getUsersForStorage($storage) as $user) {
77+
foreach ($this->applicableHelper->getUsersForStorage($storage) as $user) {
11078
$this->userMountCache->removeMount($storage->getMountPointForUser($user));
11179
}
11280
}
11381

11482
public function handleAddedStorage(StorageConfig $storage): void {
115-
foreach ($this->getUsersForStorage($storage) as $user) {
83+
foreach ($this->applicableHelper->getUsersForStorage($storage) as $user) {
11684
$this->registerForUser($user, $storage);
11785
}
11886
}
11987

12088
public function handleUpdatedStorage(StorageConfig $oldStorage, StorageConfig $newStorage): void {
121-
/** @var array<string, IUser> $oldApplicable */
122-
$oldApplicable = iterator_to_array($this->getUsersForStorage($oldStorage));
123-
/** @var array<string, IUser> $newApplicable */
124-
$newApplicable = iterator_to_array($this->getUsersForStorage($newStorage));
125-
126-
foreach ($oldApplicable as $oldUser) {
127-
if (!isset($newApplicable[$oldUser->getUID()])) {
128-
$this->userMountCache->removeMount($oldStorage->getMountPointForUser($oldUser));
129-
}
89+
foreach ($this->applicableHelper->diffApplicable($oldStorage, $newStorage) as $user) {
90+
$this->userMountCache->removeMount($oldStorage->getMountPointForUser($user));
13091
}
131-
132-
foreach ($newApplicable as $newUser) {
133-
if (!isset($oldApplicable[$newUser->getUID()])) {
134-
$this->registerForUser($newUser, $newStorage);
135-
}
92+
foreach ($this->applicableHelper->diffApplicable($newStorage, $oldStorage) as $user) {
93+
$this->registerForUser($user, $newStorage);
13694
}
13795
}
13896

@@ -194,26 +152,10 @@ private function registerForUser(IUser $user, StorageConfig $storage): void {
194152
);
195153
}
196154

197-
private function isApplicableForUser(StorageConfig $storage, IUser $user): bool {
198-
if (count($storage->getApplicableUsers()) + count($storage->getApplicableGroups()) === 0) {
199-
return true;
200-
}
201-
if (in_array($user->getUID(), $storage->getApplicableUsers())) {
202-
return true;
203-
}
204-
$groupIds = $this->groupManager->getUserGroupIds($user);
205-
foreach ($groupIds as $groupId) {
206-
if (in_array($groupId, $storage->getApplicableGroups())) {
207-
return true;
208-
}
209-
}
210-
return false;
211-
}
212-
213155
private function handleUserRemoved(IGroup $group, IUser $user): void {
214156
$storages = $this->storagesService->getAllStoragesForGroup($group);
215157
foreach ($storages as $storage) {
216-
if (!$this->isApplicableForUser($storage, $user)) {
158+
if (!$this->applicableHelper->isApplicableForUser($storage, $user)) {
217159
$this->userMountCache->removeMount($storage->getMountPointForUser($user));
218160
}
219161
}
@@ -229,10 +171,17 @@ private function handleUserAdded(IGroup $group, IUser $user): void {
229171
private function handleGroupDeleted(IGroup $group): void {
230172
$storages = $this->storagesService->getAllStoragesForGroup($group);
231173
foreach ($storages as $storage) {
232-
foreach ($group->searchUsers('') as $user) {
233-
if (!$this->isApplicableForUser($storage, $user)) {
234-
$this->userMountCache->removeMount($storage->getMountPointForUser($user));
235-
}
174+
$this->removeGroupFromStorage($storage, $group);
175+
}
176+
}
177+
178+
/**
179+
* Remove mounts from users in a group, if they don't have access to the storage trough other means
180+
*/
181+
private function removeGroupFromStorage(StorageConfig $storage, IGroup $group): void {
182+
foreach ($group->searchUsers('') as $user) {
183+
if (!$this->applicableHelper->isApplicableForUser($storage, $user)) {
184+
$this->userMountCache->removeMount($storage->getMountPointForUser($user));
236185
}
237186
}
238187
}

0 commit comments

Comments
 (0)