diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index f73cc1f4aa..db6fa35e85 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -44,6 +44,7 @@ namespace OCA\Files_Trashbin; use OC\Files\Filesystem; +use OC\Files\ObjectStore\ObjectStoreStorage; use OC\Files\View; use OCA\Files_Trashbin\AppInfo\Application; use OCA\Files_Trashbin\Command\Expire; @@ -278,12 +279,22 @@ class Trashbin { /** @var \OC\Files\Storage\Storage $sourceStorage */ [$sourceStorage, $sourceInternalPath] = $ownerView->resolvePath('/files/' . $ownerPath); + + if ($trashStorage->file_exists($trashInternalPath)) { + $trashStorage->unlink($trashInternalPath); + } + + $connection = \OC::$server->getDatabaseConnection(); + $connection->beginTransaction(); + $trashStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath); + try { $moveSuccessful = true; - if ($trashStorage->file_exists($trashInternalPath)) { - $trashStorage->unlink($trashInternalPath); + + // when moving within the same object store, the cache update done above is enough to move the file + if (!($trashStorage->instanceOfStorage(ObjectStoreStorage::class) && $trashStorage->getId() === $sourceStorage->getId())) { + $trashStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath); } - $trashStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath); } catch (\OCA\Files_Trashbin\Exceptions\CopyRecursiveException $e) { $moveSuccessful = false; if ($trashStorage->file_exists($trashInternalPath)) { @@ -298,10 +309,11 @@ class Trashbin { } else { $sourceStorage->unlink($sourceInternalPath); } + $connection->rollBack(); return false; } - $trashStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath); + $connection->commit(); if ($moveSuccessful) { $query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)"); diff --git a/lib/private/Files/Cache/Updater.php b/lib/private/Files/Cache/Updater.php index 60534a153d..79501d910e 100644 --- a/lib/private/Files/Cache/Updater.php +++ b/lib/private/Files/Cache/Updater.php @@ -28,6 +28,7 @@ namespace OC\Files\Cache; +use OC\Files\FileInfo; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Cache\IUpdater; use OCP\Files\Storage\IStorage; @@ -187,7 +188,9 @@ class Updater implements IUpdater { $sourceUpdater = $sourceStorage->getUpdater(); $sourcePropagator = $sourceStorage->getPropagator(); - if ($sourceCache->inCache($source)) { + $sourceInfo = $sourceCache->get($source); + + if ($sourceInfo !== false) { if ($this->cache->inCache($target)) { $this->cache->remove($target); } @@ -197,13 +200,17 @@ class Updater implements IUpdater { } else { $this->cache->moveFromCache($sourceCache, $source, $target); } - } - if (pathinfo($source, PATHINFO_EXTENSION) !== pathinfo($target, PATHINFO_EXTENSION)) { - // handle mime type change - $mimeType = $this->storage->getMimeType($target); - $fileId = $this->cache->getId($target); - $this->cache->update($fileId, ['mimetype' => $mimeType]); + $sourceExtension = pathinfo($source, PATHINFO_EXTENSION); + $targetExtension = pathinfo($target, PATHINFO_EXTENSION); + $targetIsTrash = preg_match("/d\d+/", $targetExtension); + + if ($sourceExtension !== $targetExtension && $sourceInfo->getMimeType() !== FileInfo::MIMETYPE_FOLDER && !$targetIsTrash) { + // handle mime type change + $mimeType = $this->storage->getMimeType($target); + $fileId = $this->cache->getId($target); + $this->cache->update($fileId, ['mimetype' => $mimeType]); + } } if ($sourceCache instanceof Cache) { diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php index 93e13937f5..2d03598141 100644 --- a/lib/private/Files/Storage/Common.php +++ b/lib/private/Files/Storage/Common.php @@ -53,6 +53,7 @@ use OC\Files\Storage\Wrapper\Jail; use OC\Files\Storage\Wrapper\Wrapper; use OCP\Files\EmptyFileNameException; use OCP\Files\FileNameTooLongException; +use OCP\Files\GenericFileException; use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidDirectoryException; use OCP\Files\InvalidPathException; @@ -620,18 +621,19 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage { } } else { $source = $sourceStorage->fopen($sourceInternalPath, 'r'); - // TODO: call fopen in a way that we execute again all storage wrappers - // to avoid that we bypass storage wrappers which perform important actions - // for this operation. Same is true for all other operations which - // are not the same as the original one.Once this is fixed we also - // need to adjust the encryption wrapper. - $target = $this->fopen($targetInternalPath, 'w'); - [, $result] = \OC_Helper::streamCopy($source, $target); + $result = false; + if ($source) { + try { + $this->writeStream($targetInternalPath, $source); + $result = true; + } catch (\Exception $e) { + \OC::$server->getLogger()->logException($e, ['level' => ILogger::WARN, 'message' => 'Failed to copy stream to storage']); + } + } + if ($result and $preserveMtime) { $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath)); } - fclose($source); - fclose($target); if (!$result) { // delete partially written target file @@ -858,10 +860,13 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage { public function writeStream(string $path, $stream, int $size = null): int { $target = $this->fopen($path, 'w'); if (!$target) { - return 0; + throw new GenericFileException("Failed to open $path for writing"); } try { [$count, $result] = \OC_Helper::streamCopy($stream, $target); + if (!$result) { + throw new GenericFileException("Failed to copy stream"); + } } finally { fclose($target); fclose($stream); diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php index 6d350512b2..552e996b90 100644 --- a/lib/private/Files/Storage/Local.php +++ b/lib/private/Files/Storage/Local.php @@ -44,6 +44,7 @@ use OC\Files\Filesystem; use OC\Files\Storage\Wrapper\Jail; use OCP\Constants; use OCP\Files\ForbiddenException; +use OCP\Files\GenericFileException; use OCP\Files\Storage\IStorage; use OCP\ILogger; @@ -555,6 +556,11 @@ class Local extends \OC\Files\Storage\Common { } public function writeStream(string $path, $stream, int $size = null): int { - return (int)file_put_contents($this->getSourcePath($path), $stream); + $result = file_put_contents($this->getSourcePath($path), $stream); + if ($result === false) { + throw new GenericFileException("Failed write steam to $path"); + } else { + return $result; + } } } diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 9dbe2a4e10..e69e49f8c8 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -13,7 +13,6 @@ use OC\Files\Filesystem; use OC\Files\Mount\MountPoint; use OC\Files\Storage\Common; use OC\Files\Storage\Temporary; -use OC\Files\Stream\Quota; use OC\Files\View; use OCP\Constants; use OCP\Files\Config\IMountProvider; @@ -1167,13 +1166,8 @@ class ViewTest extends \Test\TestCase { ->setMethods(['fopen']) ->getMock(); - $storage2->expects($this->any()) - ->method('fopen') - ->willReturnCallback(function ($path, $mode) use ($storage2) { - /** @var \PHPUnit_Framework_MockObject_MockObject | \OC\Files\Storage\Temporary $storage2 */ - $source = fopen($storage2->getSourcePath($path), $mode); - return Quota::wrap($source, 9); - }); + $storage2->method('writeStream') + ->willReturn(0); $storage1->mkdir('sub'); $storage1->file_put_contents('foo.txt', '0123456789ABCDEFGH');