diff --git a/apps/files_external/lib/config/configadapter.php b/apps/files_external/lib/config/configadapter.php index a255a7b3d2..cb8c2f24ca 100644 --- a/apps/files_external/lib/config/configadapter.php +++ b/apps/files_external/lib/config/configadapter.php @@ -114,7 +114,7 @@ class ConfigAdapter implements IMountProvider { $this->userStoragesService->setUser($user); $this->userGlobalStoragesService->setUser($user); - foreach ($this->userGlobalStoragesService->getAllStorages() as $storage) { + foreach ($this->userGlobalStoragesService->getUniqueStorages() as $storage) { try { $this->prepareStorageConfig($storage, $user); $impl = $this->constructStorage($storage); diff --git a/apps/files_external/service/userglobalstoragesservice.php b/apps/files_external/service/userglobalstoragesservice.php index c59652d057..b60473f131 100644 --- a/apps/files_external/service/userglobalstoragesservice.php +++ b/apps/files_external/service/userglobalstoragesservice.php @@ -26,6 +26,7 @@ use \OCA\Files_External\Service\BackendService; use \OCP\IUserSession; use \OCP\IGroupManager; use \OCA\Files_External\Service\UserTrait; +use \OCA\Files_External\Lib\StorageConfig; /** * Service class to read global storages applicable to the user @@ -109,4 +110,60 @@ class UserGlobalStoragesService extends GlobalStoragesService { throw new \DomainException('UserGlobalStoragesService writing disallowed'); } + /** + * Get unique storages, in case two are defined with the same mountpoint + * Higher priority storages take precedence + * + * @return StorageConfig[] + */ + public function getUniqueStorages() { + $storages = $this->getAllStorages(); + + $storagesByMountpoint = []; + foreach ($storages as $storage) { + $storagesByMountpoint[$storage->getMountPoint()][] = $storage; + } + + $result = []; + foreach ($storagesByMountpoint as $storageList) { + $storage = array_reduce($storageList, function($carry, $item) { + if (isset($carry)) { + $carryPriorityType = $this->getPriorityType($carry); + $itemPriorityType = $this->getPriorityType($item); + if ($carryPriorityType > $itemPriorityType) { + return $carry; + } elseif ($carryPriorityType === $itemPriorityType) { + if ($carry->getPriority() > $item->getPriority()) { + return $carry; + } + } + } + return $item; + }); + $result[$storage->getID()] = $storage; + } + + return $result; + } + + /** + * Get a priority 'type', where a bigger number means higher priority + * user applicable > group applicable > 'all' + * + * @param StorageConfig $storage + * @return int + */ + protected function getPriorityType(StorageConfig $storage) { + $applicableUsers = $storage->getApplicableUsers(); + $applicableGroups = $storage->getApplicableGroups(); + + if ($applicableUsers && $applicableUsers[0] !== 'all') { + return 2; + } + if ($applicableGroups) { + return 1; + } + return 0; + } + } diff --git a/apps/files_external/tests/service/userglobalstoragesservicetest.php b/apps/files_external/tests/service/userglobalstoragesservicetest.php index b9e2c08c93..867872f368 100644 --- a/apps/files_external/tests/service/userglobalstoragesservicetest.php +++ b/apps/files_external/tests/service/userglobalstoragesservicetest.php @@ -35,6 +35,7 @@ class UserGlobalStoragesServiceTest extends GlobalStoragesServiceTest { const USER_ID = 'test_user'; const GROUP_ID = 'test_group'; + const GROUP_ID2 = 'test_group2'; public function setUp() { parent::setUp(); @@ -51,8 +52,12 @@ class UserGlobalStoragesServiceTest extends GlobalStoragesServiceTest { $this->groupManager = $this->getMock('\OCP\IGroupManager'); $this->groupManager->method('isInGroup') ->will($this->returnCallback(function($userId, $groupId) { - if ($userId === self::USER_ID && $groupId === self::GROUP_ID) { - return true; + if ($userId === self::USER_ID) { + switch ($groupId) { + case self::GROUP_ID: + case self::GROUP_ID2: + return true; + } } return false; })); @@ -167,6 +172,77 @@ class UserGlobalStoragesServiceTest extends GlobalStoragesServiceTest { $this->service->removeStorage(1); } + public function getUniqueStoragesProvider() { + return [ + // 'all' vs group + [100, [], [], 100, [], [self::GROUP_ID], 2], + [100, [], [self::GROUP_ID], 100, [], [], 1], + + // 'all' vs user + [100, [], [], 100, [self::USER_ID], [], 2], + [100, [self::USER_ID], [], 100, [], [], 1], + + // group vs user + [100, [], [self::GROUP_ID], 100, [self::USER_ID], [], 2], + [100, [self::USER_ID], [], 100, [], [self::GROUP_ID], 1], + + // group+user vs group + [100, [], [self::GROUP_ID2], 100, [self::USER_ID], [self::GROUP_ID], 2], + [100, [self::USER_ID], [self::GROUP_ID], 100, [], [self::GROUP_ID2], 1], + + // user vs 'all' (higher priority) + [200, [], [], 100, [self::USER_ID], [], 2], + [100, [self::USER_ID], [], 200, [], [], 1], + + // group vs group (higher priority) + [100, [], [self::GROUP_ID2], 200, [], [self::GROUP_ID], 2], + [200, [], [self::GROUP_ID], 100, [], [self::GROUP_ID2], 1], + ]; + } + + /** + * @dataProvider getUniqueStoragesProvider + */ + public function testGetUniqueStorages( + $priority1, $applicableUsers1, $applicableGroups1, + $priority2, $applicableUsers2, $applicableGroups2, + $expectedPrecedence + ) { + $backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB'); + $authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism'); + + $storage1 = new StorageConfig(); + $storage1->setMountPoint('mountpoint'); + $storage1->setBackend($backend); + $storage1->setAuthMechanism($authMechanism); + $storage1->setBackendOptions(['password' => 'testPassword']); + $storage1->setPriority($priority1); + $storage1->setApplicableUsers($applicableUsers1); + $storage1->setApplicableGroups($applicableGroups1); + + $storage1 = $this->globalStoragesService->addStorage($storage1); + + $storage2 = new StorageConfig(); + $storage2->setMountPoint('mountpoint'); + $storage2->setBackend($backend); + $storage2->setAuthMechanism($authMechanism); + $storage2->setBackendOptions(['password' => 'testPassword']); + $storage2->setPriority($priority2); + $storage2->setApplicableUsers($applicableUsers2); + $storage2->setApplicableGroups($applicableGroups2); + + $storage2 = $this->globalStoragesService->addStorage($storage2); + + $storages = $this->service->getUniqueStorages(); + $this->assertCount(1, $storages); + + if ($expectedPrecedence === 1) { + $this->assertArrayHasKey($storage1->getID(), $storages); + } elseif ($expectedPrecedence === 2) { + $this->assertArrayHasKey($storage2->getID(), $storages); + } + } + public function testHooksAddStorage($a = null, $b = null, $c = null) { // we don't test this here $this->assertTrue(true);