From b3c53c7436b14c5ac821dc98c361302d1b657c9a Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 5 Jun 2025 17:25:08 +0200 Subject: [PATCH] feat: allow object store configuration aliases for easier migrations Signed-off-by: Robin Appelman --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + ...validObjectStoreConfigurationException.php | 13 + .../ObjectStore/PrimaryObjectStoreConfig.php | 100 ++++-- .../PrimaryObjectStoreConfigTest.php | 285 ++++++++++++++++++ 5 files changed, 377 insertions(+), 23 deletions(-) create mode 100644 lib/private/Files/ObjectStore/InvalidObjectStoreConfigurationException.php create mode 100644 tests/lib/Files/ObjectStore/PrimaryObjectStoreConfigTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index acded1ed539..c2412f0152e 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1687,6 +1687,7 @@ return array( 'OC\\Files\\ObjectStore\\AppdataPreviewObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/AppdataPreviewObjectStoreStorage.php', 'OC\\Files\\ObjectStore\\Azure' => $baseDir . '/lib/private/Files/ObjectStore/Azure.php', 'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php', + 'OC\\Files\\ObjectStore\\InvalidObjectStoreConfigurationException' => $baseDir . '/lib/private/Files/ObjectStore/InvalidObjectStoreConfigurationException.php', 'OC\\Files\\ObjectStore\\Mapper' => $baseDir . '/lib/private/Files/ObjectStore/Mapper.php', 'OC\\Files\\ObjectStore\\ObjectStoreScanner' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php', 'OC\\Files\\ObjectStore\\ObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index bb20a68eae3..9c0cf7fb9be 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1728,6 +1728,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Files\\ObjectStore\\AppdataPreviewObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/AppdataPreviewObjectStoreStorage.php', 'OC\\Files\\ObjectStore\\Azure' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Azure.php', 'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php', + 'OC\\Files\\ObjectStore\\InvalidObjectStoreConfigurationException' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/InvalidObjectStoreConfigurationException.php', 'OC\\Files\\ObjectStore\\Mapper' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Mapper.php', 'OC\\Files\\ObjectStore\\ObjectStoreScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php', 'OC\\Files\\ObjectStore\\ObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php', diff --git a/lib/private/Files/ObjectStore/InvalidObjectStoreConfigurationException.php b/lib/private/Files/ObjectStore/InvalidObjectStoreConfigurationException.php new file mode 100644 index 00000000000..369182b069d --- /dev/null +++ b/lib/private/Files/ObjectStore/InvalidObjectStoreConfigurationException.php @@ -0,0 +1,13 @@ + + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace OC\Files\ObjectStore; + +class InvalidObjectStoreConfigurationException extends \Exception { + +} diff --git a/lib/private/Files/ObjectStore/PrimaryObjectStoreConfig.php b/lib/private/Files/ObjectStore/PrimaryObjectStoreConfig.php index a10e29a9bbb..ffc33687340 100644 --- a/lib/private/Files/ObjectStore/PrimaryObjectStoreConfig.php +++ b/lib/private/Files/ObjectStore/PrimaryObjectStoreConfig.php @@ -34,12 +34,11 @@ class PrimaryObjectStoreConfig { * @return ?ObjectStoreConfig */ public function getObjectStoreConfigForRoot(): ?array { - $configs = $this->getObjectStoreConfig(); - if (!$configs) { + if (!$this->hasObjectStore()) { return null; } - $config = $configs['root'] ?? $configs['default']; + $config = $this->getObjectStoreConfiguration('root'); if ($config['arguments']['multibucket']) { if (!isset($config['arguments']['bucket'])) { @@ -56,17 +55,12 @@ class PrimaryObjectStoreConfig { * @return ?ObjectStoreConfig */ public function getObjectStoreConfigForUser(IUser $user): ?array { - $configs = $this->getObjectStoreConfig(); - if (!$configs) { + if (!$this->hasObjectStore()) { return null; } $store = $this->getObjectStoreForUser($user); - - if (!isset($configs[$store])) { - throw new \Exception("Object store configuration for '{$store}' not found"); - } - $config = $configs[$store]; + $config = $this->getObjectStoreConfiguration($store); if ($config['arguments']['multibucket']) { $config['arguments']['bucket'] = $this->getBucketForUser($user, $config); @@ -75,9 +69,46 @@ class PrimaryObjectStoreConfig { } /** - * @return ?array + * @param string $name + * @return ObjectStoreConfig + */ + public function getObjectStoreConfiguration(string $name): array { + $configs = $this->getObjectStoreConfigs(); + $name = $this->resolveAlias($name); + if (!isset($configs[$name])) { + throw new \Exception("Object store configuration for '$name' not found"); + } + if (is_string($configs[$name])) { + throw new \Exception("Object store configuration for '{$configs[$name]}' not found"); + } + return $configs[$name]; + } + + public function resolveAlias(string $name): string { + $configs = $this->getObjectStoreConfigs(); + + while (isset($configs[$name]) && is_string($configs[$name])) { + $name = $configs[$name]; + } + return $name; + } + + public function hasObjectStore(): bool { + $objectStore = $this->config->getSystemValue('objectstore', null); + $objectStoreMultiBucket = $this->config->getSystemValue('objectstore_multibucket', null); + return $objectStore || $objectStoreMultiBucket; + } + + public function hasMultipleObjectStorages(): bool { + $objectStore = $this->config->getSystemValue('objectstore', []); + return isset($objectStore['default']); + } + + /** + * @return ?array + * @throws InvalidObjectStoreConfigurationException */ - private function getObjectStoreConfig(): ?array { + public function getObjectStoreConfigs(): ?array { $objectStore = $this->config->getSystemValue('objectstore', null); $objectStoreMultiBucket = $this->config->getSystemValue('objectstore_multibucket', null); @@ -85,15 +116,25 @@ class PrimaryObjectStoreConfig { if ($objectStoreMultiBucket) { $objectStoreMultiBucket['arguments']['multibucket'] = true; return [ - 'default' => $this->validateObjectStoreConfig($objectStoreMultiBucket) + 'default' => 'server1', + 'server1' => $this->validateObjectStoreConfig($objectStoreMultiBucket), + 'root' => 'server1', ]; } elseif ($objectStore) { if (!isset($objectStore['default'])) { $objectStore = [ - 'default' => $objectStore, + 'default' => 'server1', + 'root' => 'server1', + 'server1' => $objectStore, ]; } + if (!isset($objectStore['root'])) { + $objectStore['root'] = 'default'; + } + if (!is_string($objectStore['default'])) { + throw new InvalidObjectStoreConfigurationException('The \'default\' object storage configuration is required to be a reference to another configuration.'); + } return array_map($this->validateObjectStoreConfig(...), $objectStore); } else { return null; @@ -101,11 +142,15 @@ class PrimaryObjectStoreConfig { } /** - * @return ObjectStoreConfig + * @param array|string $config + * @return string|ObjectStoreConfig */ - private function validateObjectStoreConfig(array $config) { + private function validateObjectStoreConfig(array|string $config): array|string { + if (is_string($config)) { + return $config; + } if (!isset($config['class'])) { - throw new \Exception('No class configured for object store'); + throw new InvalidObjectStoreConfigurationException('No class configured for object store'); } if (!isset($config['arguments'])) { $config['arguments'] = []; @@ -113,17 +158,17 @@ class PrimaryObjectStoreConfig { $class = $config['class']; $arguments = $config['arguments']; if (!is_array($arguments)) { - throw new \Exception('Configured object store arguments are not an array'); + throw new InvalidObjectStoreConfigurationException('Configured object store arguments are not an array'); } if (!isset($arguments['multibucket'])) { $arguments['multibucket'] = false; } if (!is_bool($arguments['multibucket'])) { - throw new \Exception('arguments.multibucket must be a boolean in object store configuration'); + throw new InvalidObjectStoreConfigurationException('arguments.multibucket must be a boolean in object store configuration'); } if (!is_string($class)) { - throw new \Exception('Configured class for object store is not a string'); + throw new InvalidObjectStoreConfigurationException('Configured class for object store is not a string'); } if (str_starts_with($class, 'OCA\\') && substr_count($class, '\\') >= 2) { @@ -132,7 +177,7 @@ class PrimaryObjectStoreConfig { } if (!is_a($class, IObjectStore::class, true)) { - throw new \Exception('Configured class for object store is not an object store'); + throw new InvalidObjectStoreConfigurationException('Configured class for object store is not an object store'); } return [ 'class' => $class, @@ -152,7 +197,7 @@ class PrimaryObjectStoreConfig { $config['arguments']['bucket'] = ''; } $mapper = new Mapper($user, $this->config); - $numBuckets = isset($config['arguments']['num_buckets']) ? $config['arguments']['num_buckets'] : 64; + $numBuckets = $config['arguments']['num_buckets'] ?? 64; $bucket = $config['arguments']['bucket'] . $mapper->getBucket($numBuckets); $this->config->setUserValue($user->getUID(), 'homeobjectstore', 'bucket', $bucket); @@ -166,6 +211,15 @@ class PrimaryObjectStoreConfig { } public function getObjectStoreForUser(IUser $user): string { - return $this->config->getUserValue($user->getUID(), 'homeobjectstore', 'objectstore', 'default'); + if ($this->hasMultipleObjectStorages()) { + $value = $this->config->getUserValue($user->getUID(), 'homeobjectstore', 'objectstore', null); + if ($value === null) { + $value = $this->resolveAlias('default'); + $this->config->setUserValue($user->getUID(), 'homeobjectstore', 'objectstore', $value); + } + return $value; + } else { + return 'default'; + } } } diff --git a/tests/lib/Files/ObjectStore/PrimaryObjectStoreConfigTest.php b/tests/lib/Files/ObjectStore/PrimaryObjectStoreConfigTest.php new file mode 100644 index 00000000000..b60b7ca4f83 --- /dev/null +++ b/tests/lib/Files/ObjectStore/PrimaryObjectStoreConfigTest.php @@ -0,0 +1,285 @@ + + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace lib\Files\ObjectStore; + +use OC\Files\ObjectStore\PrimaryObjectStoreConfig; +use OC\Files\ObjectStore\StorageObjectStore; +use OCP\App\IAppManager; +use OCP\IConfig; +use OCP\IUser; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +class PrimaryObjectStoreConfigTest extends TestCase { + private array $systemConfig = []; + private array $userConfig = []; + private IConfig&MockObject $config; + private IAppManager&MockObject $appManager; + private PrimaryObjectStoreConfig $objectStoreConfig; + + protected function setUp(): void { + parent::setUp(); + + $this->systemConfig = []; + $this->config = $this->createMock(IConfig::class); + $this->appManager = $this->createMock(IAppManager::class); + $this->config->method('getSystemValue') + ->willReturnCallback(function ($key, $default = '') { + if (isset($this->systemConfig[$key])) { + return $this->systemConfig[$key]; + } else { + return $default; + } + }); + $this->config->method('getUserValue') + ->willReturnCallback(function ($userId, $appName, $key, $default = '') { + if (isset($this->userConfig[$userId][$appName][$key])) { + return $this->userConfig[$userId][$appName][$key]; + } else { + return $default; + } + }); + $this->config->method('setUserValue') + ->willReturnCallback(function ($userId, $appName, $key, $value) { + $this->userConfig[$userId][$appName][$key] = $value; + }); + + $this->objectStoreConfig = new PrimaryObjectStoreConfig($this->config, $this->appManager); + } + + private function getUser(string $uid): IUser { + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn($uid); + return $user; + } + + private function setConfig(string $key, $value) { + $this->systemConfig[$key] = $value; + } + + public function testNewUserGetsDefault() { + $this->setConfig('objectstore', [ + 'default' => 'server1', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + ], + ], + ]); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test')); + $this->assertEquals('server1', $result['arguments']['host']); + + $this->assertEquals('server1', $this->config->getUserValue('test', 'homeobjectstore', 'objectstore', null)); + } + + public function testExistingUserKeepsStorage() { + // setup user with `server1` as storage + $this->testNewUserGetsDefault(); + + $this->setConfig('objectstore', [ + 'default' => 'server2', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + ], + ], + 'server2' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server2', + ], + ], + ]); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test')); + $this->assertEquals('server1', $result['arguments']['host']); + + $this->assertEquals('server1', $this->config->getUserValue('test', 'homeobjectstore', 'objectstore', null)); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('other-user')); + $this->assertEquals('server2', $result['arguments']['host']); + } + + public function testNestedAliases() { + $this->setConfig('objectstore', [ + 'default' => 'a1', + 'a1' => 'a2', + 'a2' => 'server1', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + ], + ], + ]); + $this->assertEquals('server1', $this->objectStoreConfig->resolveAlias('default')); + } + + public function testMultibucketChangedConfig() { + $this->setConfig('objectstore', [ + 'default' => 'server1', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + 'multibucket' => true, + 'num_buckets' => 8, + 'bucket' => 'bucket-' + ], + ], + ]); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test')); + $this->assertEquals('server1', $result['arguments']['host']); + $this->assertEquals('bucket-7', $result['arguments']['bucket']); + + $this->setConfig('objectstore', [ + 'default' => 'server1', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + 'multibucket' => true, + 'num_buckets' => 64, + 'bucket' => 'bucket-' + ], + ], + ]); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test')); + $this->assertEquals('server1', $result['arguments']['host']); + $this->assertEquals('bucket-7', $result['arguments']['bucket']); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test-foo')); + $this->assertEquals('server1', $result['arguments']['host']); + $this->assertEquals('bucket-40', $result['arguments']['bucket']); + + $this->setConfig('objectstore', [ + 'default' => 'server2', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + 'multibucket' => true, + 'num_buckets' => 64, + 'bucket' => 'bucket-' + ], + ], + 'server2' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server2', + 'multibucket' => true, + 'num_buckets' => 16, + 'bucket' => 'bucket-' + ], + ], + ]); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test')); + $this->assertEquals('server1', $result['arguments']['host']); + $this->assertEquals('bucket-7', $result['arguments']['bucket']); + + $result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test-bar')); + $this->assertEquals('server2', $result['arguments']['host']); + $this->assertEquals('bucket-4', $result['arguments']['bucket']); + } + + public function testMultibucketOldConfig() { + $this->setConfig('objectstore_multibucket', [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + 'multibucket' => true, + 'num_buckets' => 8, + 'bucket' => 'bucket-' + ], + ]); + $configs = $this->objectStoreConfig->getObjectStoreConfigs(); + $this->assertEquals([ + 'default' => 'server1', + 'root' => 'server1', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + 'multibucket' => true, + 'num_buckets' => 8, + 'bucket' => 'bucket-' + ], + ], + ], $configs); + } + + public function testSingleObjectStore() { + $this->setConfig('objectstore', [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + ], + ]); + $configs = $this->objectStoreConfig->getObjectStoreConfigs(); + $this->assertEquals([ + 'default' => 'server1', + 'root' => 'server1', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + 'multibucket' => false, + ], + ], + ], $configs); + } + + public function testRoot() { + $this->setConfig('objectstore', [ + 'default' => 'server1', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + ], + ], + 'server2' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server2', + ], + ], + ]); + + $result = $this->objectStoreConfig->getObjectStoreConfigForRoot(); + $this->assertEquals('server1', $result['arguments']['host']); + + $this->setConfig('objectstore', [ + 'default' => 'server1', + 'root' => 'server2', + 'server1' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server1', + ], + ], + 'server2' => [ + 'class' => StorageObjectStore::class, + 'arguments' => [ + 'host' => 'server2', + ], + ], + ]); + + $result = $this->objectStoreConfig->getObjectStoreConfigForRoot(); + $this->assertEquals('server2', $result['arguments']['host']); + } +}