[Pkg-owncloud-commits] [owncloud] 26/46: backport of #11494
David Prévot
taffit at moszumanska.debian.org
Fri Oct 24 15:11:43 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch master
in repository owncloud.
commit 86daab2800aacdbc26dd532f348766cccaad1e51
Author: Arthur Schiwon <blizzz at owncloud.com>
Date: Thu Sep 18 17:12:35 2014 +0200
backport of #11494
fix retrievel of group members and cache group members
fix changed variable name
with several backends, more than limit can be returned
make performance less bad. Still far from good, but at least it works
add one simple cache test
adjust group manager tests
---
apps/user_ldap/group_ldap.php | 63 +++++++++++++++++++++++++++++++------
apps/user_ldap/lib/access.php | 2 +-
apps/user_ldap/tests/group_ldap.php | 33 ++++++++++++++++---
lib/private/group/manager.php | 7 ++---
tests/lib/group/manager.php | 24 ++++++--------
5 files changed, 96 insertions(+), 33 deletions(-)
diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php
index 48d097c..e8d268d 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/apps/user_ldap/group_ldap.php
@@ -29,6 +29,16 @@ use OCA\user_ldap\lib\BackendUtility;
class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
protected $enabled = false;
+ /**
+ * @var string[] $cachedGroupMembers array of users with gid as key
+ */
+ protected $cachedGroupMembers = array();
+
+ /**
+ * @var string[] $cachedGroupsByMember array of groups with uid as key
+ */
+ protected $cachedGroupsByMember = array();
+
public function __construct(Access $access) {
parent::__construct($access);
$filter = $this->access->connection->ldapGroupFilter;
@@ -56,6 +66,21 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
}
$userDN = $this->access->username2dn($uid);
+
+ if(isset($this->cachedGroupMembers[$gid])) {
+ $isInGroup = in_array($userDN, $this->cachedGroupMembers[$gid]);
+ return $isInGroup;
+ }
+
+ $cacheKeyMembers = 'inGroup-members:'.$gid;
+ if($this->access->connection->isCached($cacheKeyMembers)) {
+ $members = $this->access->connection->getFromCache($cacheKeyMembers);
+ $this->cachedGroupMembers[$gid] = $members;
+ $isInGroup = in_array($userDN, $members);
+ $this->access->connection->writeToCache($cacheKey, $isInGroup);
+ return $isInGroup;
+ }
+
$groupDN = $this->access->groupname2dn($gid);
// just in case
if(!$groupDN || !$userDN) {
@@ -70,29 +95,44 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
}
//usually, LDAP attributes are said to be case insensitive. But there are exceptions of course.
- $members = array_keys($this->_groupMembers($groupDN));
- if(!$members) {
+ $members = $this->_groupMembers($groupDN);
+ $members = array_keys($members); // uids are returned as keys
+ if(!is_array($members) || count($members) === 0) {
$this->access->connection->writeToCache($cacheKey, false);
return false;
}
//extra work if we don't get back user DNs
- //TODO: this can be done with one LDAP query
if(strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid') {
$dns = array();
+ $filterParts = array();
+ $bytes = 0;
foreach($members as $mid) {
$filter = str_replace('%uid', $mid, $this->access->connection->ldapLoginFilter);
- $ldap_users = $this->access->fetchListOfUsers($filter, 'dn');
- if(count($ldap_users) < 1) {
- continue;
+ $filterParts[] = $filter;
+ $bytes += strlen($filter);
+ if($bytes >= 9000000) {
+ // AD has a default input buffer of 10 MB, we do not want
+ // to take even the chance to exceed it
+ $filter = $this->access->combineFilterWithOr($filterParts);
+ $bytes = 0;
+ $filterParts = array();
+ $users = $this->access->fetchListOfUsers($filter, 'dn', count($filterParts));
+ $dns = array_merge($dns, $users);
}
- $dns[] = $ldap_users[0];
+ }
+ if(count($filterParts) > 0) {
+ $filter = $this->access->combineFilterWithOr($filterParts);
+ $users = $this->access->fetchListOfUsers($filter, 'dn', count($filterParts));
+ $dns = array_merge($dns, $users);
}
$members = $dns;
}
$isInGroup = in_array($userDN, $members);
$this->access->connection->writeToCache($cacheKey, $isInGroup);
+ $this->access->connection->writeToCache($cacheKeyMembers, $members);
+ $this->cachedGroupMembers[$gid] = $members;
return $isInGroup;
}
@@ -293,8 +333,13 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
$uid = $userDN;
}
- $groups = array_values($this->getGroupsByMember($uid));
- $groups = $this->access->ownCloudGroupNames($groups);
+ if(isset($this->cachedGroupsByMember[$uid])) {
+ $groups = $this->cachedGroupsByMember[$uid];
+ } else {
+ $groups = array_values($this->getGroupsByMember($uid));
+ $groups = $this->access->ownCloudGroupNames($groups);
+ $this->cachedGroupsByMember[$uid] = $groups;
+ }
$primaryGroup = $this->getUserPrimaryGroup($userDN);
if($primaryGroup !== false) {
diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index 159b0d7..44162e3 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -1359,7 +1359,7 @@ class Access extends LDAPUtility implements user\IUserTools {
* @param string[] $bases array containing the allowed base DN or DNs
* @return bool
*/
- private function isDNPartOfBase($dn, $bases) {
+ public function isDNPartOfBase($dn, $bases) {
$belongsToBase = false;
$bases = $this->sanitizeDN($bases);
diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php
index c4aed25..d1262e4 100644
--- a/apps/user_ldap/tests/group_ldap.php
+++ b/apps/user_ldap/tests/group_ldap.php
@@ -59,10 +59,7 @@ class Test_Group_Ldap extends \PHPUnit_Framework_TestCase {
private function enableGroups($access) {
$access->connection->expects($this->any())
->method('__get')
- ->will($this->returnCallback(function($name) {
-// if($name === 'ldapLoginFilter') {
-// return '%uid';
-// }
+ ->will($this->returnCallback(function() {
return 1;
}));
}
@@ -269,4 +266,32 @@ class Test_Group_Ldap extends \PHPUnit_Framework_TestCase {
$this->assertSame(false, $gid);
}
+ /**
+ * tests whether Group Backend behaves correctly when cache with uid and gid
+ * is hit
+ */
+ public function testInGroupHitsUidGidCache() {
+ $access = $this->getAccessMock();
+ $this->enableGroups($access);
+
+ $uid = 'someUser';
+ $gid = 'someGroup';
+ $cacheKey = 'inGroup'.$uid.':'.$gid;
+ $access->connection->expects($this->once())
+ ->method('isCached')
+ ->with($cacheKey)
+ ->will($this->returnValue(true));
+
+ $access->connection->expects($this->once())
+ ->method('getFromCache')
+ ->with($cacheKey)
+ ->will($this->returnValue(true));
+
+ $access->expects($this->never())
+ ->method('username2dn');
+
+ $groupBackend = new GroupLDAP($access);
+ $groupBackend->inGroup($uid, $gid);
+ }
+
}
diff --git a/lib/private/group/manager.php b/lib/private/group/manager.php
index 816e7b4..417be79 100644
--- a/lib/private/group/manager.php
+++ b/lib/private/group/manager.php
@@ -223,10 +223,9 @@ class Manager extends PublicEmitter implements IGroupManager {
if(!empty($search)) {
// only user backends have the capability to do a complex search for users
$searchOffset = 0;
+ $searchLimit = $limit * 100;
if($limit === -1) {
- $searchLimit = $group->count('');
- } else {
- $searchLimit = $limit * 2;
+ $searchLimit = 500;
}
do {
@@ -237,7 +236,7 @@ class Manager extends PublicEmitter implements IGroupManager {
}
}
$searchOffset += $searchLimit;
- } while(count($groupUsers) < $searchLimit+$offset && count($filteredUsers) === $searchLimit);
+ } while(count($groupUsers) < $searchLimit+$offset && count($filteredUsers) >= $searchLimit);
if($limit === -1) {
$groupUsers = array_slice($groupUsers, $offset);
diff --git a/tests/lib/group/manager.php b/tests/lib/group/manager.php
index e4b3a52..8fd1951 100644
--- a/tests/lib/group/manager.php
+++ b/tests/lib/group/manager.php
@@ -369,15 +369,6 @@ class Manager extends \PHPUnit_Framework_TestCase {
}
}));
- $backend->expects($this->once())
- ->method('implementsActions')
- ->will($this->returnValue(true));
-
- $backend->expects($this->once())
- ->method('countUsersInGroup')
- ->with('testgroup', '')
- ->will($this->returnValue(2));
-
/**
* @var \OC\User\Manager $userManager
*/
@@ -496,9 +487,9 @@ class Manager extends \PHPUnit_Framework_TestCase {
->with('testgroup')
->will($this->returnValue(true));
- $backend->expects($this->any())
- ->method('InGroup')
- ->will($this->returnCallback(function($uid, $gid) {
+ $backend->expects($this->any())
+ ->method('inGroup')
+ ->will($this->returnCallback(function($uid) {
switch($uid) {
case 'user1' : return false;
case 'user2' : return true;
@@ -521,9 +512,12 @@ class Manager extends \PHPUnit_Framework_TestCase {
->with('user3')
->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) {
switch($offset) {
- case 0 : return array('user3' => new User('user3', $userBackend),
- 'user33' => new User('user33', $userBackend));
- case 2 : return array('user333' => new User('user333', $userBackend));
+ case 0 :
+ return array(
+ 'user3' => new User('user3', $userBackend),
+ 'user33' => new User('user33', $userBackend),
+ 'user333' => new User('user333', $userBackend)
+ );
}
}));
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list