Objectstore multibucket #24760
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,9 +52,27 @@ public function __construct(IConfig $config) { | |
| * @return \OCP\Files\Mount\IMountPoint[] | ||
| */ | ||
| public function getHomeMountForUser(IUser $user, IStorageFactory $loader) { | ||
|
|
||
| $config = $this->getMultiBucketObjectStoreConfig($user); | ||
| if ($config === null) { | ||
| $config = $this->getSingleBucketObjectStoreConfig($user); | ||
| } | ||
|
|
||
| if ($config === null) { | ||
| return null; | ||
| } | ||
|
|
||
| return new MountPoint('\OC\Files\ObjectStore\HomeObjectStoreStorage', '/' . $user->getUID(), $config['arguments'], $loader); | ||
| } | ||
|
|
||
| /** | ||
| * @param IUser $user | ||
| * @return array|null | ||
| */ | ||
| private function getSingleBucketObjectStoreConfig(IUser $user) { | ||
| $config = $this->config->getSystemValue('objectstore'); | ||
| if (!is_array($config)) { | ||
| return null; | ||
| } | ||
|
|
||
| // sanity checks | ||
|
|
@@ -68,6 +86,49 @@ public function getHomeMountForUser(IUser $user, IStorageFactory $loader) { | |
| // instantiate object store implementation | ||
| $config['arguments']['objectstore'] = new $config['class']($config['arguments']); | ||
|
|
||
| return $config; | ||
| } | ||
|
|
||
| /** | ||
| * @param IUser $user | ||
| * @return array|null | ||
| */ | ||
| private function getMultiBucketObjectStoreConfig(IUser $user) { | ||
| $config = $this->config->getSystemValue('objectstore_multibucket'); | ||
| if (!is_array($config)) { | ||
| return null; | ||
| } | ||
|
|
||
| // sanity checks | ||
| if (empty($config['class'])) { | ||
| \OCP\Util::writeLog('files', 'No class given for objectstore', \OCP\Util::ERROR); | ||
| } | ||
| if (!isset($config['arguments'])) { | ||
| $config['arguments'] = []; | ||
| } | ||
| $config['arguments']['user'] = $user; | ||
|
|
||
| $bucket = $this->config->getUserValue($user->getUID(), 'homeobjectstore', 'bucket', null); | ||
|
|
||
| if ($bucket === null) { | ||
| /* | ||
| * Use any provided bucket argument as prefix | ||
| * and add the mapping from username => bucket | ||
| */ | ||
| if (!isset($config['arguments']['bucket'])) { | ||
| $config['arguments']['bucket'] = ''; | ||
| } | ||
| $mapper = new \OC\Files\ObjectStore\Mapper($user); | ||
| $config['arguments']['bucket'] .= $mapper->getBucket(); | ||
|
There was a problem hiding this comment. Well, this part obviously depends on the object storage type. If we dou it like this we would add switch case statements and it would be impossible to implement the next objectstorage api without touching core. I vote for extending the @DeepDiver1975 for architecture. There was a problem hiding this comment. Or we add a if ($storage implements IBucketMapper) {
$bucket = $storage->mapBucket();
}And we don't need to touch current implementations unless we want them to support multiple buckets. There was a problem hiding this comment. I can have a look next week once back from vacation .... Generally speaking please implement features as discussed and desribed ... Otherwise please ping me before wasting time. Thanks There was a problem hiding this comment. As stated above. Eventually we should add the mapper to the config as a kind of factory that can map userNames to buckets. So people can also write their own mapper. But for now I think it would be best to stick to a default one. We don't even properly test this yet so I'd like to keep stuff private for 9.1 so we can figure out what we actually need to get it to work for 9.2 There was a problem hiding this comment. I still consider it problematic that this approach breaks the self containment of objectstorage implementations. There was a problem hiding this comment. @butonic but how would we do self containment... while at the same time letting admins decide which mapper they want to use? There was a problem hiding this comment. The object storage implementation needs to read the mapping class to instantiate from the config. We could also pass in the Mapper to the IObjectStorage implementation as a dependency and add a getBucket() to the IObjectStorage interface. Then the mappe could be used in the constructor to determine the final bucket. ... this is all crap The root cause of the problem is that the configuration parameter for the bucket differ based on implementation. So we cannot really map the bucket outside of them. What are the options:
What about a config option like: $CONFIG = [
'objectstore' => [
'class' => 'OCA\ObjectStore\S3',
'arguments' => [
'bucket' => [
'class' => '\OC\Files\ObjectStore\Mapper\UserHash',
'param' => 'bucket', // for swift this would be 'container'
'arguments' => [
// just ideas:
'prefix' => 'oc_',
'hashlength' => 3,
'algorithm' => 'sha256',
]
],
'options' => [ ... ],
],
],
];Instead of a plain string we bass an array with the Mapper class to instantiate. This allows tweaking the mapper further and specifying the We can now specify which mapper to use (or how to distribute users over objects) as well as change the options of a IObjectStorage implementation without changing code in core. Taking this one step further this still prevents us from distributing the files of a user over multiple buckets. For that the mapper would have to be injected into the IObjectStorage implementation. But I guess that is a problem for later. There was a problem hiding this comment. mmmm sure... we could also do something like that... altough then reusing bucket is bad since that is used by the S3 config... It all is not clean basically :( But does this need to happen now? or should we slowly migrate during 9.2? I don't really feel comfortable doing design like this hours before the freeze... There was a problem hiding this comment. This PR was already merged in 9.1. |
||
|
|
||
| $this->config->setUserValue($user->getUID(), 'homeobjectstore', 'bucket', $config['arguments']['bucket']); | ||
|
|
||
| } else { | ||
| $config['arguments']['bucket'] = $bucket; | ||
| } | ||
|
|
||
| // instantiate object store implementation | ||
| $config['arguments']['objectstore'] = new $config['class']($config['arguments']); | ||
|
|
||
| return $config; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| <?php | ||
| /** | ||
| * @author Roeland Jago Douma <rullzer@owncloud.com> | ||
| * | ||
| * @copyright Copyright (c) 2016, ownCloud, Inc. | ||
| * @license AGPL-3.0 | ||
| * | ||
| * This code is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License, version 3, | ||
| * as published by the Free Software Foundation. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License, version 3, | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/> | ||
| * | ||
| */ | ||
| namespace OC\Files\ObjectStore; | ||
|
|
||
| use OCP\IUser; | ||
|
|
||
| /** | ||
| * Class Mapper | ||
| * | ||
| * @package OC\Files\ObjectStore | ||
| * | ||
| * Map a user to a bucket. | ||
| */ | ||
| class Mapper { | ||
| /** @var IUser */ | ||
| private $user; | ||
|
|
||
| /** | ||
| * Mapper constructor. | ||
| * | ||
| * @param IUser $user | ||
| */ | ||
| public function __construct(IUser $user) { | ||
| $this->user = $user; | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
| public function getBucket() { | ||
| $hash = md5($this->user->getUID()); | ||
| return substr($hash, 0, 3); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,244 @@ | ||
| <?php | ||
|
|
||
| namespace Test\Files\Mount; | ||
|
|
||
| use OC\Files\Mount\ObjectHomeMountProvider; | ||
| use OCP\Files\Storage\IStorageFactory; | ||
| use OCP\IConfig; | ||
| use OCP\IUser; | ||
|
|
||
| class ObjectHomeMountProviderTest extends \Test\TestCase { | ||
|
|
||
| /** @var ObjectHomeMountProvider */ | ||
| protected $provider; | ||
|
|
||
| /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ | ||
| protected $config; | ||
|
|
||
| /** @var IUser|\PHPUnit_Framework_MockObject_MockObject */ | ||
| protected $user; | ||
|
|
||
| /** @var IStorageFactory|\PHPUnit_Framework_MockObject_MockObject */ | ||
| protected $loader; | ||
|
|
||
| public function setUp() { | ||
| parent::setUp(); | ||
|
|
||
| $this->config = $this->getMock('OCP\IConfig'); | ||
| $this->user = $this->getMock('OCP\IUser'); | ||
| $this->loader = $this->getMock('OCP\Files\Storage\IStorageFactory'); | ||
|
|
||
| $this->provider = new ObjectHomeMountProvider($this->config); | ||
| } | ||
|
|
||
| public function testSingleBucket() { | ||
| $this->config->expects($this->once()) | ||
| ->method('getSystemValue') | ||
| ->with($this->equalTo('objectstore'), '') | ||
| ->willReturn([ | ||
| 'class' => 'Test\Files\Mount\FakeObjectStore', | ||
| ]); | ||
|
|
||
| $this->user->expects($this->never())->method($this->anything()); | ||
| $this->loader->expects($this->never())->method($this->anything()); | ||
|
|
||
| $config = $this->invokePrivate($this->provider, 'getSingleBucketObjectStoreConfig', [$this->user, $this->loader]); | ||
|
|
||
| $this->assertArrayHasKey('class', $config); | ||
| $this->assertEquals($config['class'], 'Test\Files\Mount\FakeObjectStore'); | ||
| $this->assertArrayHasKey('arguments', $config); | ||
| $this->assertArrayHasKey('user', $config['arguments']); | ||
| $this->assertSame($this->user, $config['arguments']['user']); | ||
| $this->assertArrayHasKey('objectstore', $config['arguments']); | ||
| $this->assertInstanceOf('Test\Files\Mount\FakeObjectStore', $config['arguments']['objectstore']); | ||
| } | ||
|
|
||
| public function testMultiBucket() { | ||
| $this->config->expects($this->once()) | ||
| ->method('getSystemValue') | ||
| ->with($this->equalTo('objectstore_multibucket'), '') | ||
| ->willReturn([ | ||
| 'class' => 'Test\Files\Mount\FakeObjectStore', | ||
| ]); | ||
|
|
||
| $this->user->method('getUID') | ||
| ->willReturn('uid'); | ||
| $this->loader->expects($this->never())->method($this->anything()); | ||
|
|
||
| $this->config->expects($this->once()) | ||
| ->method('getUserValue') | ||
| ->with( | ||
| $this->equalTo('uid'), | ||
| $this->equalTo('homeobjectstore'), | ||
| $this->equalTo('bucket'), | ||
| $this->equalTo(null) | ||
| )->willReturn(null); | ||
|
|
||
| $this->config->expects($this->once()) | ||
| ->method('setUserValue') | ||
| ->with( | ||
| $this->equalTo('uid'), | ||
| $this->equalTo('homeobjectstore'), | ||
| $this->equalTo('bucket'), | ||
| $this->equalTo('987'), | ||
| $this->equalTo(null) | ||
| ); | ||
|
|
||
| $config = $this->invokePrivate($this->provider, 'getMultiBucketObjectStoreConfig', [$this->user, $this->loader]); | ||
|
|
||
| $this->assertArrayHasKey('class', $config); | ||
| $this->assertEquals($config['class'], 'Test\Files\Mount\FakeObjectStore'); | ||
| $this->assertArrayHasKey('arguments', $config); | ||
| $this->assertArrayHasKey('user', $config['arguments']); | ||
| $this->assertSame($this->user, $config['arguments']['user']); | ||
| $this->assertArrayHasKey('objectstore', $config['arguments']); | ||
| $this->assertInstanceOf('Test\Files\Mount\FakeObjectStore', $config['arguments']['objectstore']); | ||
| $this->assertArrayHasKey('bucket', $config['arguments']); | ||
| $this->assertEquals('987', $config['arguments']['bucket']); | ||
| } | ||
|
|
||
| public function testMultiBucketWithPrefix() { | ||
| $this->config->expects($this->once()) | ||
| ->method('getSystemValue') | ||
| ->with($this->equalTo('objectstore_multibucket'), '') | ||
| ->willReturn([ | ||
| 'class' => 'Test\Files\Mount\FakeObjectStore', | ||
| 'arguments' => [ | ||
| 'bucket' => 'myBucketPrefix', | ||
| ], | ||
| ]); | ||
|
|
||
| $this->user->method('getUID') | ||
| ->willReturn('uid'); | ||
| $this->loader->expects($this->never())->method($this->anything()); | ||
|
|
||
| $this->config->expects($this->once()) | ||
| ->method('getUserValue') | ||
| ->with( | ||
| $this->equalTo('uid'), | ||
| $this->equalTo('homeobjectstore'), | ||
| $this->equalTo('bucket'), | ||
| $this->equalTo(null) | ||
| )->willReturn(null); | ||
|
|
||
| $this->config->expects($this->once()) | ||
| ->method('setUserValue') | ||
| ->with( | ||
| $this->equalTo('uid'), | ||
| $this->equalTo('homeobjectstore'), | ||
| $this->equalTo('bucket'), | ||
| $this->equalTo('myBucketPrefix987'), | ||
| $this->equalTo(null) | ||
| ); | ||
|
|
||
| $config = $this->invokePrivate($this->provider, 'getMultiBucketObjectStoreConfig', [$this->user, $this->loader]); | ||
|
|
||
| $this->assertArrayHasKey('class', $config); | ||
| $this->assertEquals($config['class'], 'Test\Files\Mount\FakeObjectStore'); | ||
| $this->assertArrayHasKey('arguments', $config); | ||
| $this->assertArrayHasKey('user', $config['arguments']); | ||
| $this->assertSame($this->user, $config['arguments']['user']); | ||
| $this->assertArrayHasKey('objectstore', $config['arguments']); | ||
| $this->assertInstanceOf('Test\Files\Mount\FakeObjectStore', $config['arguments']['objectstore']); | ||
| $this->assertArrayHasKey('bucket', $config['arguments']); | ||
| $this->assertEquals('myBucketPrefix987', $config['arguments']['bucket']); | ||
| } | ||
|
|
||
| public function testMultiBucketBucketAlreadySet() { | ||
| $this->config->expects($this->once()) | ||
| ->method('getSystemValue') | ||
| ->with($this->equalTo('objectstore_multibucket'), '') | ||
| ->willReturn([ | ||
| 'class' => 'Test\Files\Mount\FakeObjectStore', | ||
| 'arguments' => [ | ||
| 'bucket' => 'myBucketPrefix', | ||
| ], | ||
| ]); | ||
|
|
||
| $this->user->method('getUID') | ||
| ->willReturn('uid'); | ||
| $this->loader->expects($this->never())->method($this->anything()); | ||
|
|
||
| $this->config->expects($this->once()) | ||
| ->method('getUserValue') | ||
| ->with( | ||
| $this->equalTo('uid'), | ||
| $this->equalTo('homeobjectstore'), | ||
| $this->equalTo('bucket'), | ||
| $this->equalTo(null) | ||
| )->willReturn('awesomeBucket1'); | ||
|
|
||
| $this->config->expects($this->never()) | ||
| ->method('setUserValue'); | ||
|
|
||
| $config = $this->invokePrivate($this->provider, 'getMultiBucketObjectStoreConfig', [$this->user, $this->loader]); | ||
|
|
||
| $this->assertArrayHasKey('class', $config); | ||
| $this->assertEquals($config['class'], 'Test\Files\Mount\FakeObjectStore'); | ||
| $this->assertArrayHasKey('arguments', $config); | ||
| $this->assertArrayHasKey('user', $config['arguments']); | ||
| $this->assertSame($this->user, $config['arguments']['user']); | ||
| $this->assertArrayHasKey('objectstore', $config['arguments']); | ||
| $this->assertInstanceOf('Test\Files\Mount\FakeObjectStore', $config['arguments']['objectstore']); | ||
| $this->assertArrayHasKey('bucket', $config['arguments']); | ||
| $this->assertEquals('awesomeBucket1', $config['arguments']['bucket']); | ||
| } | ||
|
|
||
| public function testMultiBucketConfigFirst() { | ||
| $this->config->expects($this->once()) | ||
| ->method('getSystemValue') | ||
| ->with($this->equalTo('objectstore_multibucket')) | ||
| ->willReturn([ | ||
| 'class' => 'Test\Files\Mount\FakeObjectStore', | ||
| ]); | ||
|
|
||
| $this->user->method('getUID') | ||
| ->willReturn('uid'); | ||
| $this->loader->expects($this->never())->method($this->anything()); | ||
|
|
||
| $mount = $this->provider->getHomeMountForUser($this->user, $this->loader); | ||
| $this->assertInstanceOf('OC\Files\Mount\MountPoint', $mount); | ||
| } | ||
|
|
||
| public function testMultiBucketConfigFirstFallBackSingle() { | ||
| $this->config->expects($this->at(0)) | ||
| ->method('getSystemValue') | ||
| ->with($this->equalTo('objectstore_multibucket')) | ||
| ->willReturn(''); | ||
|
|
||
| $this->config->expects($this->at(1)) | ||
| ->method('getSystemValue') | ||
| ->with($this->equalTo('objectstore')) | ||
| ->willReturn([ | ||
| 'class' => 'Test\Files\Mount\FakeObjectStore', | ||
| ]); | ||
|
|
||
| $this->user->method('getUID') | ||
| ->willReturn('uid'); | ||
| $this->loader->expects($this->never())->method($this->anything()); | ||
|
|
||
| $mount = $this->provider->getHomeMountForUser($this->user, $this->loader); | ||
| $this->assertInstanceOf('OC\Files\Mount\MountPoint', $mount); | ||
| } | ||
|
|
||
| public function testNoObjectStore() { | ||
| $this->config->expects($this->exactly(2)) | ||
| ->method('getSystemValue') | ||
| ->willReturn(''); | ||
|
|
||
| $mount = $this->provider->getHomeMountForUser($this->user, $this->loader); | ||
| $this->assertNull($mount); | ||
| } | ||
| } | ||
|
|
||
| class FakeObjectStore { | ||
| private $arguments; | ||
|
|
||
| public function __construct(array $arguments) { | ||
| $this->arguments = $arguments; | ||
| } | ||
|
|
||
| public function getArguments() { | ||
| return $this->arguments; | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For swift you would call this
container. In ceph you might want to call itpool. We should stick to one terminology. I guess s3 is the most prominent, sobucketis fine. At least for our internal config.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we have to use something... I really don't care what ;)