Adapt files_versions tests to modernized hooks

Signed-off-by: Louis Chemineau <louis@chmn.me>

4 files changed, 51 insertions, 22 deletions

diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php
index 0a7a129ca28..0a22a9ea555 100644
--- a/apps/files_trashbin/tests/StorageTest.php
+++ b/apps/files_trashbin/tests/StorageTest.php
@@ -35,11 +35,13 @@ use OC\Files\Filesystem;
 use OC\Files\Storage\Common;
 use OC\Files\Storage\Local;
 use OC\Files\Storage\Temporary;
+use OCA\Files_Versions\AppInfo\Application as FVApplication;
 use OCA\Files_Trashbin\AppInfo\Application;
 use OCA\Files_Trashbin\Events\MoveToTrashEvent;
 use OCA\Files_Trashbin\Storage;
 use OCA\Files_Trashbin\Trash\ITrashManager;
 use OCP\AppFramework\Bootstrap\IBootContext;
+use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\Cache\ICache;
 use OCP\Files\Folder;
@@ -92,6 +94,8 @@ class StorageTest extends \Test\TestCase {
 		parent::setUp();
 
 		\OC_Hook::clear();
+		\OC::$server->boot();
+
 		// register trashbin hooks
 		$trashbinApp = new Application();
 		$trashbinApp->boot($this->createMock(IBootContext::class));
@@ -224,8 +228,6 @@ class StorageTest extends \Test\TestCase {
 	 * Test that deleted versions properly land in the trashbin.
 	 */
 	public function testDeleteVersionsOfFile() {
-		\OCA\Files_Versions\Hooks::connectHooks();
-
 		// trigger a version (multiple would not work because of the expire logic)
 		$this->userView->file_put_contents('test.txt', 'v1');
 
@@ -253,8 +255,6 @@ class StorageTest extends \Test\TestCase {
 	 * Test that deleted versions properly land in the trashbin.
 	 */
 	public function testDeleteVersionsOfFolder() {
-		\OCA\Files_Versions\Hooks::connectHooks();
-
 		// trigger a version (multiple would not work because of the expire logic)
 		$this->userView->file_put_contents('folder/inside.txt', 'v1');
 
@@ -288,8 +288,6 @@ class StorageTest extends \Test\TestCase {
 	 * Test that deleted versions properly land in the trashbin when deleting as share recipient.
 	 */
 	public function testDeleteVersionsOfFileAsRecipient() {
-		\OCA\Files_Versions\Hooks::connectHooks();
-
 		$this->userView->mkdir('share');
 		// trigger a version (multiple would not work because of the expire logic)
 		$this->userView->file_put_contents('share/test.txt', 'v1');
@@ -341,8 +339,6 @@ class StorageTest extends \Test\TestCase {
 	 * Test that deleted versions properly land in the trashbin when deleting as share recipient.
 	 */
 	public function testDeleteVersionsOfFolderAsRecipient() {
-		\OCA\Files_Versions\Hooks::connectHooks();
-
 		$this->userView->mkdir('share');
 		$this->userView->mkdir('share/folder');
 		// trigger a version (multiple would not work because of the expire logic)
@@ -410,8 +406,6 @@ class StorageTest extends \Test\TestCase {
 	 * unlink() which should NOT trigger the version deletion logic.
 	 */
 	public function testKeepFileAndVersionsWhenMovingFileBetweenStorages() {
-		\OCA\Files_Versions\Hooks::connectHooks();
-
 		$storage2 = new Temporary([]);
 		\OC\Files\Filesystem::mount($storage2, [], $this->user . '/files/substorage');
 
@@ -451,8 +445,6 @@ class StorageTest extends \Test\TestCase {
 	 * unlink() which should NOT trigger the version deletion logic.
 	 */
 	public function testKeepFileAndVersionsWhenMovingFolderBetweenStorages() {
-		\OCA\Files_Versions\Hooks::connectHooks();
-
 		$storage2 = new Temporary([]);
 		\OC\Files\Filesystem::mount($storage2, [], $this->user . '/files/substorage');
 
diff --git a/apps/files_versions/lib/Listener/FileEventsListener.php b/apps/files_versions/lib/Listener/FileEventsListener.php
index 12bc8b03fe9..c28a5a4c999 100644
--- a/apps/files_versions/lib/Listener/FileEventsListener.php
+++ b/apps/files_versions/lib/Listener/FileEventsListener.php
@@ -11,6 +11,7 @@
  * @author Robin Appelman <robin@icewind.nl>
  * @author Robin McCorkell <robin@mccorkell.me.uk>
  * @author Sam Tuke <mail@samtuke.com>
+ * @author Louis Chmn <louis@chmn.me>
  *
  * @license AGPL-3.0
  *
@@ -58,6 +59,10 @@ class FileEventsListener implements IEventListener {
 	 * @var array<int, bool>
 	 */
 	private array $versionsCreated = [];
+	/**
+	 * @var array<string, Node>
+	 */
+	private array $versionsDeleted = [];
 	private IMimeTypeLoader $mimeTypeLoader;
 
 	public function __construct(
@@ -108,12 +113,17 @@ class FileEventsListener implements IEventListener {
 	 * listen to write event.
 	 */
 	public function write_hook(Node $node): void {
-		// Prevent exception during installation.
+		// $node is be nonexisting on file creation.
 		if ($node instanceof NonExistingFolder || $node instanceof NonExistingFile) {
 			return;
 		}
 
-		$userFolder = $this->rootFolder->getUserFolder($node->getOwner()->getUID());
+		$owner = $node->getOwner();
+		// if ($owner === null) {
+		// 	return;
+		// }
+		$userFolder = $this->rootFolder->getUserFolder($owner->getUID());
+
 		$path = $userFolder->getRelativePath($node->getPath());
 		$result = Storage::store($path);
 
@@ -151,9 +161,15 @@ class FileEventsListener implements IEventListener {
 	 * cleanup the versions directory if the actual file gets deleted
 	 */
 	public function remove_hook(Node $node): void {
+		$path = Filesystem::normalizePath($node->getPath());
+		if (!array_key_exists($path, $this->versionsDeleted)) {
+			return;
+		}
+		$node = $this->versionsDeleted[$path];
 		$userFolder = $this->rootFolder->getUserFolder($node->getOwner()->getUID());
-		$path = $userFolder->getRelativePath($node->getPath());
-		Storage::delete($path);
+		$relativePath = $userFolder->getRelativePath($node->getPath());
+		unset($this->versionsDeleted[$path]);
+		Storage::delete($relativePath);
 		$this->versionsMapper->deleteAllVersionsForFileId($node->getId());
 	}
 
@@ -161,9 +177,15 @@ class FileEventsListener implements IEventListener {
 	 * mark file as "deleted" so that we can clean up the versions if the file is gone
 	 */
 	public function pre_remove_hook(Node $node): void {
-		$userFolder = $this->rootFolder->getUserFolder($node->getOwner()->getUID());
+		$owner = $node->getOwner();
+		// if ($owner === null) {
+		// 	return;
+		// }
+		$userFolder = $this->rootFolder->getUserFolder($owner->getUID());
+
 		$path = $userFolder->getRelativePath($node->getPath());
 		Storage::markDeletedFile($path);
+		$this->versionsDeleted[$node->getPath()] = $node;
 	}
 
 	/**
@@ -173,7 +195,12 @@ class FileEventsListener implements IEventListener {
 	 * of the stored versions along the actual file
 	 */
 	public function rename_hook(Node $source, Node $target): void {
-		$userFolder = $this->rootFolder->getUserFolder($target->getOwner()->getUID());
+		$owner = $target->getOwner();
+		// if ($owner === null) {
+		// 	return;
+		// }
+		$userFolder = $this->rootFolder->getUserFolder($owner->getUID());
+
 		$oldPath = $userFolder->getRelativePath($source->getPath());
 		$newPath = $userFolder->getRelativePath($target->getPath());
 		Storage::renameOrCopy($oldPath, $newPath, 'rename');
@@ -186,7 +213,12 @@ class FileEventsListener implements IEventListener {
 	 * the stored versions to the new location
 	 */
 	public function copy_hook(Node $source, Node $target): void {
-		$userFolder = $this->rootFolder->getUserFolder($target->getOwner()->getUID());
+		$owner = $target->getOwner();
+		// if ($owner === null) {
+		// 	return;
+		// }
+		$userFolder = $this->rootFolder->getUserFolder($owner->getUID());
+
 		$oldPath = $userFolder->getRelativePath($source->getPath());
 		$newPath = $userFolder->getRelativePath($target->getPath());
 		Storage::renameOrCopy($oldPath, $newPath, 'copy');
@@ -202,7 +234,12 @@ class FileEventsListener implements IEventListener {
 	public function pre_renameOrCopy_hook(Node $source, Node $target): void {
 		// if we rename a movable mount point, then the versions don't have
 		// to be renamed
-		$userFolder = $this->rootFolder->getUserFolder($source->getOwner()->getUID());
+		$owner = $source->getOwner();
+		// if ($owner === null) {
+		// 	return;
+		// }
+		$userFolder = $this->rootFolder->getUserFolder($owner->getUID());
+
 		$oldPath = $userFolder->getRelativePath($source->getPath());
 		$newPath = $userFolder->getRelativePath($target->getPath());
 		$absOldPath = Filesystem::normalizePath('/' . \OC_User::getUser() . '/files' . $oldPath);
diff --git a/apps/files_versions/tests/StorageTest.php b/apps/files_versions/tests/StorageTest.php
index d16b9ecdfd8..ef599a3e479 100644
--- a/apps/files_versions/tests/StorageTest.php
+++ b/apps/files_versions/tests/StorageTest.php
@@ -51,7 +51,7 @@ class StorageTest extends TestCase {
 			});
 		$this->overwriteService(Expiration::class, $expiration);
 
-		Hooks::connectHooks();
+		// Hooks::connectHooks();
 
 		$this->createUser('version_test', '');
 		$this->loginAsUser('version_test');
diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php
index b8f58ecff2b..15173dac902 100644
--- a/apps/files_versions/tests/VersioningTest.php
+++ b/apps/files_versions/tests/VersioningTest.php
@@ -100,7 +100,7 @@ class VersioningTest extends \Test\TestCase {
 		// clear hooks
 		\OC_Hook::clear();
 		\OC::registerShareHooks(\OC::$server->getSystemConfig());
-		\OCA\Files_Versions\Hooks::connectHooks();
+		// \OCA\Files_Versions\Hooks::connectHooks();
 
 		self::loginHelper(self::TEST_VERSIONS_USER);
 		$this->rootView = new \OC\Files\View();