Merge pull request #22252 from owncloud/consolidate-user-set-quota

Consolidate getQuota and setQuota methods in User instance

apps/files_trashbin/lib/trashbin.php

@@ -564,11 +564,8 @@ class Trashbin {
 		$config = \OC::$server->getConfig();
 
 		$softQuota = true;
-		$quota = $config->getUserValue($user, 'files', 'quota', null);
+		$quota = \OC::$server->getUserManager()->get($user)->getQuota();
 		$view = new \OC\Files\View('/' . $user);
-		if ($quota === null || $quota === 'default') {
-			$quota = $config->getAppValue('files', 'default_quota', null);
-		}
 		if ($quota === null || $quota === 'none') {
 			$quota = \OC\Files\Filesystem::free_space('/');
 			$softQuota = false;

apps/files_versions/lib/storage.php

@@ -653,11 +653,9 @@ class Storage {
 			$versionsFileview = new \OC\Files\View('/'.$uid.'/files_versions');
 
 			// get available disk space for user
+			$user = \OC::$server->getUserManager()->get($uid);
 			$softQuota = true;
-			$quota = $config->getUserValue($uid, 'files', 'quota', null);
-			if ( $quota === null || $quota === 'default') {
-				$quota = $config->getAppValue('files', 'default_quota', null);
-			}
+			$quota = $user->getQuota();
 			if ( $quota === null || $quota === 'none' ) {
 				$quota = \OC\Files\Filesystem::free_space('/');
 				$softQuota = false;

apps/provisioning_api/lib/users.php

@@ -278,7 +278,7 @@ class Users {
 						$quota = \OCP\Util::humanFileSize($quota);
 					}
 				}
-				$this->config->setUserValue($targetUserId, 'files', 'quota', $quota);
+				$targetUser->setQuota($quota);
 				break;
 			case 'password':
 				$targetUser->setPassword($parameters['_put']['value']);

apps/provisioning_api/tests/userstest.php

@@ -1015,6 +1015,9 @@ class UsersTest extends OriginalTest {
 			->method('getUID')
 			->will($this->returnValue('UserToEdit'));
 		$targetUser = $this->getMock('\OCP\IUser');
+		$targetUser->expects($this->once())
+			->method('setQuota')
+			->with('2.9 MB');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1029,10 +1032,6 @@ class UsersTest extends OriginalTest {
 			->method('isAdmin')
 			->with('UserToEdit')
 			->will($this->returnValue(true));
-		$this->config
-			->expects($this->once())
-			->method('setUserValue')
-			->with('UserToEdit', 'files', 'quota', '2.9 MB');
 
 		$expected = new \OC_OCS_Result(null, 100);
 		$this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => '3042824']]));
@@ -1071,6 +1070,9 @@ class UsersTest extends OriginalTest {
 			->method('getUID')
 			->will($this->returnValue('admin'));
 		$targetUser = $this->getMock('\OCP\IUser');
+		$targetUser->expects($this->once())
+			->method('setQuota')
+			->with('2.9 MB');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1092,10 +1094,6 @@ class UsersTest extends OriginalTest {
 			->expects($this->once())
 			->method('getSubAdmin')
 			->will($this->returnValue($subAdminManager));
-		$this->config
-			->expects($this->once())
-			->method('setUserValue')
-			->with('UserToEdit', 'files', 'quota', '2.9 MB');
 
 		$expected = new \OC_OCS_Result(null, 100);
 		$this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => '3042824']]));
@@ -1108,6 +1106,9 @@ class UsersTest extends OriginalTest {
 			->method('getUID')
 			->will($this->returnValue('subadmin'));
 		$targetUser = $this->getMock('\OCP\IUser');
+		$targetUser->expects($this->once())
+			->method('setQuota')
+			->with('2.9 MB');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1129,10 +1130,6 @@ class UsersTest extends OriginalTest {
 			->expects($this->once())
 			->method('getSubAdmin')
 			->will($this->returnValue($subAdminManager));
-		$this->config
-			->expects($this->once())
-			->method('setUserValue')
-			->with('UserToEdit', 'files', 'quota', '2.9 MB');
 
 		$expected = new \OC_OCS_Result(null, 100);
 		$this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => '3042824']]));

apps/user_ldap/lib/user/user.php

