Merge pull request #32018 from nextcloud/cleanup/event/trashbin

Port files trashbin events to IEventDispatcher/IEventListener

apps/files/tests/Command/DeleteOrphanedFilesTest.php

@@ -9,7 +9,9 @@ namespace OCA\Files\Tests\Command;
 
 use OC\Files\View;
 use OCA\Files\Command\DeleteOrphanedFiles;
+use OCP\Files\IRootFolder;
 use OCP\Files\StorageNotAvailableException;
+use OCP\IDBConnection;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Test\TestCase;
@@ -23,25 +25,14 @@ use Test\TestCase;
  */
 class DeleteOrphanedFilesTest extends TestCase {
 
-	/**
+	private DeleteOrphanedFiles $command;
-	 * @var DeleteOrphanedFiles
+	private IDBConnection $connection;
-	 */
+	private string $user1;
-	private $command;
-
-	/**
-	 * @var \OCP\IDBConnection
-	 */
-	private $connection;
-
-	/**
-	 * @var string
-	 */
-	private $user1;
 
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->connection = \OC::$server->getDatabaseConnection();
+		$this->connection = \OCP\Server::get(IDBConnection::class);
 
 		$this->user1 = $this->getUniqueID('user1_');
 
@@ -90,12 +81,13 @@ class DeleteOrphanedFilesTest extends TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 
+		$rootFolder = \OCP\Server::get(IRootFolder::class);
+
 		// scan home storage so that mounts are properly setup
-		\OC::$server->getRootFolder()->getUserFolder($this->user1)->getStorage()->getScanner()->scan('');
+		$rootFolder->getUserFolder($this->user1)->getStorage()->getScanner()->scan('');
 
 		$this->loginAsUser($this->user1);
 
-
 		$view = new View('/' . $this->user1 . '/');
 		$view->mkdir('files/test');
 
@@ -132,6 +124,8 @@ class DeleteOrphanedFilesTest extends TestCase {
 		$this->assertCount(0, $this->getFile($fileInfo->getId()), 'Asserts that file gets cleaned up');
 		$this->assertCount(0, $this->getMounts($numericStorageId), 'Asserts that mount gets cleaned up');
 
+		// Rescan folder to add back to cache before deleting
+		$rootFolder->getUserFolder($this->user1)->getStorage()->getScanner()->scan('');
 		// since we deleted the storage it might throw a (valid) StorageNotAvailableException
 		try {
 			$view->unlink('files/test');

apps/files_trashbin/composer/composer/autoload_classmap.php

@@ -22,7 +22,7 @@ return array(
     'OCA\\Files_Trashbin\\Exceptions\\CopyRecursiveException' => $baseDir . '/../lib/Exceptions/CopyRecursiveException.php',
     'OCA\\Files_Trashbin\\Expiration' => $baseDir . '/../lib/Expiration.php',
     'OCA\\Files_Trashbin\\Helper' => $baseDir . '/../lib/Helper.php',
-    'OCA\\Files_Trashbin\\Hooks' => $baseDir . '/../lib/Hooks.php',
+    'OCA\\Files_Trashbin\\Listener\\EventListener' => $baseDir . '/../lib/Listener/EventListener.php',
     'OCA\\Files_Trashbin\\Listeners\\LoadAdditionalScripts' => $baseDir . '/../lib/Listeners/LoadAdditionalScripts.php',
     'OCA\\Files_Trashbin\\Listeners\\SyncLivePhotosListener' => $baseDir . '/../lib/Listeners/SyncLivePhotosListener.php',
     'OCA\\Files_Trashbin\\Migration\\Version1010Date20200630192639' => $baseDir . '/../lib/Migration/Version1010Date20200630192639.php',

apps/files_trashbin/composer/composer/autoload_static.php

@@ -37,7 +37,7 @@ class ComposerStaticInitFiles_Trashbin
         'OCA\\Files_Trashbin\\Exceptions\\CopyRecursiveException' => __DIR__ . '/..' . '/../lib/Exceptions/CopyRecursiveException.php',
         'OCA\\Files_Trashbin\\Expiration' => __DIR__ . '/..' . '/../lib/Expiration.php',
         'OCA\\Files_Trashbin\\Helper' => __DIR__ . '/..' . '/../lib/Helper.php',
-        'OCA\\Files_Trashbin\\Hooks' => __DIR__ . '/..' . '/../lib/Hooks.php',
+        'OCA\\Files_Trashbin\\Listener\\EventListener' => __DIR__ . '/..' . '/../lib/Listener/EventListener.php',
         'OCA\\Files_Trashbin\\Listeners\\LoadAdditionalScripts' => __DIR__ . '/..' . '/../lib/Listeners/LoadAdditionalScripts.php',
         'OCA\\Files_Trashbin\\Listeners\\SyncLivePhotosListener' => __DIR__ . '/..' . '/../lib/Listeners/SyncLivePhotosListener.php',
         'OCA\\Files_Trashbin\\Migration\\Version1010Date20200630192639' => __DIR__ . '/..' . '/../lib/Migration/Version1010Date20200630192639.php',

apps/files_trashbin/lib/AppInfo/Application.php

@@ -11,16 +11,22 @@ use OCA\Files\Event\LoadAdditionalScriptsEvent;
 use OCA\Files_Trashbin\Capabilities;
 use OCA\Files_Trashbin\Events\BeforeNodeRestoredEvent;
 use OCA\Files_Trashbin\Expiration;
+use OCA\Files_Trashbin\Listener\EventListener;
 use OCA\Files_Trashbin\Listeners\LoadAdditionalScripts;
 use OCA\Files_Trashbin\Listeners\SyncLivePhotosListener;
 use OCA\Files_Trashbin\Trash\ITrashManager;
 use OCA\Files_Trashbin\Trash\TrashManager;
+use OCA\Files_Trashbin\Trashbin;
 use OCA\Files_Trashbin\UserMigration\TrashbinMigrator;
 use OCP\App\IAppManager;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
+use OCP\Files\Events\BeforeFileSystemSetupEvent;
+use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
+use OCP\Files\Events\Node\NodeWrittenEvent;
+use OCP\User\Events\BeforeUserDeletedEvent;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
 
@@ -47,19 +53,17 @@ class Application extends App implements IBootstrap {
 		);
 
 		$context->registerEventListener(BeforeNodeRestoredEvent::class, SyncLivePhotosListener::class);
+
+		$context->registerEventListener(NodeWrittenEvent::class, EventListener::class);
+		$context->registerEventListener(BeforeUserDeletedEvent::class, EventListener::class);
+		$context->registerEventListener(BeforeFileSystemSetupEvent::class, EventListener::class);
+
+		// pre and post-rename, disable trash logic for the copy+unlink case
+		$context->registerEventListener(BeforeNodeDeletedEvent::class, Trashbin::class);
 	}
 
 	public function boot(IBootContext $context): void {
 		$context->injectFn([$this, 'registerTrashBackends']);
-
-		// create storage wrapper on setup
-		\OCP\Util::connectHook('OC_Filesystem', 'preSetup', 'OCA\Files_Trashbin\Storage', 'setupStorage');
-		//Listen to delete user signal
-		\OCP\Util::connectHook('OC_User', 'pre_deleteUser', 'OCA\Files_Trashbin\Hooks', 'deleteUser_hook');
-		//Listen to post write hook
-		\OCP\Util::connectHook('OC_Filesystem', 'post_write', 'OCA\Files_Trashbin\Hooks', 'post_write_hook');
-		// pre and post-rename, disable trash logic for the copy+unlink case
-		\OCP\Util::connectHook('OC_Filesystem', 'delete', 'OCA\Files_Trashbin\Trashbin', 'ensureFileScannedHook');
 	}
 
 	public function registerTrashBackends(ContainerInterface $serverContainer, LoggerInterface $logger, IAppManager $appManager, ITrashManager $trashManager): void {

apps/files_trashbin/lib/Hooks.php

@@ -1,29 +0,0 @@
-<?php
-/**
- * SPDX-FileCopyrightText: 2021-2024 Nextcloud GmbH and Nextcloud contributors
- * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-namespace OCA\Files_Trashbin;
-
-class Hooks {
-
-	/**
-	 * clean up user specific settings if user gets deleted
-	 * @param array $params array with uid
-	 *
-	 * This function is connected to the pre_deleteUser signal of OC_Users
-	 * to remove the used space for the trash bin stored in the database
-	 */
-	public static function deleteUser_hook($params) {
-		$uid = $params['uid'];
-		Trashbin::deleteUser($uid);
-	}
-
-	public static function post_write_hook($params) {
-		$user = \OC_User::getUser();
-		if (!empty($user)) {
-			Trashbin::resizeTrash($user);
-		}
-	}
-}

apps/files_trashbin/lib/Listener/EventListener.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCA\Files_Trashbin\Listener;
+
+use OCA\Files_Trashbin\Storage;
+use OCA\Files_Trashbin\Trashbin;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\Files\Events\BeforeFileSystemSetupEvent;
+use OCP\Files\Events\Node\NodeWrittenEvent;
+use OCP\User\Events\BeforeUserDeletedEvent;
+
+/** @template-implements IEventListener<NodeWrittenEvent|BeforeUserDeletedEvent|BeforeFileSystemSetupEvent> */
+class EventListener implements IEventListener {
+	private ?string $userId;
+
+	public function __construct(?string $userId = null) {
+		$this->userId = $userId;
+	}
+
+	public function handle(Event $event): void {
+		if ($event instanceof NodeWrittenEvent) {
+			// Resize trash
+			if (!empty($this->userId)) {
+				Trashbin::resizeTrash($this->userId);
+			}
+		}
+
+		// Clean up user specific settings if user gets deleted
+		if ($event instanceof BeforeUserDeletedEvent) {
+			Trashbin::deleteUser($event->getUser()->getUID());
+		}
+
+		if ($event instanceof BeforeFileSystemSetupEvent) {
+			Storage::setupStorage();
+		}
+	}
+}

apps/files_trashbin/lib/Storage.php

@@ -29,13 +29,7 @@ class Storage extends Wrapper {
 
 	/**
 	 * Storage constructor.
-	 *
 	 * @param array $parameters
-	 * @param ITrashManager|null $trashManager
-	 * @param IUserManager|null $userManager
-	 * @param LoggerInterface|null $logger
-	 * @param IEventDispatcher|null $eventDispatcher
-	 * @param IRootFolder|null $rootFolder
 	 */
 	public function __construct(
 		$parameters,

apps/files_trashbin/lib/Trashbin.php

@@ -11,8 +11,6 @@ use OC\Files\Cache\Cache;
 use OC\Files\Cache\CacheEntry;
 use OC\Files\Cache\CacheQueryBuilder;
 use OC\Files\Filesystem;
-use OC\Files\Node\File;
-use OC\Files\Node\Folder;
 use OC\Files\Node\NonExistingFile;
 use OC\Files\Node\NonExistingFolder;
 use OC\Files\View;
@@ -23,7 +21,12 @@ use OCA\Files_Trashbin\Events\BeforeNodeRestoredEvent;
 use OCA\Files_Trashbin\Events\NodeRestoredEvent;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\EventDispatcher\IEventListener;
+use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
+use OCP\Files\File;
+use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
 use OCP\Files\NotFoundException;
@@ -38,21 +41,20 @@ use OCP\Lock\LockedException;
 use OCP\Server;
 use Psr\Log\LoggerInterface;
 
-class Trashbin {
+/** @template-implements IEventListener<BeforeNodeDeletedEvent> */
+class Trashbin implements IEventListener {
 	// unit: percentage; 50% of available disk space/quota
 	public const DEFAULTMAXSIZE = 50;
 
 	/**
 	 * Ensure we don't need to scan the file during the move to trash
 	 * by triggering the scan in the pre-hook
-	 *
-	 * @param array $params
 	 */
-	public static function ensureFileScannedHook($params) {
+	public static function ensureFileScannedHook(Node $node): void {
 		try {
-			self::getUidAndFilename($params['path']);
+			self::getUidAndFilename($node->getPath());
 		} catch (NotFoundException $e) {
-			// nothing to scan for non existing files
+			// Nothing to scan for non existing files
 		}
 	}
 
@@ -1170,4 +1172,10 @@ class Trashbin {
 			return new NonExistingFile($rootFolder, $view, $fullPath);
 		}
 	}
+
+	public function handle(Event $event): void {
+		if ($event instanceof BeforeNodeDeletedEvent) {
+			self::ensureFileScannedHook($event->getNode());
+		}
+	}
 }

lib/composer/composer/autoload_classmap.php

@@ -349,6 +349,7 @@ return array(
     'OCP\\Files\\EntityTooLargeException' => $baseDir . '/lib/public/Files/EntityTooLargeException.php',
     'OCP\\Files\\Events\\BeforeDirectFileDownloadEvent' => $baseDir . '/lib/public/Files/Events/BeforeDirectFileDownloadEvent.php',
     'OCP\\Files\\Events\\BeforeFileScannedEvent' => $baseDir . '/lib/public/Files/Events/BeforeFileScannedEvent.php',
+    'OCP\\Files\\Events\\BeforeFileSystemSetupEvent' => $baseDir . '/lib/public/Files/Events/BeforeFileSystemSetupEvent.php',
     'OCP\\Files\\Events\\BeforeFolderScannedEvent' => $baseDir . '/lib/public/Files/Events/BeforeFolderScannedEvent.php',
     'OCP\\Files\\Events\\BeforeZipCreatedEvent' => $baseDir . '/lib/public/Files/Events/BeforeZipCreatedEvent.php',
     'OCP\\Files\\Events\\FileCacheUpdated' => $baseDir . '/lib/public/Files/Events/FileCacheUpdated.php',

lib/composer/composer/autoload_static.php

@@ -382,6 +382,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\Files\\EntityTooLargeException' => __DIR__ . '/../../..' . '/lib/public/Files/EntityTooLargeException.php',
         'OCP\\Files\\Events\\BeforeDirectFileDownloadEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeDirectFileDownloadEvent.php',
         'OCP\\Files\\Events\\BeforeFileScannedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeFileScannedEvent.php',
+        'OCP\\Files\\Events\\BeforeFileSystemSetupEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeFileSystemSetupEvent.php',
         'OCP\\Files\\Events\\BeforeFolderScannedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeFolderScannedEvent.php',
         'OCP\\Files\\Events\\BeforeZipCreatedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeZipCreatedEvent.php',
         'OCP\\Files\\Events\\FileCacheUpdated' => __DIR__ . '/../../..' . '/lib/public/Files/Events/FileCacheUpdated.php',

lib/private/Files/SetupManager.php

@@ -35,6 +35,7 @@ use OCP\Files\Config\ICachedMountInfo;
 use OCP\Files\Config\IHomeMountProvider;
 use OCP\Files\Config\IMountProvider;
 use OCP\Files\Config\IUserMountCache;
+use OCP\Files\Events\BeforeFileSystemSetupEvent;
 use OCP\Files\Events\InvalidateMountCacheEvent;
 use OCP\Files\Events\Node\FilesystemTornDownEvent;
 use OCP\Files\Mount\IMountManager;
@@ -227,8 +228,12 @@ class SetupManager {
 
 		$prevLogging = Filesystem::logWarningWhenAddingStorageWrapper(false);
 
+		// TODO remove hook
 		OC_Hook::emit('OC_Filesystem', 'preSetup', ['user' => $user->getUID()]);
 
+		$event = new BeforeFileSystemSetupEvent($user);
+		$this->eventDispatcher->dispatchTyped($event);
+
 		Filesystem::logWarningWhenAddingStorageWrapper($prevLogging);
 
 		$userDir = '/' . $user->getUID() . '/files';

lib/public/Files/Events/BeforeFileSystemSetupEvent.php

@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCP\Files\Events;
+
+use OCP\EventDispatcher\Event;
+use OCP\IUser;
+
+/**
+ * Event triggered before the file system is setup
+ *
+ * @since 31.0.0
+ */
+class BeforeFileSystemSetupEvent extends Event {
+	/**
+	 * @since 31.0.0
+	 */
+	public function __construct(
+		private IUser $user,
+	) {
+		parent::__construct();
+	}
+
+	/**
+	 * @since 31.0.0
+	 */
+	public function getUser(): IUser {
+		return $this->user;
+	}
+}

tests/lib/Files/ViewTest.php

@@ -16,6 +16,7 @@ use OC\Files\Storage\Storage;
 use OC\Files\Storage\Temporary;
 use OC\Files\View;
 use OC\Share20\ShareDisableChecker;
+use OCA\Files_Trashbin\Trash\ITrashManager;
 use OCP\Cache\CappedMemoryCache;
 use OCP\Constants;
 use OCP\Files\Config\IMountProvider;
@@ -26,9 +27,11 @@ use OCP\Files\Storage\IStorage;
 use OCP\IDBConnection;
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
+use OCP\Server;
 use OCP\Share\IManager as IShareManager;
 use OCP\Share\IShare;
 use OCP\Util;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\HookHelper;
 use Test\TestMoveableMountPoint;
 use Test\Traits\UserTrait;
@@ -1858,11 +1861,14 @@ class ViewTest extends \Test\TestCase {
 	): void {
 		$view = new View('/' . $this->user . '/files/');
 
-		/** @var Temporary|\PHPUnit\Framework\MockObject\MockObject $storage */
+		/** @var Temporary&MockObject $storage */
 		$storage = $this->getMockBuilder(Temporary::class)
 			->setMethods([$operation])
 			->getMock();
 
+		/* Pause trash to avoid the trashbin intercepting rmdir and unlink calls */
+		Server::get(ITrashManager::class)->pauseTrash();
+
 		Filesystem::mount($storage, [], $this->user . '/');
 
 		// work directly on disk because mkdir might be mocked
@@ -1898,6 +1904,9 @@ class ViewTest extends \Test\TestCase {
 		}
 
 		$this->assertEquals($expectedStrayLock, $this->getFileLockType($view, $lockedPath));
+
+		/* Resume trash to avoid side effects */
+		Server::get(ITrashManager::class)->resumeTrash();
 	}
 
 	/**
@@ -2007,6 +2016,9 @@ class ViewTest extends \Test\TestCase {
 			->setMethods([$operation])
 			->getMock();
 
+		/* Pause trash to avoid the trashbin intercepting rmdir and unlink calls */
+		Server::get(ITrashManager::class)->pauseTrash();
+
 		Filesystem::mount($storage, [], $this->user . '/');
 
 		// work directly on disk because mkdir might be mocked
@@ -2033,6 +2045,9 @@ class ViewTest extends \Test\TestCase {
 		}
 		$this->assertTrue($thrown, 'Exception was rethrown');
 		$this->assertNull($this->getFileLockType($view, $path), 'File got unlocked after exception');
+
+		/* Resume trash to avoid side effects */
+		Server::get(ITrashManager::class)->resumeTrash();
 	}
 
 	public function testLockBasicOperationUnlocksAfterLockException(): void {

tests/lib/Security/CertificateManagerTest.php

@@ -41,7 +41,7 @@ class CertificateManagerTest extends \Test\TestCase {
 		$this->registerMount($this->username, $storage, '/' . $this->username . '/');
 
 		\OC_Util::tearDownFS();
-		\OC_User::setUserId('');
+		\OC_User::setUserId($this->username);
 		\OC\Files\Filesystem::tearDown();
 		\OC_Util::setupFS($this->username);