fix(encryption): Improve Update class and event listenening

 to avoid back&forth between path and Node object

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

2 files changed, 70 insertions, 85 deletions

diff --git a/tests/lib/Encryption/UpdateTest.php b/tests/lib/Encryption/UpdateTest.php
index d54aeab35dd..9940549981f 100644
--- a/tests/lib/Encryption/UpdateTest.php
+++ b/tests/lib/Encryption/UpdateTest.php
@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2017-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -10,20 +13,19 @@ namespace Test\Encryption;
 use OC\Encryption\File;
 use OC\Encryption\Update;
 use OC\Encryption\Util;
-use OC\Files\Mount\Manager;
 use OC\Files\View;
 use OCP\Encryption\IEncryptionModule;
+use OCP\Files\File as OCPFile;
+use OCP\Files\Folder;
+use OCP\IUser;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class UpdateTest extends TestCase {
-	private Update $update;
-
 	private string $uid;
 	private View&MockObject $view;
 	private Util&MockObject $util;
-	private Manager&MockObject $mountManager;
 	private \OC\Encryption\Manager&MockObject $encryptionManager;
 	private IEncryptionModule&MockObject $encryptionModule;
 	private File&MockObject $fileHelper;
@@ -34,21 +36,44 @@ class UpdateTest extends TestCase {
 
 		$this->view = $this->createMock(View::class);
 		$this->util = $this->createMock(Util::class);
-		$this->mountManager = $this->createMock(Manager::class);
 		$this->encryptionManager = $this->createMock(\OC\Encryption\Manager::class);
 		$this->fileHelper = $this->createMock(File::class);
 		$this->encryptionModule = $this->createMock(IEncryptionModule::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 
 		$this->uid = 'testUser1';
+	}
 
-		$this->update = new Update(
-			$this->util,
-			$this->mountManager,
-			$this->encryptionManager,
-			$this->fileHelper,
-			$this->logger,
-			$this->uid);
+	private function getUserMock(string $uid): IUser&MockObject {
+		$user = $this->createMock(IUser::class);
+		$user->expects(self::any())
+			->method('getUID')
+			->willReturn($uid);
+		return $user;
+	}
+
+	private function getFileMock(string $path, string $owner): OCPFile&MockObject {
+		$node = $this->createMock(OCPFile::class);
+		$node->expects(self::atLeastOnce())
+			->method('getPath')
+			->willReturn($path);
+		$node->expects(self::any())
+			->method('getOwner')
+			->willReturn($this->getUserMock($owner));
+
+		return $node;
+	}
+
+	private function getFolderMock(string $path, string $owner): Folder&MockObject {
+		$node = $this->createMock(Folder::class);
+		$node->expects(self::atLeastOnce())
+			->method('getPath')
+			->willReturn($path);
+		$node->expects(self::any())
+			->method('getOwner')
+			->willReturn($this->getUserMock($owner));
+
+		return $node;
 	}
 
 	/**
@@ -60,6 +85,10 @@ class UpdateTest extends TestCase {
 	 * @param integer $numberOfFiles
 	 */
 	public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void {
+		$updateMock = $this->getUpdateMock(['getOwnerPath']);
+		$updateMock->expects($this->once())->method('getOwnerPath')
+			->willReturnCallback(fn (OCPFile|Folder $node) => '/user/' . $node->getPath());
+
 		$this->encryptionManager->expects($this->once())
 			->method('getEncryptionModule')
 			->willReturn($this->encryptionModule);
@@ -68,6 +97,9 @@ class UpdateTest extends TestCase {
 			$this->util->expects($this->once())
 				->method('getAllFiles')
 				->willReturn($allFiles);
+			$node = $this->getFolderMock($path, 'user');
+		} else {
+			$node = $this->getFileMock($path, 'user');
 		}
 
 		$this->fileHelper->expects($this->exactly($numberOfFiles))
@@ -78,7 +110,7 @@ class UpdateTest extends TestCase {
 			->method('update')
 			->willReturn(true);
 
-		$this->update->update($isDir, $path);
+		$updateMock->update($node);
 	}
 
 	/**
@@ -98,32 +130,26 @@ class UpdateTest extends TestCase {
 	 *
 	 * @param string $source
 	 * @param string $target
-	 * @param boolean $encryptionEnabled
 	 */
-	public function testPostRename($source, $target, $encryptionEnabled): void {
-		$updateMock = $this->getUpdateMock(['update', 'getOwnerPath']);
+	public function testPostRename($source, $target): void {
+		$updateMock = $this->getUpdateMock(['update','getOwnerPath']);
 
-		$this->encryptionManager->expects($this->once())
-			->method('isEnabled')
-			->willReturn($encryptionEnabled);
+		$sourceNode = $this->getFileMock($source, 'user');
+		$targetNode = $this->getFileMock($target, 'user');
 
-		if (dirname($source) === dirname($target) || $encryptionEnabled === false) {
+		if (dirname($source) === dirname($target)) {
 			$updateMock->expects($this->never())->method('getOwnerPath');
 			$updateMock->expects($this->never())->method('update');
 		} else {
-			$updateMock->expects($this->once())
-				->method('getOwnerPath')
-				->willReturnCallback(function ($path) use ($target) {
-					$this->assertSame(
-						$target,
-						$path,
-						'update needs to be executed for the target destination');
-					return ['owner', $path];
-				});
-			$updateMock->expects($this->once())->method('update');
+			$updateMock->expects($this->once())->method('update')
+				->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame(
+					$target,
+					$node->getPath(),
+					'update needs to be executed for the target destination'
+				));
 		}
 
-		$updateMock->postRename(false, $source, $target);
+		$updateMock->postRename($sourceNode, $targetNode);
 	}
 
 	/**
@@ -133,61 +159,35 @@ class UpdateTest extends TestCase {
 	 */
 	public function dataTestPostRename() {
 		return [
-			['/test.txt', '/testNew.txt', true],
-			['/test.txt', '/testNew.txt', false],
-			['/folder/test.txt', '/testNew.txt', true],
-			['/folder/test.txt', '/testNew.txt', false],
-			['/folder/test.txt', '/testNew.txt', true],
-			['/test.txt', '/folder/testNew.txt', false],
+			['/test.txt', '/testNew.txt'],
+			['/folder/test.txt', '/testNew.txt'],
+			['/test.txt', '/folder/testNew.txt'],
 		];
 	}
 
-
-	/**
-	 * @dataProvider dataTestPostRestore
-	 *
-	 * @param boolean $encryptionEnabled
-	 */
-	public function testPostRestore($encryptionEnabled): void {
+	public function testPostRestore(): void {
 		$updateMock = $this->getUpdateMock(['update']);
 
-		$this->encryptionManager->expects($this->once())
-			->method('isEnabled')
-			->willReturn($encryptionEnabled);
-
-		if ($encryptionEnabled) {
-			$updateMock->expects($this->once())->method('update');
-		} else {
-			$updateMock->expects($this->never())->method('update');
-		}
+		$updateMock->expects($this->once())->method('update')
+			->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame(
+				'/folder/test.txt',
+				$node->getPath(),
+				'update needs to be executed for the target destination'
+			));
 
-		$updateMock->postRestore(false, '/folder/test.txt');
-	}
-
-	/**
-	 * test data for testPostRestore()
-	 *
-	 * @return array
-	 */
-	public function dataTestPostRestore() {
-		return [
-			[true],
-			[false],
-		];
+		$updateMock->postRestore($this->getFileMock('/folder/test.txt', 'user'));
 	}
 
 	/**
 	 * create mock of the update method
 	 *
 	 * @param array $methods methods which should be set
-	 * @return \OC\Encryption\Update | MockObject
 	 */
-	protected function getUpdateMock($methods) {
-		return  $this->getMockBuilder('\OC\Encryption\Update')
+	protected function getUpdateMock(array $methods): Update&MockObject {
+		return  $this->getMockBuilder(Update::class)
 			->setConstructorArgs(
 				[
 					$this->util,
-					$this->mountManager,
 					$this->encryptionManager,
 					$this->fileHelper,
 					$this->logger,
diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
index bb3df36dec2..e8ad557dae5 100644
--- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
+++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
@@ -11,7 +11,6 @@ use Exception;
 use OC;
 use OC\Encryption\Exceptions\ModuleDoesNotExistsException;
 use OC\Encryption\File;
-use OC\Encryption\Update;
 use OC\Encryption\Util;
 use OC\Files\Cache\Cache;
 use OC\Files\Cache\CacheEntry;
@@ -46,7 +45,6 @@ class EncryptionTest extends Storage {
 	private Util&MockObject $util;
 	private \OC\Encryption\Manager&MockObject $encryptionManager;
 	private IEncryptionModule&MockObject $encryptionModule;
-	private Update&MockObject $update;
 	private Cache&MockObject $cache;
 	private LoggerInterface&MockObject $logger;
 	private File&MockObject $file;
@@ -111,9 +109,6 @@ class EncryptionTest extends Storage {
 		$this->keyStore = $this->getMockBuilder('\OC\Encryption\Keys\Storage')
 			->disableOriginalConstructor()->getMock();
 
-		$this->update = $this->getMockBuilder('\OC\Encryption\Update')
-			->disableOriginalConstructor()->getMock();
-
 		$this->mount = $this->getMockBuilder('\OC\Files\Mount\MountPoint')
 			->disableOriginalConstructor()
 			->setMethods(['getOption'])
@@ -155,7 +150,6 @@ class EncryptionTest extends Storage {
 					$this->file,
 					null,
 					$this->keyStore,
-					$this->update,
 					$this->mountManager,
 					$this->arrayCache
 				]
@@ -237,7 +231,6 @@ class EncryptionTest extends Storage {
 					$this->file,
 					null,
 					$this->keyStore,
-					$this->update,
 					$this->mountManager,
 					$this->arrayCache,
 				]
@@ -316,7 +309,6 @@ class EncryptionTest extends Storage {
 					$this->file,
 					null,
 					$this->keyStore,
-					$this->update,
 					$this->mountManager,
 					$this->arrayCache,
 				]
@@ -361,7 +353,6 @@ class EncryptionTest extends Storage {
 					$this->file,
 					null,
 					$this->keyStore,
-					$this->update,
 					$this->mountManager,
 					$this->arrayCache,
 				]
@@ -491,7 +482,6 @@ class EncryptionTest extends Storage {
 			$this->file,
 			null,
 			$this->keyStore,
-			$this->update,
 			$this->mountManager,
 			$this->arrayCache,
 		);
@@ -598,7 +588,6 @@ class EncryptionTest extends Storage {
 					$this->file,
 					null,
 					$this->keyStore,
-					$this->update,
 					$this->mountManager,
 					$this->arrayCache,
 				]
@@ -692,7 +681,6 @@ class EncryptionTest extends Storage {
 					$this->file,
 					null,
 					$this->keyStore,
-					$this->update,
 					$this->mountManager,
 					$this->arrayCache,
 				]
@@ -867,7 +855,6 @@ class EncryptionTest extends Storage {
 					$this->file,
 					null,
 					$this->keyStore,
-					$this->update,
 					$this->mountManager,
 					$this->arrayCache
 				]
@@ -968,7 +955,6 @@ class EncryptionTest extends Storage {
 		$util = $this->createMock(Util::class);
 		$fileHelper = $this->createMock(IFile::class);
 		$keyStorage = $this->createMock(IStorage::class);
-		$update = $this->createMock(Update::class);
 		$mountManager = $this->createMock(\OC\Files\Mount\Manager::class);
 		$mount = $this->createMock(IMountPoint::class);
 		$arrayCache = $this->createMock(ArrayCache::class);
@@ -986,7 +972,6 @@ class EncryptionTest extends Storage {
 					$fileHelper,
 					null,
 					$keyStorage,
-					$update,
 					$mountManager,
 					$arrayCache
 				]