Merge pull request #5213 from nextcloud/fix-change-hook

Trigger changeUser hook only on real changes

2 files changed, 102 insertions, 3 deletions

diff --git a/lib/private/User/User.php b/lib/private/User/User.php
index 5e5d3f0d772..645f7b8e59c 100644
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -165,7 +165,9 @@ class User implements IUser {
 		} else {
 			$this->config->setUserValue($this->uid, 'settings', 'email', $mailAddress);
 		}
-		$this->triggerChange('eMailAddress', $mailAddress, $oldMailAddress);
+		if($oldMailAddress !== $mailAddress) {
+			$this->triggerChange('eMailAddress', $mailAddress, $oldMailAddress);
+		}
 	}
 
 	/**
@@ -388,12 +390,15 @@ class User implements IUser {
 	 * @since 9.0.0
 	 */
 	public function setQuota($quota) {
+		$oldQuota = $this->config->getUserValue($this->uid, 'files', '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);
+		if($quota !== $oldQuota) {
+			$this->triggerChange('quota', $quota);
+		}
 	}
 
 	/**
diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php
index b53d07b0d4e..089f30a1fef 100644
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@@ -16,6 +16,7 @@ use OCP\IConfig;
 use OCP\IUser;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Notification\INotification;
+use OCP\UserInterface;
 use Test\TestCase;
 
 /**
@@ -614,7 +615,7 @@ class UserTest extends TestCase {
 
 	public function testSetEMailAddress() {
 		/**
-		 * @var Backend | \PHPUnit_Framework_MockObject_MockObject $backend
+		 * @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -649,6 +650,99 @@ class UserTest extends TestCase {
 		$user->setEMailAddress('foo@bar.com');
 	}
 
+	public function testSetEMailAddressNoChange() {
+		/**
+		 * @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend
+		 */
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+
+		/** @var PublicEmitter|\PHPUnit_Framework_MockObject_MockObject $emitter */
+		$emitter = $this->createMock(PublicEmitter::class);
+		$emitter->expects($this->never())
+			->method('emit');
+
+		$config = $this->createMock(IConfig::class);
+		$config->expects($this->any())
+			->method('getUserValue')
+			->willReturn('foo@bar.com');
+		$config->expects($this->once())
+			->method('setUserValue')
+			->with(
+				'foo',
+				'settings',
+				'email',
+				'foo@bar.com'
+			);
+
+		$user = new User('foo', $backend, $emitter, $config);
+		$user->setEMailAddress('foo@bar.com');
+	}
+
+	public function testSetQuota() {
+		/**
+		 * @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend
+		 */
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+
+		$test = $this;
+		$hooksCalled = 0;
+
+		/**
+		 * @param IUser $user
+		 * @param string $feature
+		 * @param string $value
+		 */
+		$hook = function (IUser $user, $feature, $value) use ($test, &$hooksCalled) {
+			$hooksCalled++;
+			$test->assertEquals('quota', $feature);
+			$test->assertEquals('23 TB', $value);
+		};
+
+		$emitter = new PublicEmitter();
+		$emitter->listen('\OC\User', 'changeUser', $hook);
+
+		$config = $this->createMock(IConfig::class);
+		$config->expects($this->once())
+			->method('setUserValue')
+			->with(
+				'foo',
+				'files',
+				'quota',
+				'23 TB'
+			);
+
+		$user = new User('foo', $backend, $emitter, $config);
+		$user->setQuota('23 TB');
+	}
+
+	public function testSetQuotaAddressNoChange() {
+		/**
+		 * @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend
+		 */
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+
+		/** @var PublicEmitter|\PHPUnit_Framework_MockObject_MockObject $emitter */
+		$emitter = $this->createMock(PublicEmitter::class);
+		$emitter->expects($this->never())
+			->method('emit');
+
+		$config = $this->createMock(IConfig::class);
+		$config->expects($this->any())
+			->method('getUserValue')
+			->willReturn('23 TB');
+		$config->expects($this->once())
+			->method('setUserValue')
+			->with(
+				'foo',
+				'files',
+				'quota',
+				'23 TB'
+			);
+
+		$user = new User('foo', $backend, $emitter, $config);
+		$user->setQuota('23 TB');
+	}
+
 	public function testGetLastLogin() {
 		/**
 		 * @var Backend | \PHPUnit_Framework_MockObject_MockObject $backend