Merge pull request #17641 from owncloud/fix_objectstore_rename

don't move files in cache twice, fixes renaming for objectstores

9 files changed, 100 insertions, 45 deletions

diff --git a/apps/dav/tests/unit/connector/sabre/file.php b/apps/dav/tests/unit/connector/sabre/file.php
index 9ca8c994a04..9171fc3b786 100644
--- a/apps/dav/tests/unit/connector/sabre/file.php
+++ b/apps/dav/tests/unit/connector/sabre/file.php
@@ -8,6 +8,7 @@
 
 namespace Test\Connector\Sabre;
 
+use OC\Files\Storage\Local;
 use Test\HookHelper;
 use OC\Files\Filesystem;
 use OCP\Lock\ILockingProvider;
@@ -798,14 +799,16 @@ class File extends \Test\TestCase {
 		}
 		$files = [];
 		list($storage, $internalPath) = $userView->resolvePath($path);
-		$realPath = $storage->getSourcePath($internalPath);
-		$dh = opendir($realPath);
-		while (($file = readdir($dh)) !== false) {
-			if (substr($file, strlen($file) - 5, 5) === '.part') {
-				$files[] = $file;
+		if($storage instanceof Local) {
+			$realPath = $storage->getSourcePath($internalPath);
+			$dh = opendir($realPath);
+			while (($file = readdir($dh)) !== false) {
+				if (substr($file, strlen($file) - 5, 5) === '.part') {
+					$files[] = $file;
+				}
 			}
+			closedir($dh);
 		}
-		closedir($dh);
 		return $files;
 	}
 
diff --git a/apps/encryption/tests/lib/MigrationTest.php b/apps/encryption/tests/lib/MigrationTest.php
index 6146e4e7cfb..be37020660c 100644
--- a/apps/encryption/tests/lib/MigrationTest.php
+++ b/apps/encryption/tests/lib/MigrationTest.php
@@ -62,6 +62,8 @@ class MigrationTest extends \Test\TestCase {
 	}
 
 	protected function createDummyShareKeys($uid) {
+		$this->loginAsUser($uid);
+
 		$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/folder3/file3');
 		$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/file2');
 		$this->view->mkdir($uid . '/files_encryption/keys/folder1/file.1');
@@ -87,6 +89,8 @@ class MigrationTest extends \Test\TestCase {
 	}
 
 	protected function createDummyUserKeys($uid) {
+		$this->loginAsUser($uid);
+
 		$this->view->mkdir($uid . '/files_encryption/');
 		$this->view->mkdir('/files_encryption/public_keys');
 		$this->view->file_put_contents($uid . '/files_encryption/' . $uid . '.privateKey', 'privateKey');
@@ -94,6 +98,8 @@ class MigrationTest extends \Test\TestCase {
 	}
 
 	protected function createDummyFileKeys($uid) {
+		$this->loginAsUser($uid);
+
 		$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/folder3/file3');
 		$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/file2');
 		$this->view->mkdir($uid . '/files_encryption/keys/folder1/file.1');
@@ -105,6 +111,8 @@ class MigrationTest extends \Test\TestCase {
 	}
 
 	protected function createDummyFiles($uid) {
+		$this->loginAsUser($uid);
+
 		$this->view->mkdir($uid . '/files/folder1/folder2/folder3/file3');
 		$this->view->mkdir($uid . '/files/folder1/folder2/file2');
 		$this->view->mkdir($uid . '/files/folder1/file.1');
@@ -116,6 +124,8 @@ class MigrationTest extends \Test\TestCase {
 	}
 
 	protected function createDummyFilesInTrash($uid) {
+		$this->loginAsUser($uid);
+
 		$this->view->mkdir($uid . '/files_trashbin/keys/file1.d5457864');
 		$this->view->mkdir($uid . '/files_trashbin/keys/folder1.d7437648723/file2');
 		$this->view->file_put_contents($uid . '/files_trashbin/keys/file1.d5457864/' . self::TEST_ENCRYPTION_MIGRATION_USER1 . '.shareKey' , 'data');
@@ -165,6 +175,7 @@ class MigrationTest extends \Test\TestCase {
 
 		$this->createDummySystemWideKeys();
 
+		/** @var \PHPUnit_Framework_MockObject_MockObject|\OCA\Encryption\Migration $m */
 		$m = $this->getMockBuilder('OCA\Encryption\Migration')
 			->setConstructorArgs(
 				[
@@ -176,27 +187,38 @@ class MigrationTest extends \Test\TestCase {
 			)->setMethods(['getSystemMountPoints'])->getMock();
 
 		$m->expects($this->any())->method('getSystemMountPoints')
-			->willReturn([['mountpoint' => 'folder1'], ['mountpoint' => 'folder2']]);
+			->will($this->returnValue([['mountpoint' => 'folder1'], ['mountpoint' => 'folder2']]));
 
 		$m->reorganizeFolderStructure();
 		// even if it runs twice folder should always move only once
 		$m->reorganizeFolderStructure();
 
+		$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER1);
+
 		$this->assertTrue(
 			$this->view->file_exists(
 				self::TEST_ENCRYPTION_MIGRATION_USER1 . '/files_encryption/' .
 				$this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER1 . '.publicKey')
 		);
+
+		$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER2);
+
 		$this->assertTrue(
 			$this->view->file_exists(
 				self::TEST_ENCRYPTION_MIGRATION_USER2 . '/files_encryption/' .
 				$this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER2 . '.publicKey')
 		);
+
+		$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER3);
+
 		$this->assertTrue(
 			$this->view->file_exists(
 				self::TEST_ENCRYPTION_MIGRATION_USER3 . '/files_encryption/' .
 				$this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER3 . '.publicKey')
 		);
+
+		$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER1);
+
 		$this->assertTrue(
 			$this->view->file_exists(
 			    '/files_encryption/' . $this->moduleId . '/systemwide_1.publicKey')
@@ -217,6 +239,8 @@ class MigrationTest extends \Test\TestCase {
 	}
 
 	protected function verifyFilesInTrash($uid) {
+		$this->loginAsUser($uid);
+
 		// share keys
 		$this->assertTrue(
 			$this->view->file_exists($uid . '/files_encryption/keys/files_trashbin/file1.d5457864/' . $this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER1 . '.shareKey')
@@ -244,6 +268,7 @@ class MigrationTest extends \Test\TestCase {
 	protected function verifyNewKeyPath($uid) {
 		// private key
 		if ($uid !== '') {
+			$this->loginAsUser($uid);
 			$this->assertTrue($this->view->file_exists($uid . '/files_encryption/' . $this->moduleId . '/'. $uid . '.privateKey'));
 		}
 		// file keys
diff --git a/apps/files/tests/command/deleteorphanedfilestest.php b/apps/files/tests/command/deleteorphanedfilestest.php
index 76fe9dbdfa0..3a1a541d8f1 100644
--- a/apps/files/tests/command/deleteorphanedfilestest.php
+++ b/apps/files/tests/command/deleteorphanedfilestest.php
@@ -22,6 +22,7 @@
 namespace OCA\Files\Tests\Command;
 
 use OCA\Files\Command\DeleteOrphanedFiles;
+use OCP\Files\StorageNotAvailableException;
 
 class DeleteOrphanedFilesTest extends \Test\TestCase {
 
@@ -110,7 +111,11 @@ class DeleteOrphanedFilesTest extends \Test\TestCase {
 
 		$this->assertCount(0, $this->getFile($fileInfo->getId()), 'Asserts that file gets cleaned up');
 
-		$view->unlink('files/test');
+		// since we deleted the storage it might throw a (valid) StorageNotAvailableException
+		try {
+			$view->unlink('files/test');
+		} catch (StorageNotAvailableException $e) {
+		}
 	}
 }
 
diff --git a/apps/files_sharing/tests/sharedstorage.php b/apps/files_sharing/tests/sharedstorage.php
index de510cf1eec..3361d2cbd12 100644
--- a/apps/files_sharing/tests/sharedstorage.php
+++ b/apps/files_sharing/tests/sharedstorage.php
@@ -47,8 +47,10 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase {
 	}
 
 	protected function tearDown() {
-		$this->view->unlink($this->folder);
-		$this->view->unlink($this->filename);
+		if ($this->view) {
+			$this->view->unlink($this->folder);
+			$this->view->unlink($this->filename);
+		}
 
 		\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
 
@@ -85,8 +87,9 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase {
 		$this->assertFalse($user2View->is_dir($this->folder));
 
 		// delete the local folder
-		$fullPath = \OC_Config::getValue('datadirectory') . '/' . self::TEST_FILES_SHARING_API_USER2 . '/files/localfolder';
-		rmdir($fullPath);
+		/** @var \OC\Files\Storage\Storage $storage */
+		list($storage, $internalPath)  = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/localfolder');
+		$storage->rmdir($internalPath);
 
 		//enforce reload of the mount points
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
diff --git a/apps/files_sharing/tests/watcher.php b/apps/files_sharing/tests/watcher.php
index 488792db4ef..5e96a3fe68e 100644
--- a/apps/files_sharing/tests/watcher.php
+++ b/apps/files_sharing/tests/watcher.php
@@ -108,9 +108,8 @@ class Test_Files_Sharing_Watcher extends OCA\Files_sharing\Tests\TestCase {
 		$this->sharedCache->put('', array('mtime' => 10, 'storage_mtime' => 10, 'size' => '-1', 'mimetype' => 'httpd/unix-directory'));
 
 		// run the propagation code
-		$result = $this->sharedStorage->getWatcher()->checkUpdate('');
-
-		$this->assertTrue($result);
+		$this->sharedStorage->getWatcher()->checkUpdate('');
+		$this->sharedStorage->getCache()->correctFolderSize('');
 
 		// the owner's parent dirs must have increase size
 		$newSizes = self::getOwnerDirSizes('files/container/shareddir');
@@ -139,9 +138,8 @@ class Test_Files_Sharing_Watcher extends OCA\Files_sharing\Tests\TestCase {
 		$this->sharedCache->put('subdir', array('mtime' => 10, 'storage_mtime' => 10, 'size' => $dataLen, 'mimetype' => 'text/plain'));
 
 		// run the propagation code
-		$result = $this->sharedStorage->getWatcher()->checkUpdate('subdir');
-
-		$this->assertTrue($result);
+		$this->sharedStorage->getWatcher()->checkUpdate('subdir');
+		$this->sharedStorage->getCache()->correctFolderSize('subdir');
 
 		// the owner's parent dirs must have increase size
 		$newSizes = self::getOwnerDirSizes('files/container/shareddir/subdir');
diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php
index ef015c3566a..839a47a7bf2 100644
--- a/apps/files_trashbin/lib/trashbin.php
+++ b/apps/files_trashbin/lib/trashbin.php
@@ -581,8 +581,9 @@ class Trashbin {
 		if ($quota === null || $quota === 'none') {
 			$quota = \OC\Files\Filesystem::free_space('/');
 			$softQuota = false;
-			if ($quota === \OCP\Files\FileInfo::SPACE_UNKNOWN) {
-				$quota = 0;
+			// inf or unknown free space
+			if ($quota < 0) {
+				$quota = PHP_INT_MAX;
 			}
 		} else {
 			$quota = \OCP\Util::computerFileSize($quota);
diff --git a/apps/files_trashbin/tests/trashbin.php b/apps/files_trashbin/tests/trashbin.php
index 757239b8cc4..e28b854ca1f 100644
--- a/apps/files_trashbin/tests/trashbin.php
+++ b/apps/files_trashbin/tests/trashbin.php
@@ -236,6 +236,8 @@ class Test_Trashbin extends \Test\TestCase {
 		// user2-1.txt should have been expired
 		$this->verifyArray($filesInTrashUser2AfterDelete, array('user2-2.txt', 'user1-4.txt'));
 
+		self::loginHelper(self::TEST_TRASHBIN_USER1);
+
 		// user1-1.txt and user1-3.txt should have been expired
 		$filesInTrashUser1AfterDelete = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1);
 
@@ -600,22 +602,24 @@ class Test_Trashbin extends \Test\TestCase {
 
 		// delete source folder
 		list($storage, $internalPath) = $this->rootView->resolvePath('/' . self::TEST_TRASHBIN_USER1 . '/files/folder');
-		$folderAbsPath = $storage->getSourcePath($internalPath);
-		// make folder read-only
-		chmod($folderAbsPath, 0555);
+		if ($storage instanceof \OC\Files\Storage\Local) {
+			$folderAbsPath = $storage->getSourcePath($internalPath);
+			// make folder read-only
+			chmod($folderAbsPath, 0555);
 
-		$this->assertTrue(
-			OCA\Files_Trashbin\Trashbin::restore(
-				'file1.txt.d' . $trashedFile->getMtime(),
-				$trashedFile->getName(),
-				$trashedFile->getMtime()
-			)
-		);
+			$this->assertTrue(
+				OCA\Files_Trashbin\Trashbin::restore(
+					'file1.txt.d' . $trashedFile->getMtime(),
+					$trashedFile->getName(),
+					$trashedFile->getMtime()
+				)
+			);
 
-		$file = $userFolder->get('file1.txt');
-		$this->assertEquals('foo', $file->getContent());
+			$file = $userFolder->get('file1.txt');
+			$this->assertEquals('foo', $file->getContent());
 
-		chmod($folderAbsPath, 0755);
+			chmod($folderAbsPath, 0755);
+		}
 	}
 
 	/**
diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php
index fd51a54b108..dd8af1b8d18 100644
--- a/apps/files_versions/lib/storage.php
+++ b/apps/files_versions/lib/storage.php
@@ -347,7 +347,20 @@ class Storage {
 		$view->lockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
 		$view->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
 
-		$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
+		// TODO add a proper way of overwriting a file while maintaining file ids
+		if ($storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage') || $storage2->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')) {
+			$source = $storage1->fopen($internalPath1, 'r');
+			$target = $storage2->fopen($internalPath2, 'w');
+			list(, $result) = \OC_Helper::streamCopy($source, $target);
+			fclose($source);
+			fclose($target);
+
+			if ($result !== false) {
+				$storage1->unlink($internalPath1);
+			}
+		} else {
+			$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
+		}
 
 		$view->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
 		$view->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
@@ -663,17 +676,21 @@ class Storage {
 
 			// calculate available space for version history
 			// subtract size of files and current versions size from quota
-			if ($softQuota) {
-				$files_view = new \OC\Files\View('/'.$uid.'/files');
-				$rootInfo = $files_view->getFileInfo('/', false);
-				$free = $quota-$rootInfo['size']; // remaining free space for user
-				if ( $free > 0 ) {
-					$availableSpace = ($free * self::DEFAULTMAXSIZE / 100) - ($versionsSize + $offset); // how much space can be used for versions
+			if ($quota >= 0) {
+				if ($softQuota) {
+					$files_view = new \OC\Files\View('/' . $uid . '/files');
+					$rootInfo = $files_view->getFileInfo('/', false);
+					$free = $quota - $rootInfo['size']; // remaining free space for user
+					if ($free > 0) {
+						$availableSpace = ($free * self::DEFAULTMAXSIZE / 100) - ($versionsSize + $offset); // how much space can be used for versions
+					} else {
+						$availableSpace = $free - $versionsSize - $offset;
+					}
 				} else {
-					$availableSpace = $free - $versionsSize - $offset;
+					$availableSpace = $quota - $offset;
 				}
 			} else {
-				$availableSpace = $quota - $offset;
+				$availableSpace = PHP_INT_MAX;
 			}
 
 			$allVersions = Storage::getVersions($uid, $filename);
diff --git a/apps/files_versions/tests/versions.php b/apps/files_versions/tests/versions.php
index 00d2b75b7a9..2979de2ac98 100644
--- a/apps/files_versions/tests/versions.php
+++ b/apps/files_versions/tests/versions.php
@@ -301,11 +301,10 @@ class Test_Files_Versioning extends \Test\TestCase {
 		// execute rename hook of versions app
 		\OC\Files\Filesystem::rename('/folder1/test.txt', '/folder1/folder2/test.txt');
 
-
-		self::loginHelper(self::TEST_VERSIONS_USER2);
-
 		$this->runCommands();
 
+		self::loginHelper(self::TEST_VERSIONS_USER);
+
 		$this->assertFalse($this->rootView->file_exists($v1));
 		$this->assertFalse($this->rootView->file_exists($v2));