@@ -456,7 +456,7 @@ class User {
 			}
 		}
 		if(!is_null($quota)) {
-			$this->config->setUserValue($this->uid, 'files', 'quota', $quota);
+			$user = $this->userManager->get($this->uid)->setQuota($quota);
 		}
 	}
 

apps/user_ldap/tests/user/user.php

@@ -210,13 +210,15 @@ class Test_User_User extends \Test\TestCase {
 				$this->equalTo('myquota'))
 			->will($this->returnValue(array('42 GB')));
 
-		$config->expects($this->once())
-			->method('setUserValue')
-			->with($this->equalTo('alice'),
-				$this->equalTo('files'),
-				$this->equalTo('quota'),
-				$this->equalTo('42 GB'))
-			->will($this->returnValue(true));
+		$user = $this->getMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('42 GB');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
 
 		$uid = 'alice';
 		$dn  = 'uid=alice,dc=foo,dc=bar';
@@ -253,13 +255,15 @@ class Test_User_User extends \Test\TestCase {
 				$this->equalTo('myquota'))
 			->will($this->returnValue(false));
 
-		$config->expects($this->once())
-			->method('setUserValue')
-			->with($this->equalTo('alice'),
-				$this->equalTo('files'),
-				$this->equalTo('quota'),
-				$this->equalTo('25 GB'))
-			->will($this->returnValue(true));
+		$user = $this->getMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('25 GB');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
 
 		$uid = 'alice';
 		$dn  = 'uid=alice,dc=foo,dc=bar';
@@ -296,13 +300,15 @@ class Test_User_User extends \Test\TestCase {
 				$this->equalTo('myquota'))
 			->will($this->returnValue(array('27 GB')));
 
-		$config->expects($this->once())
-			->method('setUserValue')
-			->with($this->equalTo('alice'),
-				$this->equalTo('files'),
-				$this->equalTo('quota'),
-				$this->equalTo('27 GB'))
-			->will($this->returnValue(true));
+		$user = $this->getMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('27 GB');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
 
 		$uid = 'alice';
 		$dn  = 'uid=alice,dc=foo,dc=bar';
@@ -408,13 +414,15 @@ class Test_User_User extends \Test\TestCase {
 		$access->expects($this->never())
 			->method('readAttribute');
 
-		$config->expects($this->once())
-			->method('setUserValue')
-			->with($this->equalTo('alice'),
-				$this->equalTo('files'),
-				$this->equalTo('quota'),
-				$this->equalTo($readQuota))
-			->will($this->returnValue(true));
+		$user = $this->getMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with($readQuota);
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
 
 		$uid = 'alice';
 		$dn  = 'uid=alice,dc=foo,dc=bar';

lib/private/avatar.php

@@ -121,7 +121,7 @@ class Avatar implements IAvatar {
 
 		$this->remove();
 		$this->folder->newFile('avatar.'.$type)->putContent($data);
-		$this->user->triggerChange();
+		$this->user->triggerChange('avatar');
 	}
 
 	/**
@@ -137,7 +137,7 @@ class Avatar implements IAvatar {
 				$avatar->delete();
 			}
 		}
-		$this->user->triggerChange();
+		$this->user->triggerChange('avatar');
 	}
 
 	/**

lib/private/server.php

@@ -244,9 +244,9 @@ class Server extends ServerContainer implements IServerContainer {
 			$userSession->listen('\OC\User', 'logout', function () {
 				\OC_Hook::emit('OC_User', 'logout', array());
 			});
-			$userSession->listen('\OC\User', 'changeUser', function ($user) {
+			$userSession->listen('\OC\User', 'changeUser', function ($user, $feature, $value) {
 				/** @var $user \OC\User\User */
-				\OC_Hook::emit('OC_User', 'changeUser', array('run' => true, 'user' => $user));
+				\OC_Hook::emit('OC_User', 'changeUser', array('run' => true, 'user' => $user, 'feature' => $feature, 'value' => $value));
 			});
 			return $userSession;
 		});

lib/private/user/user.php

@@ -30,6 +30,7 @@
 namespace OC\User;
 
 use OC\Hooks\Emitter;
+use OC_Helper;
 use OCP\IAvatarManager;
 use OCP\IImage;
 use OCP\IURLGenerator;
