Skip to content

Commit

Permalink
Cache the user backend info for 300s
Browse files Browse the repository at this point in the history 
Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
@nickvergessen
nickvergessen committed Feb 15, 2021
1 parent b418a68 commit f45570b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 34 deletions.
25 changes: 24 additions & 1 deletion lib/private/User/Manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
use OC\Hooks\PublicEmitter;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IUser;
Expand Down Expand Up @@ -84,14 +86,19 @@ class Manager extends PublicEmitter implements IUserManager {
/** @var EventDispatcherInterface */
private $dispatcher;

/** @var ICache */
private $cache;

/** @var IEventDispatcher */
private $eventDispatcher;

public function __construct(IConfig $config,
EventDispatcherInterface $oldDispatcher,
ICacheFactory $cacheFactory,
IEventDispatcher $eventDispatcher) {
$this->config = $config;
$this->dispatcher = $oldDispatcher;
$this->cache = $cacheFactory->createDistributed('user_backend_map');
$cachedUsers = &$this->cachedUsers;
$this->listen('\OC\User', 'postDelete', function ($user) use (&$cachedUsers) {
/** @var \OC\User\User $user */
Expand Down Expand Up @@ -150,8 +157,24 @@ public function get($uid) {
if (isset($this->cachedUsers[$uid])) { //check the cache first to prevent having to loop over the backends
return $this->cachedUsers[$uid];
}
foreach ($this->backends as $backend) {

$cachedBackend = $this->cache->get($uid);
if ($cachedBackend !== null && isset($this->backends[$cachedBackend])) {
// Cache has the info of the user backend already, so ask that one directly
$backend = $this->backends[$cachedBackend];
if ($backend->userExists($uid)) {
return $this->getUserObject($uid, $backend);
}
}

foreach ($this->backends as $i => $backend) {
if ($i === $cachedBackend) {
// Tried that one already
continue;
}

if ($backend->userExists($uid)) {
$this->cache->set($uid, $i, 300);
return $this->getUserObject($uid, $backend);
}
}
Expand Down
3 changes: 2 additions & 1 deletion tests/lib/Files/Config/UserMountCacheTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OC\User\Manager;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Config\ICachedMountInfo;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IUserManager;
Expand Down Expand Up @@ -46,7 +47,7 @@ class UserMountCacheTest extends TestCase {
protected function setUp(): void {
$this->fileIds = [];
$this->connection = \OC::$server->getDatabaseConnection();
$this->userManager = new Manager($this->createMock(IConfig::class), $this->createMock(EventDispatcherInterface::class), $this->createMock(IEventDispatcher::class));
$this->userManager = new Manager($this->createMock(IConfig::class), $this->createMock(EventDispatcherInterface::class), $this->createMock(ICacheFactory::class), $this->createMock(IEventDispatcher::class));
$userBackend = new Dummy();
$userBackend->createUser('u1', '');
$userBackend->createUser('u2', '');
Expand Down
62 changes: 37 additions & 25 deletions tests/lib/User/ManagerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
use OC\User\Database;
use OC\User\Manager;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IUser;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
Expand All @@ -33,18 +35,28 @@ class ManagerTest extends TestCase {
private $oldDispatcher;
/** @var IEventDispatcher */
private $eventDispatcher;
/** @var ICacheFactory */
private $cacheFactory;
/** @var ICache */
private $cache;

protected function setUp(): void {
parent::setUp();

$this->config = $this->createMock(IConfig::class);
$this->oldDispatcher = $this->createMock(EventDispatcherInterface::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->cacheFactory = $this->createMock(ICacheFactory::class);
$this->cache = $this->createMock(ICache::class);

$this->cacheFactory->method('createDistributed')
->with('user_backend_map')
->willReturn($this->cache);
}

public function testGetBackends() {
$userDummyBackend = $this->createMock(\Test\Util\User\Dummy::class);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($userDummyBackend);
$this->assertEquals([$userDummyBackend], $manager->getBackends());
$dummyDatabaseBackend = $this->createMock(Database::class);
Expand All @@ -63,7 +75,7 @@ public function testUserExistsSingleBackendExists() {
->with($this->equalTo('foo'))
->willReturn(true);

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->assertTrue($manager->userExists('foo'));
Expand All @@ -79,14 +91,14 @@ public function testUserExistsSingleBackendNotExists() {
->with($this->equalTo('foo'))
->willReturn(false);

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->assertFalse($manager->userExists('foo'));
}

public function testUserExistsNoBackends() {
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);

$this->assertFalse($manager->userExists('foo'));
}
Expand All @@ -110,7 +122,7 @@ public function testUserExistsTwoBackendsSecondExists() {
->with($this->equalTo('foo'))
->willReturn(true);

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend1);
$manager->registerBackend($backend2);

Expand All @@ -134,7 +146,7 @@ public function testUserExistsTwoBackendsFirstExists() {
$backend2->expects($this->never())
->method('userExists');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend1);
$manager->registerBackend($backend2);

Expand All @@ -161,7 +173,7 @@ public function testCheckPassword() {
}
});

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$user = $manager->checkPassword('foo', 'bar');
Expand All @@ -180,7 +192,7 @@ public function testCheckPasswordNotSupported() {
->method('implementsActions')
->willReturn(false);

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->assertFalse($manager->checkPassword('foo', 'bar'));
Expand All @@ -198,7 +210,7 @@ public function testGetOneBackendExists() {
$backend->expects($this->never())
->method('loginName2UserName');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->assertEquals('foo', $manager->get('foo')->getUID());
Expand All @@ -214,7 +226,7 @@ public function testGetOneBackendNotExists() {
->with($this->equalTo('foo'))
->willReturn(false);

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->assertEquals(null, $manager->get('foo'));
Expand All @@ -232,7 +244,7 @@ public function testGetOneBackendDoNotTranslateLoginNames() {
$backend->expects($this->never())
->method('loginName2UserName');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->assertEquals('bLeNdEr', $manager->get('bLeNdEr')->getUID());
Expand All @@ -250,7 +262,7 @@ public function testSearchOneBackend() {
$backend->expects($this->never())
->method('loginName2UserName');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$result = $manager->search('fo');
Expand Down Expand Up @@ -284,7 +296,7 @@ public function testSearchTwoBackendLimitOffset() {
$backend2->expects($this->never())
->method('loginName2UserName');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend1);
$manager->registerBackend($backend2);

Expand Down Expand Up @@ -338,7 +350,7 @@ public function testCreateUserInvalid($uid, $password, $exception) {
->willReturn(true);


$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->expectException(\InvalidArgumentException::class, $exception);
Expand All @@ -365,7 +377,7 @@ public function testCreateUserSingleBackendNotExists() {
$backend->expects($this->never())
->method('loginName2UserName');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$user = $manager->createUser('foo', 'bar');
Expand All @@ -392,7 +404,7 @@ public function testCreateUserSingleBackendExists() {
->with($this->equalTo('foo'))
->willReturn(true);

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$manager->createUser('foo', 'bar');
Expand All @@ -413,14 +425,14 @@ public function testCreateUserSingleBackendNotSupported() {
$backend->expects($this->never())
->method('userExists');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$this->assertFalse($manager->createUser('foo', 'bar'));
}

public function testCreateUserNoBackends() {
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);

$this->assertFalse($manager->createUser('foo', 'bar'));
}
Expand All @@ -440,7 +452,7 @@ public function testCreateUserFromBackendWithBackendError() {
->with('MyUid', 'MyPassword')
->willReturn(false);

$manager = new Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->createUserFromBackend('MyUid', 'MyPassword', $backend);
}

Expand Down Expand Up @@ -480,15 +492,15 @@ public function testCreateUserTwoBackendExists() {
->with($this->equalTo('foo'))
->willReturn(true);

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend1);
$manager->registerBackend($backend2);

$manager->createUser('foo', 'bar');
}

public function testCountUsersNoBackend() {
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);

$result = $manager->countUsers();
$this->assertTrue(is_array($result));
Expand All @@ -513,7 +525,7 @@ public function testCountUsersOneBackend() {
->method('getBackendName')
->willReturn('Mock_Test_Util_User_Dummy');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$result = $manager->countUsers();
Expand Down Expand Up @@ -554,7 +566,7 @@ public function testCountUsersTwoBackends() {
->method('getBackendName')
->willReturn('Mock_Test_Util_User_Dummy');

$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend1);
$manager->registerBackend($backend2);

Expand Down Expand Up @@ -655,7 +667,7 @@ public function testCallForSeenUsers() {
}

public function testDeleteUser() {
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($this->config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$backend = new \Test\Util\User\Dummy();

$manager->registerBackend($backend);
Expand Down Expand Up @@ -689,7 +701,7 @@ public function testGetByEmail() {
->with($this->equalTo('uid2'))
->willReturn(true);

$manager = new \OC\User\Manager($config, $this->oldDispatcher, $this->eventDispatcher);
$manager = new \OC\User\Manager($config, $this->oldDispatcher, $this->cacheFactory, $this->eventDispatcher);
$manager->registerBackend($backend);

$users = $manager->getByEmail('test@example.com');
Expand Down
Loading

0 comments on commit f45570b

Please sign in to comment.