Merge #283 [FIX 29]After applying the patch, you should be able to execute multiple inst…

Author: memurats

apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php

@@ -26,27 +26,31 @@
  */
 namespace OCA\Files_Trashbin\BackgroundJob;
 
+use OC\Files\SetupManager;
+use OC\Files\View;
 use OCA\Files_Trashbin\Expiration;
 use OCA\Files_Trashbin\Helper;
 use OCA\Files_Trashbin\Trashbin;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\TimedJob;
 use OCP\IAppConfig;
+use OCP\IUser;
 use OCP\IUserManager;
 use Psr\Log\LoggerInterface;
 
 class ExpireTrash extends TimedJob {
+	private const THIRTY_MINUTES = 30 * 60;
 
 	public function __construct(
 		private IAppConfig $appConfig,
 		private IUserManager $userManager,
 		private Expiration $expiration,
 		private LoggerInterface $logger,
+		private SetupManager $setupManager,
 		ITimeFactory $time
 	) {
 		parent::__construct($time);
-		// Run once per 30 minutes
-		$this->setInterval(60 * 30);
+		$this->setInterval(self::THIRTY_MINUTES);
 	}
 
 	protected function run($argument) {
@@ -60,44 +64,47 @@ protected function run($argument) {
 			return;
 		}
 
-		$stopTime = time() + 60 * 30; // Stops after 30 minutes.
-		$offset = $this->appConfig->getValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
-		$users = $this->userManager->getSeenUsers($offset);
+		$stopTime = time() + self::THIRTY_MINUTES;
 
-		foreach ($users as $user) {
-			try {
+		do {
+			$this->appConfig->clearCache();
+			$offset = $this->appConfig->getValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
+			$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', $offset + 10);
+
+			$users = $this->userManager->getSeenUsers($offset, 10);
+			$count = 0;
+
+			foreach ($users as $user) {
 				$uid = $user->getUID();
-				if (!$this->setupFS($uid)) {
-					continue;
+				$count++;
+
+				try {
+					if ($this->setupFS($user)) {
+						$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
+						Trashbin::deleteExpiredFiles($dirContent, $uid);
+					}
+				} catch (\Throwable $e) {
+					$this->logger->error('Error while expiring trashbin for user ' . $uid, ['exception' => $e]);
+				} finally {
+					$this->setupManager->tearDown();
 				}
-				$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
-				Trashbin::deleteExpiredFiles($dirContent, $uid);
-			} catch (\Throwable $e) {
-				$this->logger->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]);
 			}
 
-			$offset++;
+		} while (time() < $stopTime && $count === 10);
 
-			if ($stopTime < time()) {
-				$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', $offset);
-				\OC_Util::tearDownFS();
-				return;
-			}
+		if ($count < 10) {
+			$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
 		}
-
-		$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
-		\OC_Util::tearDownFS();
 	}
 
 	/**
 	 * Act on behalf on trash item owner
 	 */
-	protected function setupFS(string $user): bool {
-		\OC_Util::tearDownFS();
-		\OC_Util::setupFS($user);
+	protected function setupFS(IUser $user): bool {
+		$this->setupManager->setupForUser($user);
 
 		// Check if this user has a trashbin directory
-		$view = new \OC\Files\View('/' . $user);
+		$view = new View('/' . $user->getUID());
 		if (!$view->is_dir('/files_trashbin/files')) {
 			return false;
 		}

lib/private/User/Manager.php

@@ -633,7 +633,7 @@ public function callForSeenUsers(\Closure $callback) {
 	}
 
 	/**
-	 * Getting all userIds that have a listLogin value requires checking the
+	 * Getting all userIds that have a lastLogin value requires checking the
 	 * value in php because on oracle you cannot use a clob in a where clause,
 	 * preventing us from doing a not null or length(value) > 0 check.
 	 *
@@ -755,19 +755,19 @@ public function getDisplayNameCache(): DisplayNameCache {
 		return $this->displayNameCache;
 	}
 
-	/**
-	 * Gets the list of users sorted by lastLogin, from most recent to least recent
-	 *
-	 * @param int $offset from which offset to fetch
-	 * @return \Iterator<IUser> list of user IDs
-	 * @since 30.0.0
-	 */
-	public function getSeenUsers(int $offset = 0): \Iterator {
-		$limit = 1000;
+	public function getSeenUsers(int $offset = 0, ?int $limit = null): \Iterator {
+		$maxBatchSize = 1000;
 
 		do {
-			$userIds = $this->getSeenUserIds($limit, $offset);
-			$offset += $limit;
+			if ($limit !== null) {
+				$batchSize = min($limit, $maxBatchSize);
+				$limit -= $batchSize;
+			} else {
+				$batchSize = $maxBatchSize;
+			}
+
+			$userIds = $this->getSeenUserIds($batchSize, $offset);
+			$offset += $batchSize;
 
 			foreach ($userIds as $userId) {
 				foreach ($this->backends as $backend) {
@@ -778,6 +778,6 @@ public function getSeenUsers(int $offset = 0): \Iterator {
 					}
 				}
 			}
-		} while (count($userIds) === $limit);
+		} while (count($userIds) === $batchSize && $limit !== 0);
 	}
 }

lib/public/IUserManager.php

@@ -239,8 +239,10 @@ public function validateUserId(string $uid, bool $checkDataDirectory = false): v
 	 * The offset argument allows the caller to continue the iteration at a specific offset.
 	 *
 	 * @param int $offset from which offset to fetch
+	 * @param int|null $limit maximum number of records to fetch
 	 * @return \Iterator<IUser> list of IUser object
 	 * @since 29.0.15
+	 * @since 29.0.16 Added the $limit argument
 	 */
-	public function getSeenUsers(int $offset = 0): \Iterator;
+	public function getSeenUsers(int $offset = 0, ?int $limit = null): \Iterator;
 }