Don't process known avatars from LDAP

* avoids useless FS operation
* avoids useless DB writes
* avoids useless addressbook updates
* addendum to #17001

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

2 files changed, 113 insertions, 2 deletions

diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index 5c89950e080..95e29689224 100644
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -584,10 +584,26 @@ class User {
 			//not set, nothing left to do;
 			return false;
 		}
+
 		if(!$this->image->loadFromBase64(base64_encode($avatarImage))) {
 			return false;
 		}
-		return $this->setOwnCloudAvatar();
+
+		// use the checksum before modifications
+		$checksum = md5($this->image->data());
+
+		if($checksum === $this->config->getUserValue($this->uid, 'user_ldap', 'lastAvatarChecksum', '')) {
+			return true;
+		}
+
+		$isSet = $this->setOwnCloudAvatar();
+
+		if($isSet) {
+			// save checksum only after successful setting
+			$this->config->setUserValue($this->uid, 'user_ldap', 'lastAvatarChecksum', $checksum);
+		}
+
+		return $isSet;
 	}
 
 	/**
@@ -599,8 +615,10 @@ class User {
 			$this->log->log('avatar image data from LDAP invalid for '.$this->dn, ILogger::ERROR);
 			return false;
 		}
+
+
 		//make sure it is a square and not bigger than 128x128
-		$size = min(array($this->image->width(), $this->image->height(), 128));
+		$size = min([$this->image->width(), $this->image->height(), 128]);
 		if(!$this->image->centerCrop($size)) {
 			$this->log->log('croping image for avatar failed for '.$this->dn, ILogger::ERROR);
 			return false;
diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php
index f99100789d8..c4563bf1f31 100644
--- a/apps/user_ldap/tests/User/UserTest.php
+++ b/apps/user_ldap/tests/User/UserTest.php
@@ -521,6 +521,17 @@ class UserTest extends \Test\TestCase {
 		$this->image->expects($this->once())
 			->method('centerCrop')
 			->will($this->returnValue(true));
+		$this->image->expects($this->once())
+			->method('data')
+			->will($this->returnValue('this is a photo'));
+
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
+			->willReturn('');
+		$this->config->expects($this->once())
+			->method('setUserValue')
+			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo'));
 
 		$this->filesystemhelper->expects($this->once())
 			->method('isLoaded')
@@ -544,6 +555,53 @@ class UserTest extends \Test\TestCase {
 		$this->user->updateAvatar();
 	}
 
+	public function testUpdateAvatarKnownJpegPhotoProvided() {
+		$this->access->expects($this->once())
+			->method('readAttribute')
+			->with($this->equalTo($this->dn),
+				$this->equalTo('jpegphoto'))
+			->will($this->returnValue(['this is a photo']));
+
+		$this->image->expects($this->once())
+			->method('loadFromBase64')
+			->willReturn('imageResource');
+		$this->image->expects($this->never())
+			->method('valid');
+		$this->image->expects($this->never())
+			->method('width');
+		$this->image->expects($this->never())
+			->method('height');
+		$this->image->expects($this->never())
+			->method('centerCrop');
+		$this->image->expects($this->once())
+			->method('data')
+			->will($this->returnValue('this is a photo'));
+
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
+			->willReturn(md5('this is a photo'));
+		$this->config->expects($this->never())
+			->method('setUserValue');
+
+		$this->filesystemhelper->expects($this->never())
+			->method('isLoaded');
+
+		$avatar = $this->createMock(IAvatar::class);
+		$avatar->expects($this->never())
+			->method('set');
+
+		$this->avatarManager->expects($this->never())
+			->method('getAvatar');
+
+		$this->connection->expects($this->any())
+			->method('resolveRule')
+			->with('avatar')
+			->willReturn(['jpegphoto', 'thumbnailphoto']);
+
+		$this->assertTrue($this->user->updateAvatar());
+	}
+
 	public function testUpdateAvatarThumbnailPhotoProvided() {
 		$this->access->expects($this->any())
 			->method('readAttribute')
@@ -575,6 +633,17 @@ class UserTest extends \Test\TestCase {
 		$this->image->expects($this->once())
 			->method('centerCrop')
 			->will($this->returnValue(true));
+		$this->image->expects($this->once())
+			->method('data')
+			->will($this->returnValue('this is a photo'));
+
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
+			->willReturn('');
+		$this->config->expects($this->once())
+			->method('setUserValue')
+			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo'));
 
 		$this->filesystemhelper->expects($this->once())
 			->method('isLoaded')
@@ -625,6 +694,13 @@ class UserTest extends \Test\TestCase {
 			->method('height');
 		$this->image->expects($this->never())
 			->method('centerCrop');
+		$this->image->expects($this->never())
+			->method('data');
+
+		$this->config->expects($this->never())
+			->method('getUserValue');
+		$this->config->expects($this->never())
+			->method('setUserValue');
 
 		$this->filesystemhelper->expects($this->never())
 			->method('isLoaded');
@@ -675,6 +751,16 @@ class UserTest extends \Test\TestCase {
 		$this->image->expects($this->once())
 			->method('centerCrop')
 			->will($this->returnValue(true));
+		$this->image->expects($this->once())
+			->method('data')
+			->will($this->returnValue('this is a photo'));
+
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
+			->willReturn('');
+		$this->config->expects($this->never())
+			->method('setUserValue');
 
 		$this->filesystemhelper->expects($this->once())
 			->method('isLoaded')
@@ -723,6 +809,13 @@ class UserTest extends \Test\TestCase {
 			->method('height');
 		$this->image->expects($this->never())
 			->method('centerCrop');
+		$this->image->expects($this->never())
+			->method('data');
+
+		$this->config->expects($this->never())
+			->method('getUserValue');
+		$this->config->expects($this->never())
+			->method('setUserValue');
 
 		$this->filesystemhelper->expects($this->never())
 			->method('isLoaded');