@@ -140,7 +141,7 @@ class User implements IUser {
 			$result = $this->backend->setDisplayName($this->uid, $displayName);
 			if ($result) {
 				$this->displayName = $displayName;
-				$this->triggerChange();
+				$this->triggerChange('displayName', $displayName);
 			}
 			return $result !== false;
 		} else {
@@ -161,7 +162,7 @@ class User implements IUser {
 		} else {
 			$this->config->setUserValue($this->uid, 'settings', 'email', $mailAddress);
 		}
-		$this->triggerChange();
+		$this->triggerChange('eMailAddress', $mailAddress);
 	}
 
 	/**
@@ -338,6 +339,36 @@ class User implements IUser {
 		return $this->config->getUserValue($this->uid, 'settings', 'email', null);
 	}
 
+	/**
+	 * get the users' quota
+	 *
+	 * @return string
+	 * @since 9.0.0
+	 */
+	public function getQuota() {
+		$quota = $this->config->getUserValue($this->uid, 'files', 'quota', 'default');
+		if($quota === 'default') {
+			$quota = $this->config->getAppValue('files', 'default_quota', 'none');
+		}
+		return $quota;
+	}
+
+	/**
+	 * set the users' quota
+	 *
+	 * @param string $quota
+	 * @return void
+	 * @since 9.0.0
+	 */
+	public function setQuota($quota) {
+		if($quota !== 'none' and $quota !== 'default') {
+			$quota = OC_Helper::computerFileSize($quota);
+			$quota = OC_Helper::humanFileSize($quota);
+		}
+		$this->config->setUserValue($this->uid, 'files', 'quota', $quota);
+		$this->triggerChange('quota', $quota);
+	}
+
 	/**
 	 * get the avatar image if it exists
 	 *
@@ -386,9 +417,9 @@ class User implements IUser {
 		return $url;
 	}
 
-	public function triggerChange() {
+	public function triggerChange($feature, $value = null) {
 		if ($this->emitter) {
-			$this->emitter->emit('\OC\User', 'changeUser', array($this));
+			$this->emitter->emit('\OC\User', 'changeUser', array($this, $feature, $value));
 		}
 	}
 

lib/private/util.php

@@ -285,11 +285,7 @@ class OC_Util {
 	 * @return int Quota bytes
 	 */
 	public static function getUserQuota($user) {
-		$config = \OC::$server->getConfig();
-		$userQuota = $config->getUserValue($user, 'files', 'quota', 'default');
-		if ($userQuota === 'default') {
-			$userQuota = $config->getAppValue('files', 'default_quota', 'none');
-		}
+		$userQuota = \OC::$server->getUserManager()->get($user)->getQuota();
 		if($userQuota === 'none') {
 			return \OCP\Files\FileInfo::SPACE_UNLIMITED;
 		}else{

lib/public/iuser.php

@@ -178,4 +178,23 @@ interface IUser {
 	 * @since 9.0.0
 	 */
 	public function setEMailAddress($mailAddress);
+
+	/**
+	 * get the users' quota in human readable form. If a specific quota is not
+	 * set for the user, the default value is returned. If a default setting
+	 * was not set otherwise, it is return as 'none', i.e. quota is not limited.
+	 *
+	 * @return string
+	 * @since 9.0.0
+	 */
+	public function getQuota();
+
+	/**
+	 * set the users' quota
+	 *
+	 * @param string $quota
+	 * @return void
+	 * @since 9.0.0
+	 */
+	public function setQuota($quota);
 }

settings/ajax/setquota.php

@@ -56,7 +56,7 @@ if($quota !== 'none' and $quota !== 'default') {
 
 // Return Success story
 if($username) {
-	\OC::$server->getConfig()->setUserValue($username, 'files', 'quota', $quota);
+	$targetUserObject->setQuota($quota);
 }else{//set the default quota when no username is specified
 	if($quota === 'default') {//'default' as default quota makes no sense
 		$quota='none';

settings/controller/userscontroller.php

@@ -184,7 +184,7 @@ class UsersController extends Controller {
 			'displayname' => $user->getDisplayName(),
 			'groups' => (empty($userGroups)) ? $this->groupManager->getUserGroupIds($user) : $userGroups,
 			'subadmin' => $subAdminGroups,
-			'quota' => $this->config->getUserValue($user->getUID(), 'files', 'quota', 'default'),
+			'quota' => $user->getQuota(),
 			'storageLocation' => $user->getHome(),
 			'lastLogin' => $user->getLastLogin() * 1000,
 			'backend' => $user->getBackendClassName(),

tests/settings/controller/userscontrollertest.php

@@ -86,7 +86,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$foo = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$foo
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$foo
@@ -97,6 +97,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('foo@bar.com'));
+		$foo
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('1024'));
 		$foo
 			->method('getLastLogin')
 			->will($this->returnValue(500));
@@ -110,7 +114,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$admin = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$admin
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('admin'));
 		$admin
@@ -121,6 +125,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('admin@bar.com'));
+		$admin
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('404'));
 		$admin
 			->expects($this->once())
 			->method('getLastLogin')
@@ -136,7 +144,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$bar = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$bar
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('bar'));
 		$bar
