Merge pull request #54920 from nextcloud/object-store-unique-buckets-names

Author: provokateurin

lib/private/Files/ObjectStore/PrimaryObjectStoreConfig.php

@@ -115,7 +115,7 @@ public function getObjectStoreConfigs(): ?array {
 		// new-style multibucket config uses the same 'objectstore' key but sets `'multibucket' => true`, transparently upgrade older style config
 		if ($objectStoreMultiBucket) {
 			$objectStoreMultiBucket['arguments']['multibucket'] = true;
-			return [
+			$configs = [
 				'default' => 'server1',
 				'server1' => $this->validateObjectStoreConfig($objectStoreMultiBucket),
 				'root' => 'server1',
@@ -135,10 +135,23 @@ public function getObjectStoreConfigs(): ?array {
 			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);
+			$configs = array_map($this->validateObjectStoreConfig(...), $objectStore);
 		} else {
 			return null;
 		}
+
+		$usedBuckets = [];
+		foreach ($configs as $config) {
+			if (is_array($config)) {
+				$bucket = $config['arguments']['bucket'] ?? '';
+				if (in_array($bucket, $usedBuckets)) {
+					throw new InvalidObjectStoreConfigurationException('Each object store configuration must use distinct bucket names');
+				}
+				$usedBuckets[] = $bucket;
+			}
+		}
+
+		return $configs;
 	}
 
 	/**

tests/lib/Files/ObjectStore/PrimaryObjectStoreConfigTest.php

@@ -91,12 +91,14 @@ public function testExistingUserKeepsStorage() {
 				'class' => StorageObjectStore::class,
 				'arguments' => [
 					'host' => 'server1',
+					'bucket' => '1',
 				],
 			],
 			'server2' => [
 				'class' => StorageObjectStore::class,
 				'arguments' => [
 					'host' => 'server2',
+					'bucket' => '2',
 				],
 			],
 		]);
@@ -119,6 +121,7 @@ public function testNestedAliases() {
 				'class' => StorageObjectStore::class,
 				'arguments' => [
 					'host' => 'server1',
+					'bucket' => '1',
 				],
 			],
 		]);
@@ -134,14 +137,14 @@ public function testMultibucketChangedConfig() {
 					'host' => 'server1',
 					'multibucket' => true,
 					'num_buckets' => 8,
-					'bucket' => 'bucket-'
+					'bucket' => 'bucket1-'
 				],
 			],
 		]);
 
 		$result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test'));
 		$this->assertEquals('server1', $result['arguments']['host']);
-		$this->assertEquals('bucket-7', $result['arguments']['bucket']);
+		$this->assertEquals('bucket1-7', $result['arguments']['bucket']);
 
 		$this->setConfig('objectstore', [
 			'default' => 'server1',
@@ -151,18 +154,18 @@ public function testMultibucketChangedConfig() {
 					'host' => 'server1',
 					'multibucket' => true,
 					'num_buckets' => 64,
-					'bucket' => 'bucket-'
+					'bucket' => 'bucket1-'
 				],
 			],
 		]);
 
 		$result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test'));
 		$this->assertEquals('server1', $result['arguments']['host']);
-		$this->assertEquals('bucket-7', $result['arguments']['bucket']);
+		$this->assertEquals('bucket1-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->assertEquals('bucket1-40', $result['arguments']['bucket']);
 
 		$this->setConfig('objectstore', [
 			'default' => 'server2',
@@ -172,7 +175,7 @@ public function testMultibucketChangedConfig() {
 					'host' => 'server1',
 					'multibucket' => true,
 					'num_buckets' => 64,
-					'bucket' => 'bucket-'
+					'bucket' => 'bucket1-'
 				],
 			],
 			'server2' => [
@@ -181,18 +184,18 @@ public function testMultibucketChangedConfig() {
 					'host' => 'server2',
 					'multibucket' => true,
 					'num_buckets' => 16,
-					'bucket' => 'bucket-'
+					'bucket' => 'bucket2-'
 				],
 			],
 		]);
 
 		$result = $this->objectStoreConfig->getObjectStoreConfigForUser($this->getUser('test'));
 		$this->assertEquals('server1', $result['arguments']['host']);
-		$this->assertEquals('bucket-7', $result['arguments']['bucket']);
+		$this->assertEquals('bucket1-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']);
+		$this->assertEquals('bucket2-4', $result['arguments']['bucket']);
 	}
 
 	public function testMultibucketOldConfig() {
@@ -249,12 +252,14 @@ public function testRoot() {
 				'class' => StorageObjectStore::class,
 				'arguments' => [
 					'host' => 'server1',
+					'bucket' => '1',
 				],
 			],
 			'server2' => [
 				'class' => StorageObjectStore::class,
 				'arguments' => [
 					'host' => 'server2',
+					'bucket' => '2',
 				],
 			],
 		]);
@@ -269,12 +274,14 @@ public function testRoot() {
 				'class' => StorageObjectStore::class,
 				'arguments' => [
 					'host' => 'server1',
+					'bucket' => '1',
 				],
 			],
 			'server2' => [
 				'class' => StorageObjectStore::class,
 				'arguments' => [
 					'host' => 'server2',
+					'bucket' => '2',
 				],
 			],
 		]);