Merge pull request #51355 from AIlkiv/refactor/rollback-hook-usage

refactor: cleanup rollback hook usage

apps/admin_audit/lib/Actions/Versions.php

@@ -8,16 +8,6 @@ declare(strict_types=1);
 namespace OCA\AdminAudit\Actions;
 
 class Versions extends Action {
-	public function rollback(array $params): void {
-		$this->log('Version "%s" of "%s" was restored.',
-			[
-				'version' => $params['revision'],
-				'path' => $params['path']
-			],
-			['version', 'path']
-		);
-	}
-
 	public function delete(array $params): void {
 		$this->log('Version "%s" was deleted.',
 			['path' => $params['path']],

apps/admin_audit/lib/AppInfo/Application.php

@@ -27,6 +27,7 @@ use OCA\AdminAudit\Listener\GroupManagementEventListener;
 use OCA\AdminAudit\Listener\SecurityEventListener;
 use OCA\AdminAudit\Listener\SharingEventListener;
 use OCA\AdminAudit\Listener\UserManagementEventListener;
+use OCA\Files_Versions\Events\VersionRestoredEvent;
 use OCP\App\Events\AppDisableEvent;
 use OCP\App\Events\AppEnableEvent;
 use OCP\App\Events\AppUpdateEvent;
@@ -110,6 +111,7 @@ class Application extends App implements IBootstrap {
 
 		// File events
 		$context->registerEventListener(BeforePreviewFetchedEvent::class, FileEventListener::class);
+		$context->registerEventListener(VersionRestoredEvent::class, FileEventListener::class);
 
 		// Security events
 		$context->registerEventListener(TwoFactorProviderChallengePassed::class, SecurityEventListener::class);
@@ -220,7 +222,6 @@ class Application extends App implements IBootstrap {
 
 	private function versionsHooks(IAuditLogger $logger): void {
 		$versionsActions = new Versions($logger);
-		Util::connectHook('\OCP\Versions', 'rollback', $versionsActions, 'rollback');
 		Util::connectHook('\OCP\Versions', 'delete', $versionsActions, 'delete');
 	}
 

apps/admin_audit/lib/Listener/FileEventListener.php

@@ -10,6 +10,7 @@ declare(strict_types=1);
 namespace OCA\AdminAudit\Listener;
 
 use OCA\AdminAudit\Actions\Action;
+use OCA\Files_Versions\Events\VersionRestoredEvent;
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventListener;
 use OCP\Files\InvalidPathException;
@@ -19,12 +20,14 @@ use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 /**
- * @template-implements IEventListener<BeforePreviewFetchedEvent>
+ * @template-implements IEventListener<BeforePreviewFetchedEvent|VersionRestoredEvent>
  */
 class FileEventListener extends Action implements IEventListener {
 	public function handle(Event $event): void {
 		if ($event instanceof BeforePreviewFetchedEvent) {
 			$this->beforePreviewFetched($event);
+		} elseif ($event instanceof VersionRestoredEvent) {
+			$this->versionRestored($event);
 		}
 	}
 
@@ -54,4 +57,18 @@ class FileEventListener extends Action implements IEventListener {
 			return;
 		}
 	}
+
+	/**
+	 * Logs when a version is restored
+	 */
+	private function versionRestored(VersionRestoredEvent $event): void {
+		$version = $event->getVersion();
+		$this->log('Version "%s" of "%s" was restored.',
+			[
+				'version' => $version->getRevisionId(),
+				'path' => $version->getVersionPath()
+			],
+			['version', 'path']
+		);
+	}
 }

apps/files_versions/tests/VersioningTest.php

@@ -16,9 +16,11 @@ use OC\User\NoUserException;
 use OCA\Files_Sharing\AppInfo\Application;
 use OCA\Files_Versions\Db\VersionEntity;
 use OCA\Files_Versions\Db\VersionsMapper;
+use OCA\Files_Versions\Events\VersionRestoredEvent;
 use OCA\Files_Versions\Storage;
 use OCA\Files_Versions\Versions\IVersionManager;
 use OCP\Constants;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\IMimeTypeLoader;
 use OCP\IConfig;
 use OCP\IUser;
@@ -804,8 +806,13 @@ class VersioningTest extends \Test\TestCase {
 		$this->assertEquals('test file', $this->rootView->file_get_contents($filePath));
 		$info1 = $this->rootView->getFileInfo($filePath);
 
-		$params = [];
-		$this->connectMockHooks('rollback', $params);
+		$eventDispatcher = Server::get(IEventDispatcher::class);
+		$eventFired = false;
+		$eventDispatcher->addListener(VersionRestoredEvent::class, function ($event) use (&$eventFired, $t2) {
+			$eventFired = true;
+			$this->assertEquals('/sub/test.txt', $event->getVersion()->getVersionPath());
+			$this->assertTrue($event->getVersion()->getRevisionId() > 0);
+		});
 
 		$versionManager = Server::get(IVersionManager::class);
 		$versions = $versionManager->getVersionsForFile($this->user1, $info1);
@@ -813,13 +820,8 @@ class VersioningTest extends \Test\TestCase {
 			return $version->getRevisionId() === $t2;
 		});
 		$this->assertTrue($versionManager->rollback(current($version)));
-		$expectedParams = [
-			'path' => '/sub/test.txt',
-		];
 
-		$this->assertEquals($expectedParams['path'], $params['path']);
-		$this->assertTrue(array_key_exists('revision', $params));
-		$this->assertTrue($params['revision'] > 0);
+		$this->assertTrue($eventFired, 'VersionRestoredEvent was not fired');
 
 		$this->assertEquals('version2', $this->rootView->file_get_contents($filePath));
 		$info2 = $this->rootView->getFileInfo($filePath);

lib/private/Preview/Watcher.php

@@ -8,6 +8,7 @@ declare(strict_types=1);
  */
 namespace OC\Preview;
 
+use OCP\Files\FileInfo;
 use OCP\Files\Folder;
 use OCP\Files\IAppData;
 use OCP\Files\Node;
@@ -37,7 +38,7 @@ class Watcher {
 		$this->deleteNode($node);
 	}
 
-	protected function deleteNode(Node $node) {
+	protected function deleteNode(FileInfo $node) {
 		// We only handle files
 		if ($node instanceof Folder) {
 			return;

lib/private/Preview/WatcherConnector.php

@@ -9,43 +9,33 @@ declare(strict_types=1);
 namespace OC\Preview;
 
 use OC\SystemConfig;
+use OCA\Files_Versions\Events\VersionRestoredEvent;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
 
 class WatcherConnector {
-	/** @var IRootFolder */
-	private $root;
-
-	/** @var SystemConfig */
-	private $config;
-
-	/**
-	 * WatcherConnector constructor.
-	 *
-	 * @param IRootFolder $root
-	 * @param SystemConfig $config
-	 */
-	public function __construct(IRootFolder $root,
-		SystemConfig $config) {
-		$this->root = $root;
-		$this->config = $config;
+	public function __construct(
+		private IRootFolder $root,
+		private SystemConfig $config,
+		private IEventDispatcher $dispatcher,
+	) {
 	}
 
-	/**
-	 * @return Watcher
-	 */
 	private function getWatcher(): Watcher {
 		return \OCP\Server::get(Watcher::class);
 	}
 
-	public function connectWatcher() {
+	public function connectWatcher(): void {
 		// Do not connect if we are not setup yet!
 		if ($this->config->getValue('instanceid', null) !== null) {
 			$this->root->listen('\OC\Files', 'postWrite', function (Node $node) {
 				$this->getWatcher()->postWrite($node);
 			});
 
-			\OC_Hook::connect('\OCP\Versions', 'rollback', $this->getWatcher(), 'versionRollback');
+			$this->dispatcher->addListener(VersionRestoredEvent::class, function (VersionRestoredEvent $event) {
+				$this->getWatcher()->versionRollback(['node' => $event->getVersion()->getSourceFile()]);
+			});
 		}
 	}
 }

lib/private/Server.php

@@ -411,7 +411,8 @@ class Server extends ServerContainer implements IServerContainer {
 
 			$previewConnector = new \OC\Preview\WatcherConnector(
 				$root,
-				$c->get(SystemConfig::class)
+				$c->get(SystemConfig::class),
+				$this->get(IEventDispatcher::class)
 			);
 			$previewConnector->connectWatcher();