Merge pull request #22583 from nextcloud/fix/noid/remove-unused-code

LDAP: remove unused methods and DB values

6 files changed, 69 insertions, 133 deletions

diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml
index 7a44f7f654c..145fb871a8c 100644
--- a/apps/user_ldap/appinfo/info.xml
+++ b/apps/user_ldap/appinfo/info.xml
@@ -9,7 +9,7 @@
 A user logs into Nextcloud with their LDAP or AD credentials, and is granted access based on an authentication request handled by the LDAP or AD server. Nextcloud does not store LDAP or AD passwords, rather these credentials are used to authenticate a user and then Nextcloud uses a session for the user ID. More information is available in the LDAP User and Group Backend documentation.
 
 	</description>
-	<version>1.10.1</version>
+	<version>1.10.2</version>
 	<licence>agpl</licence>
 	<author>Dominik Schmidt</author>
 	<author>Arthur Schiwon</author>
@@ -36,6 +36,7 @@ A user logs into Nextcloud with their LDAP or AD credentials, and is granted acc
 	<repair-steps>
 		<post-migration>
 			<step>OCA\User_LDAP\Migration\UUIDFixInsert</step>
+			<step>OCA\User_LDAP\Migration\RemoveRefreshTime</step>
 		</post-migration>
 	</repair-steps>
 
diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php
index f6d9777ab00..465c577f869 100644
--- a/apps/user_ldap/composer/composer/autoload_classmap.php
+++ b/apps/user_ldap/composer/composer/autoload_classmap.php
@@ -50,6 +50,7 @@ return array(
     'OCA\\User_LDAP\\Mapping\\AbstractMapping' => $baseDir . '/../lib/Mapping/AbstractMapping.php',
     'OCA\\User_LDAP\\Mapping\\GroupMapping' => $baseDir . '/../lib/Mapping/GroupMapping.php',
     'OCA\\User_LDAP\\Mapping\\UserMapping' => $baseDir . '/../lib/Mapping/UserMapping.php',
+    'OCA\\User_LDAP\\Migration\\RemoveRefreshTime' => $baseDir . '/../lib/Migration/RemoveRefreshTime.php',
     'OCA\\User_LDAP\\Migration\\UUIDFix' => $baseDir . '/../lib/Migration/UUIDFix.php',
     'OCA\\User_LDAP\\Migration\\UUIDFixGroup' => $baseDir . '/../lib/Migration/UUIDFixGroup.php',
     'OCA\\User_LDAP\\Migration\\UUIDFixInsert' => $baseDir . '/../lib/Migration/UUIDFixInsert.php',
diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php
index c3d17e86881..4760607713c 100644
--- a/apps/user_ldap/composer/composer/autoload_static.php
+++ b/apps/user_ldap/composer/composer/autoload_static.php
@@ -65,6 +65,7 @@ class ComposerStaticInitUser_LDAP
         'OCA\\User_LDAP\\Mapping\\AbstractMapping' => __DIR__ . '/..' . '/../lib/Mapping/AbstractMapping.php',
         'OCA\\User_LDAP\\Mapping\\GroupMapping' => __DIR__ . '/..' . '/../lib/Mapping/GroupMapping.php',
         'OCA\\User_LDAP\\Mapping\\UserMapping' => __DIR__ . '/..' . '/../lib/Mapping/UserMapping.php',
+        'OCA\\User_LDAP\\Migration\\RemoveRefreshTime' => __DIR__ . '/..' . '/../lib/Migration/RemoveRefreshTime.php',
         'OCA\\User_LDAP\\Migration\\UUIDFix' => __DIR__ . '/..' . '/../lib/Migration/UUIDFix.php',
         'OCA\\User_LDAP\\Migration\\UUIDFixGroup' => __DIR__ . '/..' . '/../lib/Migration/UUIDFixGroup.php',
         'OCA\\User_LDAP\\Migration\\UUIDFixInsert' => __DIR__ . '/..' . '/../lib/Migration/UUIDFixInsert.php',
diff --git a/apps/user_ldap/lib/Migration/RemoveRefreshTime.php b/apps/user_ldap/lib/Migration/RemoveRefreshTime.php
new file mode 100644
index 00000000000..b4b4e2c2628
--- /dev/null
+++ b/apps/user_ldap/lib/Migration/RemoveRefreshTime.php
@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020 Arthur Schiwon <blizzz@arthur-schiwon.de>
+ *
+ * @author Arthur Schiwon <blizzz@arthur-schiwon.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_LDAP\Migration;
+
+use OCP\IConfig;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+/**
+ * Class RmRefreshTime
+ *
+ * this can be removed with Nextcloud 21
+ *
+ * @package OCA\User_LDAP\Migration
+ */
+class RemoveRefreshTime implements IRepairStep {
+
+	/** @var IDBConnection */
+	private $dbc;
+	/** @var IConfig */
+	private $config;
+
+	public function __construct(IDBConnection $dbc, IConfig $config) {
+		$this->dbc = $dbc;
+		$this->config = $config;
+	}
+
+	public function getName() {
+		return 'Remove deprecated refresh time markers for LDAP user records';
+	}
+
+	public function run(IOutput $output) {
+		$this->config->deleteAppValue('user_ldap', 'updateAttributesInterval');
+
+		$qb = $this->dbc->getQueryBuilder();
+		$qb->delete('preferences')
+			->where($qb->expr()->eq('appid', $qb->createNamedParameter('user_ldap')))
+			->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('lastFeatureRefresh')))
+			->execute();
+	}
+}
diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index 4ec7b27017b..6117f34a0c6 100644
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -108,7 +108,6 @@ class User {
 	 * DB config keys for user preferences
 	 */
 	public const USER_PREFKEY_FIRSTLOGIN  = 'firstLoginAccomplished';
-	public const USER_PREFKEY_LASTREFRESH = 'lastFeatureRefresh';
 
 	/**
 	 * @brief constructor, make sure the subclasses call this one!
@@ -151,32 +150,6 @@ class User {
 	}
 
 	/**
-	 * @brief updates properties like email, quota or avatar provided by LDAP
-	 * @return null
-	 */
-	public function update() {
-		if (is_null($this->dn)) {
-			return null;
-		}
-
-		$hasLoggedIn = $this->config->getUserValue($this->uid, 'user_ldap',
-				self::USER_PREFKEY_FIRSTLOGIN, 0);
-
-		if ($this->needsRefresh()) {
-			$this->updateEmail();
-			$this->updateQuota();
-			if ($hasLoggedIn !== 0) {
-				//we do not need to try it, when the user has not been logged in
-				//before, because the file system will not be ready.
-				$this->updateAvatar();
-				//in order to get an avatar as soon as possible, mark the user
-				//as refreshed only when updating the avatar did happen
-				$this->markRefreshTime();
-			}
-		}
-	}
-
-	/**
 	 * marks a user as deleted
 	 *
 	 * @throws \OCP\PreConditionNotMetException
@@ -196,7 +169,6 @@ class User {
 	 * @param array $ldapEntry the user entry as retrieved from LDAP
 	 */
 	public function processAttributes($ldapEntry) {
-		$this->markRefreshTime();
 		//Quota
 		$attr = strtolower($this->connection->ldapQuotaAttribute);
 		if (isset($ldapEntry[$attr])) {
@@ -397,31 +369,6 @@ class User {
 	}
 
 	/**
-	 * @brief marks the time when user features like email have been updated
-	 * @return null
-	 */
-	public function markRefreshTime() {
-		$this->config->setUserValue(
-			$this->uid, 'user_ldap', self::USER_PREFKEY_LASTREFRESH, time());
-	}
-
-	/**
-	 * @brief checks whether user features needs to be updated again by
-	 * comparing the difference of time of the last refresh to now with the
-	 * desired interval
-	 * @return bool
-	 */
-	private function needsRefresh() {
-		$lastChecked = $this->config->getUserValue($this->uid, 'user_ldap',
-			self::USER_PREFKEY_LASTREFRESH, 0);
-
-		if ((time() - (int)$lastChecked) < (int)$this->config->getAppValue('user_ldap', 'updateAttributesInterval', 86400)) {
-			return false;
-		}
-		return  true;
-	}
-
-	/**
 	 * Stores a key-value pair in relation to this user
 	 *
 	 * @param string $key
diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php
index 12991835daf..6ad69b500e7 100644
--- a/apps/user_ldap/tests/User/UserTest.php
+++ b/apps/user_ldap/tests/User/UserTest.php
@@ -824,57 +824,6 @@ class UserTest extends \Test\TestCase {
 		$this->user->updateAvatar();
 	}
 
-	public function testUpdateBeforeFirstLogin() {
-		$this->config->expects($this->at(0))
-			->method('getUserValue')
-			->with($this->equalTo($this->uid), $this->equalTo('user_ldap'),
-				$this->equalTo(User::USER_PREFKEY_FIRSTLOGIN),
-				$this->equalTo(0))
-			->willReturn(0);
-		$this->config->expects($this->at(1))
-			->method('getUserValue')
-			->with($this->equalTo($this->uid), $this->equalTo('user_ldap'),
-				$this->equalTo(User::USER_PREFKEY_LASTREFRESH),
-				$this->equalTo(0))
-			->willReturn(0);
-		$this->config->expects($this->exactly(2))
-			->method('getUserValue');
-		$this->config->expects($this->never())
-			->method('setUserValue');
-
-		$this->user->update();
-	}
-
-	public function testUpdateAfterFirstLogin() {
-		$this->config->expects($this->at(0))
-			->method('getUserValue')
-			->with($this->equalTo($this->uid), $this->equalTo('user_ldap'),
-				$this->equalTo(User::USER_PREFKEY_FIRSTLOGIN),
-				$this->equalTo(0))
-			->willReturn(1);
-		$this->config->expects($this->at(1))
-			->method('getUserValue')
-			->with($this->equalTo($this->uid), $this->equalTo('user_ldap'),
-				$this->equalTo(User::USER_PREFKEY_LASTREFRESH),
-				$this->equalTo(0))
-			->willReturn(0);
-		$this->config->expects($this->exactly(2))
-			->method('getUserValue');
-		$this->config->expects($this->once())
-			->method('setUserValue')
-			->with($this->equalTo($this->uid), $this->equalTo('user_ldap'),
-				$this->equalTo(User::USER_PREFKEY_LASTREFRESH),
-				$this->anything())
-			->willReturn(true);
-
-		$this->connection->expects($this->any())
-			->method('resolveRule')
-			->with('avatar')
-			->willReturn(['jpegphoto', 'thumbnailphoto']);
-
-		$this->user->update();
-	}
-
 	public function extStorageHomeDataProvider() {
 		return [
 			[ 'myFolder', null ],
@@ -918,33 +867,6 @@ class UserTest extends \Test\TestCase {
 		$this->assertSame($expected, $actual);
 	}
 
-	public function testUpdateNoRefresh() {
-		$this->config->expects($this->at(0))
-			->method('getUserValue')
-			->with($this->equalTo($this->uid), $this->equalTo('user_ldap'),
-				$this->equalTo(User::USER_PREFKEY_FIRSTLOGIN),
-				$this->equalTo(0))
-			->willReturn(1);
-		$this->config->expects($this->at(1))
-			->method('getUserValue')
-			->with($this->equalTo($this->uid), $this->equalTo('user_ldap'),
-				$this->equalTo(User::USER_PREFKEY_LASTREFRESH),
-				$this->equalTo(0))
-			->willReturn(time() - 10);
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with($this->equalTo('user_ldap'),
-				$this->equalTo('updateAttributesInterval'),
-				$this->anything())
-			->willReturn(1800);
-		$this->config->expects($this->exactly(2))
-			->method('getUserValue');
-		$this->config->expects($this->never())
-			->method('setUserValue');
-
-		$this->user->update();
-	}
-
 	public function testMarkLogin() {
 		$this->config->expects($this->once())
 			->method('setUserValue')
@@ -997,7 +919,6 @@ class UserTest extends \Test\TestCase {
 
 	public function testProcessAttributes() {
 		$requiredMethods = [
-			'markRefreshTime',
 			'updateQuota',
 			'updateEmail',
 			'composeAndStoreDisplayName',