Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
Joas Schilling 2017-03-17 10:23:04 +01:00
parent 377fdf3860
commit bd97b7d130
No known key found for this signature in database
GPG Key ID: E166FD8976B3BAC8
4 changed files with 49 additions and 37 deletions

View File

@ -182,7 +182,7 @@ class MountProvider implements IMountProvider {
// such issue can usually happen when dealing with // such issue can usually happen when dealing with
// null groups which usually appear with group backend // null groups which usually appear with group backend
// caching inconsistencies // caching inconsistencies
\OC::$server->getLogger()->debug( $this->logger->debug(
'Could not adjust share target for share ' . $share->getId() . ' to make it consistent: ' . $e->getMessage(), 'Could not adjust share target for share ' . $share->getId() . ' to make it consistent: ' . $e->getMessage(),
['app' => 'files_sharing'] ['app' => 'files_sharing']
); );

View File

@ -37,7 +37,10 @@ namespace OC\Group;
use OC\Hooks\PublicEmitter; use OC\Hooks\PublicEmitter;
use OCP\GroupInterface; use OCP\GroupInterface;
use OCP\IGroup;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\ILogger;
use OCP\IUser;
/** /**
* Class Manager * Class Manager
@ -78,11 +81,16 @@ class Manager extends PublicEmitter implements IGroupManager {
/** @var \OC\SubAdmin */ /** @var \OC\SubAdmin */
private $subAdmin = null; private $subAdmin = null;
/** @var ILogger */
private $logger;
/** /**
* @param \OC\User\Manager $userManager * @param \OC\User\Manager $userManager
* @param ILogger $logger
*/ */
public function __construct(\OC\User\Manager $userManager) { public function __construct(\OC\User\Manager $userManager, ILogger $logger) {
$this->userManager = $userManager; $this->userManager = $userManager;
$this->logger = $logger;
$cachedGroups = & $this->cachedGroups; $cachedGroups = & $this->cachedGroups;
$cachedUserGroups = & $this->cachedUserGroups; $cachedUserGroups = & $this->cachedUserGroups;
$this->listen('\OC\Group', 'postDelete', function ($group) use (&$cachedGroups, &$cachedUserGroups) { $this->listen('\OC\Group', 'postDelete', function ($group) use (&$cachedGroups, &$cachedUserGroups) {
@ -186,7 +194,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return bool * @return bool
*/ */
public function groupExists($gid) { public function groupExists($gid) {
return !is_null($this->get($gid)); return $this->get($gid) instanceof IGroup;
} }
/** /**
@ -194,7 +202,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return \OC\Group\Group * @return \OC\Group\Group
*/ */
public function createGroup($gid) { public function createGroup($gid) {
if ($gid === '' || is_null($gid)) { if ($gid === '' || $gid === null) {
return false; return false;
} else if ($group = $this->get($gid)) { } else if ($group = $this->get($gid)) {
return $group; return $group;
@ -224,10 +232,10 @@ class Manager extends PublicEmitter implements IGroupManager {
$groupIds = $backend->getGroups($search, $limit, $offset); $groupIds = $backend->getGroups($search, $limit, $offset);
foreach ($groupIds as $groupId) { foreach ($groupIds as $groupId) {
$aGroup = $this->get($groupId); $aGroup = $this->get($groupId);
if (!is_null($aGroup)) { if ($aGroup instanceof IGroup) {
$groups[$groupId] = $aGroup; $groups[$groupId] = $aGroup;
} else { } else {
\OC::$server->getLogger()->debug('Group "' . $groupId . '" was returned by search but not found through direct access', array('app' => 'core')); $this->logger->debug('Group "' . $groupId . '" was returned by search but not found through direct access', ['app' => 'core']);
} }
} }
if (!is_null($limit) and $limit <= 0) { if (!is_null($limit) and $limit <= 0) {
@ -242,7 +250,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return \OC\Group\Group[] * @return \OC\Group\Group[]
*/ */
public function getUserGroups($user) { public function getUserGroups($user) {
if (is_null($user)) { if (!$user instanceof IUser) {
return []; return [];
} }
return $this->getUserIdGroups($user->getUID()); return $this->getUserIdGroups($user->getUID());
@ -262,10 +270,10 @@ class Manager extends PublicEmitter implements IGroupManager {
if (is_array($groupIds)) { if (is_array($groupIds)) {
foreach ($groupIds as $groupId) { foreach ($groupIds as $groupId) {
$aGroup = $this->get($groupId); $aGroup = $this->get($groupId);
if (!is_null($aGroup)) { if ($aGroup instanceof IGroup) {
$groups[$groupId] = $aGroup; $groups[$groupId] = $aGroup;
} else { } else {
\OC::$server->getLogger()->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', array('app' => 'core')); $this->logger->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', ['app' => 'core']);
} }
} }
} }

View File

@ -227,7 +227,7 @@ class Server extends ServerContainer implements IServerContainer {
return new \OC\User\Manager($config); return new \OC\User\Manager($config);
}); });
$this->registerService('GroupManager', function (Server $c) { $this->registerService('GroupManager', function (Server $c) {
$groupManager = new \OC\Group\Manager($this->getUserManager()); $groupManager = new \OC\Group\Manager($this->getUserManager(), $this->getLogger());
$groupManager->listen('\OC\Group', 'preCreate', function ($gid) { $groupManager->listen('\OC\Group', 'preCreate', function ($gid) {
\OC_Hook::emit('OC_Group', 'pre_createGroup', array('run' => true, 'gid' => $gid)); \OC_Hook::emit('OC_Group', 'pre_createGroup', array('run' => true, 'gid' => $gid));
}); });

View File

@ -23,17 +23,21 @@
namespace Test\Group; namespace Test\Group;
use OC\User\Manager; use OC\User\Manager;
use OCP\ILogger;
use OCP\IUser; use OCP\IUser;
use OCP\GroupInterface; use OCP\GroupInterface;
class ManagerTest extends \Test\TestCase { class ManagerTest extends \Test\TestCase {
/** @var Manager|\PHPUnit_Framework_MockObject_MockObject $userManager */ /** @var Manager|\PHPUnit_Framework_MockObject_MockObject $userManager */
protected $userManager; protected $userManager;
/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject $userManager */
protected $logger;
protected function setUp() { protected function setUp() {
parent::setUp(); parent::setUp();
$this->userManager = $this->createMock(Manager::class); $this->userManager = $this->createMock(Manager::class);
$this->logger = $this->createMock(ILogger::class);
} }
private function getTestUser($userId) { private function getTestUser($userId) {
@ -91,7 +95,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->get('group1'); $group = $manager->get('group1');
@ -100,7 +104,7 @@ class ManagerTest extends \Test\TestCase {
} }
public function testGetNoBackend() { public function testGetNoBackend() {
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$this->assertNull($manager->get('group1')); $this->assertNull($manager->get('group1'));
} }
@ -115,7 +119,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(false)); ->will($this->returnValue(false));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertNull($manager->get('group1')); $this->assertNull($manager->get('group1'));
@ -125,7 +129,7 @@ class ManagerTest extends \Test\TestCase {
$backend = new \Test\Util\Group\Dummy(); $backend = new \Test\Util\Group\Dummy();
$backend->createGroup('group1'); $backend->createGroup('group1');
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->get('group1'); $group = $manager->get('group1');
@ -152,7 +156,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
@ -179,7 +183,7 @@ class ManagerTest extends \Test\TestCase {
$backendGroupCreated = true; $backendGroupCreated = true;
}));; }));;
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->createGroup('group1'); $group = $manager->createGroup('group1');
@ -198,7 +202,7 @@ class ManagerTest extends \Test\TestCase {
$backend->expects($this->never()) $backend->expects($this->never())
->method('createGroup'); ->method('createGroup');
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->createGroup('group1'); $group = $manager->createGroup('group1');
@ -219,7 +223,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$groups = $manager->search('1'); $groups = $manager->search('1');
@ -253,7 +257,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
@ -290,7 +294,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
@ -321,7 +325,7 @@ class ManagerTest extends \Test\TestCase {
*/ */
$userManager = $this->createMock('\OC\User\Manager'); $userManager = $this->createMock('\OC\User\Manager');
$manager = new \OC\Group\Manager($userManager); $manager = new \OC\Group\Manager($userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$groups = $manager->search('1'); $groups = $manager->search('1');
@ -342,7 +346,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$groups = $manager->getUserGroups($this->getTestUser('user1')); $groups = $manager->getUserGroups($this->getTestUser('user1'));
@ -396,7 +400,7 @@ class ManagerTest extends \Test\TestCase {
*/ */
$userManager = $this->createMock('\OC\User\Manager'); $userManager = $this->createMock('\OC\User\Manager');
$userBackend = $this->createMock('\OC_User_Backend'); $userBackend = $this->createMock('\OC_User_Backend');
$manager = new \OC\Group\Manager($userManager); $manager = new \OC\Group\Manager($userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
/** @var \OC\User\User $user */ /** @var \OC\User\User $user */
@ -420,7 +424,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertTrue($manager->isInGroup('user1', 'group1')); $this->assertTrue($manager->isInGroup('user1', 'group1'));
@ -439,7 +443,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertTrue($manager->isAdmin('user1')); $this->assertTrue($manager->isAdmin('user1'));
@ -458,7 +462,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertFalse($manager->isAdmin('user1')); $this->assertFalse($manager->isAdmin('user1'));
@ -489,7 +493,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
@ -552,7 +556,7 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3'); $users = $manager->displayNamesInGroup('testgroup', 'user3');
@ -616,7 +620,7 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3', 1); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1);
@ -684,7 +688,7 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1);
@ -728,7 +732,7 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', ''); $users = $manager->displayNamesInGroup('testgroup', '');
@ -771,7 +775,7 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', '', 1); $users = $manager->displayNamesInGroup('testgroup', '', 1);
@ -814,7 +818,7 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', '', 1, 1); $users = $manager->displayNamesInGroup('testgroup', '', 1, 1);
@ -842,7 +846,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
// prime cache // prime cache
@ -885,7 +889,7 @@ class ManagerTest extends \Test\TestCase {
->method('removeFromGroup') ->method('removeFromGroup')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
// prime cache // prime cache
@ -915,7 +919,7 @@ class ManagerTest extends \Test\TestCase {
->with('user1') ->with('user1')
->will($this->returnValue(null)); ->will($this->returnValue(null));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$groups = $manager->getUserIdGroups('user1'); $groups = $manager->getUserIdGroups('user1');
@ -941,7 +945,7 @@ class ManagerTest extends \Test\TestCase {
['group2', ['gid' => 'group2']], ['group2', ['gid' => 'group2']],
])); ]));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
// group with display name // group with display name