@@ -147,6 +155,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('bar@dummy.com'));
+		$bar
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('2323'));
 		$bar
 			->method('getLastLogin')
 			->will($this->returnValue(3999));
@@ -182,12 +194,6 @@ class UsersControllerTest extends \Test\TestCase {
 			->method('get')
 			->with('bar')
 			->will($this->returnValue($bar));
-		$this->container['Config']
-			->expects($this->exactly(3))
-			->method('getUserValue')
-			->will($this->onConsecutiveCalls(1024,
-											404,
-											2323));
 
 		$subadmin = $this->getMockBuilder('\OC\SubAdmin')
 			->disableOriginalConstructor()
@@ -272,7 +278,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$foo = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$foo
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$foo
@@ -283,6 +289,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('foo@bar.com'));
+		$foo
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('1024'));
 		$foo
 			->method('getLastLogin')
 			->will($this->returnValue(500));
@@ -296,7 +306,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$admin = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$admin
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('admin'));
 		$admin
@@ -307,6 +317,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('admin@bar.com'));
+		$admin
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('404'));
 		$admin
 			->expects($this->once())
 			->method('getLastLogin')
@@ -322,7 +336,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$bar = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$bar
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('bar'));
 		$bar
@@ -333,6 +347,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('bar@dummy.com'));
+		$bar
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('2323'));
 		$bar
 			->method('getLastLogin')
 			->will($this->returnValue(3999));
@@ -377,14 +395,6 @@ class UsersControllerTest extends \Test\TestCase {
 			->method('get')
 			->with('admin')
 			->will($this->returnValue($admin));
-		$this->container['Config']
-			->expects($this->exactly(3))
-			->method('getUserValue')
-			->will($this->onConsecutiveCalls(
-				2323,
-				1024,
-				404
-			));
 
 		$subgroup1 = $this->getMockBuilder('\OCP\IGroup')
 			->disableOriginalConstructor()
@@ -472,7 +482,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$foo = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$foo
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$foo
@@ -483,6 +493,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('foo@bar.com'));
+		$foo
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('1024'));
 		$foo
 			->method('getLastLogin')
 			->will($this->returnValue(500));
@@ -496,7 +510,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$admin = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$admin
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('admin'));
 		$admin
@@ -507,6 +521,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('admin@bar.com'));
+		$admin
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('404'));
 		$admin
 			->expects($this->once())
 			->method('getLastLogin')
@@ -522,7 +540,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$bar = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$bar
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('bar'));
 		$bar
@@ -533,6 +551,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue('bar@dummy.com'));
+		$bar
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('2323'));
 		$bar
 			->method('getLastLogin')
 			->will($this->returnValue(3999));
@@ -553,10 +575,6 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->exactly(3))
 			->method('getUserGroupIds')
 			->will($this->onConsecutiveCalls(array('Users', 'Support'), array('admins', 'Support'), array('External Users')));
-		$this->container['Config']
-			->expects($this->exactly(3))
-			->method('getUserValue')
-			->will($this->onConsecutiveCalls(1024, 404, 2323));
 
 		$subadmin = $this->getMockBuilder('\OC\SubAdmin')
 			->disableOriginalConstructor()
@@ -622,7 +640,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$user = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$user
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$user
@@ -633,6 +651,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getEMailAddress')
 			->will($this->returnValue(null));
+		$user
+			->expects($this->once())
+			->method('getQuota')
+			->will($this->returnValue('none'));
 		$user
 			->method('getLastLogin')
 			->will($this->returnValue(500));
@@ -674,7 +696,7 @@ class UsersControllerTest extends \Test\TestCase {
 					'displayname' => 'M. Foo',
 					'groups' => null,
 					'subadmin' => array(),
-					'quota' => null,
+					'quota' => 'none',
 					'storageLocation' => '/home/foo',
 					'lastLogin' => 500000,
 					'backend' => 'OC_User_Database',