[Pkg-owncloud-commits] [owncloud] 114/258: Backport of #9225

David Prévot taffit at moszumanska.debian.org
Sat Oct 11 17:22:27 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 3ebd0b898309c4ec57112b3f1210f63d82561358
Author: voxsim <Simon Vocella>
Date:   Thu Jun 26 12:38:33 2014 +0200

    Backport of #9225
    
    fix in displayNamesInGroup: when specified limit N, we did complex search only in the first N users
    
    change logic in displayNamesInGroup and add some unit tests
    
    add more logic in displayNamesInGroup for big user bases
    
    1. remove sizeof($filteredUsers) > 0 as condition
    2. use count instead of sizeof. Latter is an alias to first one, practically we stick to count everywhere. Having it consistent helps with readability.
    3. move whitespace so we have $groupUsers[] = $filteredUser; instead of $groupUsers []= $filteredUser;
---
 lib/private/group/manager.php |  40 ++++--
 tests/lib/group/manager.php   | 310 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 328 insertions(+), 22 deletions(-)

diff --git a/lib/private/group/manager.php b/lib/private/group/manager.php
index bea7ad1..58a2392 100644
--- a/lib/private/group/manager.php
+++ b/lib/private/group/manager.php
@@ -209,23 +209,41 @@ class Manager extends PublicEmitter implements IGroupManager {
 		if(is_null($group)) {
 			return array();
 		}
-		// only user backends have the capability to do a complex search for users
-		$groupUsers  = $group->searchUsers('', $limit, $offset);
+
 		$search = trim($search);
+		$groupUsers = array();
+
 		if(!empty($search)) {
-			//TODO: for OC 7 earliest: user backend should get a method to check selected users against a pattern
-			$filteredUsers = $this->userManager->search($search);
-			$testUsers = true;
+			// only user backends have the capability to do a complex search for users
+			$searchOffset = 0;
+			if($limit === -1) {
+				$searchLimit = $group->count('');
+			} else {
+				$searchLimit = $limit * 2;
+			}
+
+			do {
+				$filteredUsers = $this->userManager->search($search, $searchLimit, $searchOffset);
+				foreach($filteredUsers as $filteredUser) {
+					if($group->inGroup($filteredUser)) {
+						$groupUsers[]= $filteredUser;
+					}
+				}
+				$searchOffset += $searchLimit;
+			} while(count($groupUsers) < $searchLimit+$offset && count($filteredUsers) === $searchLimit);
+
+			if($limit === -1) {
+				$groupUsers = array_slice($groupUsers, $offset);
+			} else {
+				$groupUsers = array_slice($groupUsers, $offset, $limit);
+			}
 		} else {
-			$filteredUsers = array();
-			$testUsers = false;
+			$groupUsers = $group->searchUsers('', $limit, $offset);
 		}
 
 		$matchingUsers = array();
-		foreach($groupUsers as $user) {
-			if(!$testUsers || isset($filteredUsers[$user->getUID()])) {
-				$matchingUsers[$user->getUID()] = $user->getDisplayName();
-			}
+		foreach($groupUsers as $groupUser) {
+			$matchingUsers[$groupUser->getUID()] = $groupUser->getDisplayName();
 		}
 		return $matchingUsers;
 	}
diff --git a/tests/lib/group/manager.php b/tests/lib/group/manager.php
index 6799a59..70d9783 100644
--- a/tests/lib/group/manager.php
+++ b/tests/lib/group/manager.php
@@ -346,14 +346,80 @@ class Manager extends \PHPUnit_Framework_TestCase {
 		$this->assertEquals('group2', $group2->getGID());
 	}
 
-	public function testDisplayNamesInGroupMultipleUserBackends() {
+        public function testDisplayNamesInGroupWithOneUserBackend() {
+		/**
+		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC_Group_Backend $backend1
+		 */
+		$backend = $this->getMock('\OC_Group_Database');
+		$backend->expects($this->exactly(1))
+			->method('groupExists')
+			->with('testgroup')
+			->will($this->returnValue(true));
+
+                $backend->expects($this->any())
+			->method('InGroup')
+			->will($this->returnCallback(function($uid, $gid) {
+                                switch($uid) {
+                                        case 'user1' : return false;
+                                        case 'user2' : return true;
+                                        case 'user3' : return false;
+                                        case 'user33': return true;
+                                        default:
+                                                return null;
+                                }
+                        }));
+
+		$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
+		 */
+		$userManager = $this->getMock('\OC\User\Manager');
 		$userBackend = $this->getMock('\OC_User_Backend');
 
-		$user1 = new User('user1', $userBackend);
-		$user2 = new User('user2', $userBackend);
-		$user3 = new User('user3', $userBackend);
-		$user4 = new User('user33', $userBackend);
+		$userManager->expects($this->any())
+			->method('search')
+			->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();
+                                }
+                        }));
+
+		$userManager->expects($this->any())
+			->method('get')
+			->will($this->returnCallback(function($uid) use ($userBackend) {
+				switch($uid) {
+					case 'user1' : return new User('user1', $userBackend);
+					case 'user2' : return new User('user2', $userBackend);
+					case 'user3' : return new User('user3', $userBackend);
+					case 'user33': return new User('user33', $userBackend);
+					default:
+						return null;
+				}
+			}));
+
+		$manager = new \OC\Group\Manager($userManager);
+		$manager->addBackend($backend);
+
+		$users = $manager->displayNamesInGroup('testgroup', 'user3');
+		$this->assertEquals(1, count($users));
+		$this->assertFalse(isset($users['user1']));
+		$this->assertFalse(isset($users['user2']));
+		$this->assertFalse(isset($users['user3']));
+		$this->assertTrue(isset($users['user33']));
+	}
 
