Skip to content

Commit a119716

Browse files
come-nccome-nc
authored
Merge pull request #58826 from nextcloud/carl/code-cleaning-storage-cache
refactor(cache-storage): Make Storage and StorageGlobal psalm strict
2 parents 44ec45e + 783e2ac commit a119716

9 files changed

Lines changed: 189 additions & 291 deletions

File tree

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

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,14 @@
1414
use OCA\Files_External\Lib\StorageConfig;
1515
use OCA\Files_External\MountConfig;
1616
use OCP\IGroup;
17+
use Override;
1718

1819
/**
1920
* Service class to manage global external storage
2021
*/
2122
class GlobalStoragesService extends StoragesService {
22-
/**
23-
* Triggers $signal for all applicable users of the given
24-
* storage
25-
*
26-
* @param StorageConfig $storage storage data
27-
* @param string $signal signal to trigger
28-
*/
29-
protected function triggerHooks(StorageConfig $storage, $signal) {
23+
#[Override]
24+
protected function triggerHooks(StorageConfig $storage, $signal): void {
3025
// FIXME: Use as expression in empty once PHP 5.4 support is dropped
3126
$applicableUsers = $storage->getApplicableUsers();
3227
$applicableGroups = $storage->getApplicableGroups();
@@ -55,15 +50,8 @@ protected function triggerHooks(StorageConfig $storage, $signal) {
5550
);
5651
}
5752

58-
/**
59-
* Triggers signal_create_mount or signal_delete_mount to
60-
* accommodate for additions/deletions in applicableUsers
61-
* and applicableGroups fields.
62-
*
63-
* @param StorageConfig $oldStorage old storage config
64-
* @param StorageConfig $newStorage new storage config
65-
*/
66-
protected function triggerChangeHooks(StorageConfig $oldStorage, StorageConfig $newStorage) {
53+
#[Override]
54+
protected function triggerChangeHooks(StorageConfig $oldStorage, StorageConfig $newStorage): void {
6755
// if mount point changed, it's like a deletion + creation
6856
if ($oldStorage->getMountPoint() !== $newStorage->getMountPoint()) {
6957
$this->eventDispatcher->dispatchTyped(new StorageDeletedEvent($oldStorage));
@@ -139,16 +127,13 @@ protected function triggerChangeHooks(StorageConfig $oldStorage, StorageConfig $
139127
}
140128
}
141129

142-
/**
143-
* Get the visibility type for this controller, used in validation
144-
*
145-
* @return int BackendService::VISIBILITY_* constants
146-
*/
147-
public function getVisibilityType() {
130+
#[Override]
131+
public function getVisibilityType(): int {
148132
return BackendService::VISIBILITY_ADMIN;
149133
}
150134

151-
protected function isApplicable(StorageConfig $config) {
135+
#[Override]
136+
protected function isApplicable(StorageConfig $config): bool {
152137
return true;
153138
}
154139

@@ -157,16 +142,14 @@ protected function isApplicable(StorageConfig $config) {
157142
*
158143
* @return StorageConfig[] map of storage id to storage config
159144
*/
160-
public function getStorageForAllUsers() {
145+
public function getStorageForAllUsers(): array {
161146
$mounts = $this->dbConfig->getAllMounts();
162147
$configs = array_map([$this, 'getStorageConfigFromDBMount'], $mounts);
163148
$configs = array_filter($configs, function ($config) {
164149
return $config instanceof StorageConfig;
165150
});
166151

167-
$keys = array_map(function (StorageConfig $config) {
168-
return $config->getId();
169-
}, $configs);
152+
$keys = array_map(static fn (StorageConfig $config) => $config->getId(), $configs);
170153

171154
return array_combine($keys, $configs);
172155
}

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

Lines changed: 36 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,10 @@
3131

3232
/**
3333
* Service class to manage external storage
34+
*
35+
* @psalm-import-type ExternalMountInfo from DBConfigService
3436
*/
3537
abstract class StoragesService {
36-
37-
/**
38-
* @param BackendService $backendService
39-
* @param DBConfigService $dbConfig
40-
* @param IEventDispatcher $eventDispatcher
41-
*/
4238
public function __construct(
4339
protected BackendService $backendService,
4440
protected DBConfigService $dbConfig,
@@ -47,24 +43,21 @@ public function __construct(
4743
) {
4844
}
4945

50-
protected function readDBConfig() {
46+
/**
47+
* @return list<ExternalMountInfo>
48+
*/
49+
protected function readDBConfig(): array {
5150
return $this->dbConfig->getAdminMounts();
5251
}
5352

54-
protected function getStorageConfigFromDBMount(array $mount) {
55-
$applicableUsers = array_filter($mount['applicable'], function ($applicable) {
56-
return $applicable['type'] === DBConfigService::APPLICABLE_TYPE_USER;
57-
});
58-
$applicableUsers = array_map(function ($applicable) {
59-
return $applicable['value'];
60-
}, $applicableUsers);
61-
62-
$applicableGroups = array_filter($mount['applicable'], function ($applicable) {
63-
return $applicable['type'] === DBConfigService::APPLICABLE_TYPE_GROUP;
64-
});
65-
$applicableGroups = array_map(function ($applicable) {
66-
return $applicable['value'];
67-
}, $applicableGroups);
53+
protected function getStorageConfigFromDBMount(array $mount): ?StorageConfig {
54+
$applicableUsers = array_filter($mount['applicable'], static fn (array $applicable): bool
55+
=> $applicable['type'] === DBConfigService::APPLICABLE_TYPE_USER);
56+
$applicableUsers = array_map(static fn (array $applicable) => $applicable['value'], $applicableUsers);
57+
58+
$applicableGroups = array_filter($mount['applicable'], static fn (array $applicable): bool
59+
=> $applicable['type'] === DBConfigService::APPLICABLE_TYPE_GROUP);
60+
$applicableGroups = array_map(static fn (array $applicable) => $applicable['value'], $applicableGroups);
6861

6962
try {
7063
$config = $this->createStorage(
@@ -99,18 +92,14 @@ protected function getStorageConfigFromDBMount(array $mount) {
9992
/**
10093
* Read the external storage config
10194
*
102-
* @return array map of storage id to storage config
95+
* @return array<int, StorageConfig> map of storage id to storage config
10396
*/
104-
protected function readConfig() {
97+
protected function readConfig(): array {
10598
$mounts = $this->readDBConfig();
106-
$configs = array_map([$this, 'getStorageConfigFromDBMount'], $mounts);
107-
$configs = array_filter($configs, function ($config) {
108-
return $config instanceof StorageConfig;
109-
});
99+
$configs = array_map($this->getStorageConfigFromDBMount(...), $mounts);
100+
$configs = array_filter($configs);
110101

111-
$keys = array_map(function (StorageConfig $config) {
112-
return $config->getId();
113-
}, $configs);
102+
$keys = array_map(static fn (StorageConfig $config): int => $config->getId(), $configs);
114103

115104
return array_combine($keys, $configs);
116105
}
@@ -139,18 +128,15 @@ public function getStorage(int $id): StorageConfig {
139128

140129
/**
141130
* Check whether this storage service should provide access to a storage
142-
*
143-
* @param StorageConfig $config
144-
* @return bool
145131
*/
146-
abstract protected function isApplicable(StorageConfig $config);
132+
abstract protected function isApplicable(StorageConfig $config): bool;
147133

148134
/**
149135
* Gets all storages, valid or not
150136
*
151137
* @return StorageConfig[] array of storage configs
152138
*/
153-
public function getAllStorages() {
139+
public function getAllStorages(): array {
154140
return $this->readConfig();
155141
}
156142

@@ -159,18 +145,16 @@ public function getAllStorages() {
159145
*
160146
* @return StorageConfig[]
161147
*/
162-
public function getStorages() {
163-
return array_filter($this->getAllStorages(), [$this, 'validateStorage']);
148+
public function getStorages(): array {
149+
return array_filter($this->getAllStorages(), $this->validateStorage(...));
164150
}
165151

166152
/**
167153
* Validate storage
168154
* FIXME: De-duplicate with StoragesController::validate()
169155
*
170-
* @param StorageConfig $storage
171-
* @return bool
172156
*/
173-
protected function validateStorage(StorageConfig $storage) {
157+
protected function validateStorage(StorageConfig $storage): bool {
174158
/** @var Backend */
175159
$backend = $storage->getBackend();
176160
/** @var AuthMechanism */
@@ -180,25 +164,19 @@ protected function validateStorage(StorageConfig $storage) {
180164
// not permitted to use backend
181165
return false;
182166
}
183-
if (!$authMechanism->isVisibleFor($this->getVisibilityType())) {
184-
// not permitted to use auth mechanism
185-
return false;
186-
}
187167

188-
return true;
168+
// permitted to use auth mechanism
169+
return $authMechanism->isVisibleFor($this->getVisibilityType());
189170
}
190171

191172
/**
192173
* Get the visibility type for this controller, used in validation
193174
*
194-
* @return int BackendService::VISIBILITY_* constants
175+
* @return BackendService::VISIBILITY_*
195176
*/
196-
abstract public function getVisibilityType();
177+
abstract public function getVisibilityType(): int;
197178

198-
/**
199-
* @return integer
200-
*/
201-
protected function getType() {
179+
protected function getType(): int {
202180
return DBConfigService::MOUNT_TYPE_ADMIN;
203181
}
204182

@@ -209,7 +187,7 @@ protected function getType() {
209187
*
210188
* @return StorageConfig storage config, with added id
211189
*/
212-
public function addStorage(StorageConfig $newStorage) {
190+
public function addStorage(StorageConfig $newStorage): StorageConfig {
213191
$allStorages = $this->readConfig();
214192

215193
$configId = $this->dbConfig->addMount(
@@ -275,7 +253,7 @@ public function createStorage(
275253
?array $applicableUsers = null,
276254
?array $applicableGroups = null,
277255
?int $priority = null,
278-
) {
256+
): StorageConfig {
279257
$backend = $this->backendService->getBackend($backendIdentifier);
280258
if (!$backend) {
281259
$backend = new InvalidBackend($backendIdentifier);
@@ -313,7 +291,7 @@ public function createStorage(
313291
* @param string $mountType hook mount type param
314292
* @param array $applicableArray array of applicable users/groups for which to trigger the hook
315293
*/
316-
protected function triggerApplicableHooks($signal, $mountPoint, $mountType, $applicableArray): void {
294+
protected function triggerApplicableHooks(string $signal, string $mountPoint, string $mountType, array $applicableArray): void {
317295
$this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent(null));
318296
foreach ($applicableArray as $applicable) {
319297
Util::emitHook(
@@ -335,7 +313,7 @@ protected function triggerApplicableHooks($signal, $mountPoint, $mountType, $app
335313
* @param StorageConfig $storage storage data
336314
* @param string $signal signal to trigger
337315
*/
338-
abstract protected function triggerHooks(StorageConfig $storage, $signal);
316+
abstract protected function triggerHooks(StorageConfig $storage, string $signal): void;
339317

340318
/**
341319
* Triggers signal_create_mount or signal_delete_mount to
@@ -345,7 +323,7 @@ abstract protected function triggerHooks(StorageConfig $storage, $signal);
345323
* @param StorageConfig $oldStorage old storage data
346324
* @param StorageConfig $newStorage new storage data
347325
*/
348-
abstract protected function triggerChangeHooks(StorageConfig $oldStorage, StorageConfig $newStorage);
326+
abstract protected function triggerChangeHooks(StorageConfig $oldStorage, StorageConfig $newStorage): void;
349327

350328
/**
351329
* Update storage to the configuration
@@ -355,7 +333,7 @@ abstract protected function triggerChangeHooks(StorageConfig $oldStorage, Storag
355333
* @return StorageConfig storage config
356334
* @throws NotFoundException if the given storage does not exist in the config
357335
*/
358-
public function updateStorage(StorageConfig $updatedStorage) {
336+
public function updateStorage(StorageConfig $updatedStorage): StorageConfig {
359337
$id = $updatedStorage->getId();
360338

361339
$existingMount = $this->dbConfig->getMountById($id);
@@ -435,7 +413,7 @@ public function updateStorage(StorageConfig $updatedStorage) {
435413
*
436414
* @throws NotFoundException if no storage was found with the given id
437415
*/
438-
public function removeStorage(int $id) {
416+
public function removeStorage(int $id): void {
439417
$existingMount = $this->dbConfig->getMountById($id);
440418

441419
if (!is_array($existingMount)) {
@@ -454,29 +432,6 @@ public function removeStorage(int $id) {
454432
$this->updateOverwriteHomeFolders();
455433
}
456434

457-
/**
458-
* Construct the storage implementation
459-
*
460-
* @param StorageConfig $storageConfig
461-
* @return int
462-
*/
463-
private function getStorageId(StorageConfig $storageConfig) {
464-
try {
465-
$class = $storageConfig->getBackend()->getStorageClass();
466-
/** @var \OC\Files\Storage\Storage $storage */
467-
$storage = new $class($storageConfig->getBackendOptions());
468-
469-
// auth mechanism should fire first
470-
$storage = $storageConfig->getBackend()->wrapStorage($storage);
471-
$storage = $storageConfig->getAuthMechanism()->wrapStorage($storage);
472-
473-
/** @var \OC\Files\Storage\Storage $storage */
474-
return $storage->getStorageCache()->getNumericId();
475-
} catch (\Exception $e) {
476-
return -1;
477-
}
478-
}
479-
480435
public function updateOverwriteHomeFolders(): void {
481436
$appIdsList = $this->appConfig->getValueArray(FilesApplication::APP_ID, ConfigLexicon::OVERWRITES_HOME_FOLDERS);
482437

0 commit comments

Comments
 (0)