+        public function testDisplayNamesInGroupWithOneUserBackendWithLimitSpecified() {
 		/**
 		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC_Group_Backend $backend1
 		 */
@@ -363,7 +429,141 @@ class Manager extends \PHPUnit_Framework_TestCase {
 			->with('testgroup')
 			->will($this->returnValue(true));
 
-		$backend->expects($this->once())
+                $backend->expects($this->any())
+			->method('InGroup')
+			->will($this->returnCallback(function($uid, $gid) {
+                                switch($uid) {
+                                        case 'user1' : return false;
+                                        case 'user2' : return true;
+                                        case 'user3' : return false;
+                                        case 'user33': return true;
+                                        case 'user333': return true;
+                                        default:
+                                                return null;
+                                }
+                        }));
+
+		/**
+		 * @var \OC\User\Manager $userManager
+		 */
+		$userManager = $this->getMock('\OC\User\Manager');
+		$userBackend = $this->getMock('\OC_User_Backend');
+
+		$userManager->expects($this->any())
+			->method('search')
+			->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));
+                                }
+                        }));
+
+		$userManager->expects($this->any())
+			->method('get')
+			->will($this->returnCallback(function($uid) use ($userBackend) {
+				switch($uid) {
+					case 'user1' : return new User('user1', $userBackend);
+					case 'user2' : return new User('user2', $userBackend);
+					case 'user3' : return new User('user3', $userBackend);
+					case 'user33': return new User('user33', $userBackend);
+					case 'user333': return new User('user333', $userBackend);
+					default:
+						return null;
+				}
+			}));
+
+		$manager = new \OC\Group\Manager($userManager);
+		$manager->addBackend($backend);
+
+		$users = $manager->displayNamesInGroup('testgroup', 'user3', 1);
+		$this->assertEquals(1, count($users));
+		$this->assertFalse(isset($users['user1']));
+		$this->assertFalse(isset($users['user2']));
+		$this->assertFalse(isset($users['user3']));
+		$this->assertTrue(isset($users['user33']));
+		$this->assertFalse(isset($users['user333']));
+	}
+
+	public function testDisplayNamesInGroupWithOneUserBackendWithLimitAndOffsetSpecified() {
+		/**
+		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC_Group_Backend $backend1
+		 */
+		$backend = $this->getMock('\OC_Group_Database');
+		$backend->expects($this->exactly(1))
+			->method('groupExists')
+			->with('testgroup')
+			->will($this->returnValue(true));
+
+                $backend->expects($this->any())
+			->method('InGroup')
+			->will($this->returnCallback(function($uid, $gid) {
+                                switch($uid) {
+                                        case 'user1' : return false;
+                                        case 'user2' : return true;
+                                        case 'user3' : return false;
+                                        case 'user33': return true;
+                                        case 'user333': return true;
+                                        default:
+                                                return null;
+                                }
+                        }));
+
+		/**
+		 * @var \OC\User\Manager $userManager
+		 */
+		$userManager = $this->getMock('\OC\User\Manager');
+		$userBackend = $this->getMock('\OC_User_Backend');
+
+		$userManager->expects($this->any())
+			->method('search')
+			->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));
+                                }
+                        }));
+
+		$userManager->expects($this->any())
+			->method('get')
+			->will($this->returnCallback(function($uid) use ($userBackend) {
+				switch($uid) {
+					case 'user1' : return new User('user1', $userBackend);
+					case 'user2' : return new User('user2', $userBackend);
+					case 'user3' : return new User('user3', $userBackend);
+					case 'user33': return new User('user33', $userBackend);
+					case 'user333': return new User('user333', $userBackend);
+					default:
+						return null;
+				}
+			}));
+
+		$manager = new \OC\Group\Manager($userManager);
+		$manager->addBackend($backend);
+
+		$users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1);
+		$this->assertEquals(1, count($users));
+		$this->assertFalse(isset($users['user1']));
+		$this->assertFalse(isset($users['user2']));
+		$this->assertFalse(isset($users['user3']));
+		$this->assertFalse(isset($users['user33']));
+		$this->assertTrue(isset($users['user333']));
+	}
+
+	public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmpty() {
+		/**
+		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC_Group_Backend $backend1
+		 */
+		$backend = $this->getMock('\OC_Group_Database');
+		$backend->expects($this->exactly(1))
+			->method('groupExists')
+			->with('testgroup')
+			->will($this->returnValue(true));
+
+                $backend->expects($this->once())
 			->method('usersInGroup')
 			->with('testgroup', '', -1, 0)
 			->will($this->returnValue(array('user2', 'user33')));
@@ -373,10 +573,6 @@ class Manager extends \PHPUnit_Framework_TestCase {
 		 */
 		$userManager = $this->getMock('\OC\User\Manager');
 		$userBackend = $this->getMock('\OC_User_Backend');
-		$userManager->expects($this->once())
-			->method('search')
-			->with('user3')
-			->will($this->returnValue(array('user3' => $user3, 'user33' => $user4)));
 
 		$userManager->expects($this->any())
 			->method('get')
@@ -394,8 +590,100 @@ class Manager extends \PHPUnit_Framework_TestCase {
 		$manager = new \OC\Group\Manager($userManager);
 		$manager->addBackend($backend);
 
-		$users = $manager->displayNamesInGroup('testgroup', 'user3');
+		$users = $manager->displayNamesInGroup('testgroup', '');
+		$this->assertEquals(2, count($users));
+		$this->assertFalse(isset($users['user1']));
+		$this->assertTrue(isset($users['user2']));
+		$this->assertFalse(isset($users['user3']));
+		$this->assertTrue(isset($users['user33']));
+	}
+
+	public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitSpecified() {
+		/**
+		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC_Group_Backend $backend1
+		 */
+		$backend = $this->getMock('\OC_Group_Database');
+		$backend->expects($this->exactly(1))
+			->method('groupExists')
+			->with('testgroup')
+			->will($this->returnValue(true));
+
+		$backend->expects($this->once())
+			->method('usersInGroup')
+			->with('testgroup', '', 1, 0)
+			->will($this->returnValue(array('user2')));
+		/**
+		 * @var \OC\User\Manager $userManager
+		 */
+		$userManager = $this->getMock('\OC\User\Manager');
+		$userBackend = $this->getMock('\OC_User_Backend');
+
+		$userManager->expects($this->any())
+			->method('get')
+			->will($this->returnCallback(function($uid) use ($userBackend) {
+				switch($uid) {
+					case 'user1' : return new User('user1', $userBackend);
+					case 'user2' : return new User('user2', $userBackend);
+					case 'user3' : return new User('user3', $userBackend);
+					case 'user33': return new User('user33', $userBackend);
+					default:
+						return null;
+				}
+			}));
+
+		$manager = new \OC\Group\Manager($userManager);
+		$manager->addBackend($backend);
+
+		$users = $manager->displayNamesInGroup('testgroup', '', 1);
+		$this->assertEquals(1, count($users));
+		$this->assertFalse(isset($users['user1']));
+		$this->assertTrue(isset($users['user2']));
+		$this->assertFalse(isset($users['user3']));
+		$this->assertFalse(isset($users['user33']));
+	}
+
+        public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitAndOffsetSpecified() {
+		/**
+		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC_Group_Backend $backend1
+		 */
+		$backend = $this->getMock('\OC_Group_Database');
+		$backend->expects($this->exactly(1))
+			->method('groupExists')
+			->with('testgroup')
+			->will($this->returnValue(true));
+
+		$backend->expects($this->once())
+			->method('usersInGroup')
+			->with('testgroup', '', 1, 1)
+			->will($this->returnValue(array('user33')));
+
+		/**
+		 * @var \OC\User\Manager $userManager
+		 */
+		$userManager = $this->getMock('\OC\User\Manager');
+		$userBackend = $this->getMock('\OC_User_Backend');
+
+		$userManager->expects($this->any())
+			->method('get')
+			->will($this->returnCallback(function($uid) use ($userBackend) {
+				switch($uid) {
+					case 'user1' : return new User('user1', $userBackend);
+					case 'user2' : return new User('user2', $userBackend);
+					case 'user3' : return new User('user3', $userBackend);
+					case 'user33': return new User('user33', $userBackend);
+					default:
+						return null;
+				}
+			}));
+
+		$manager = new \OC\Group\Manager($userManager);
+		$manager->addBackend($backend);
+
+		$users = $manager->displayNamesInGroup('testgroup', '', 1, 1);
 		$this->assertEquals(1, count($users));
+		$this->assertFalse(isset($users['user1']));
+		$this->assertFalse(isset($users['user2']));
+		$this->assertFalse(isset($users['user3']));
 		$this->assertTrue(isset($users['user33']));
 	}
 

-- 
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