disable trasbin during the moveFromStorage fallback

Signed-off-by: Robin Appelman <robin@icewind.nl>

diff --git a/apps/files_trashbin/lib/Storage.php b/apps/files_trashbin/lib/Storage.php
index 936e6d646751c288022b6157d711c70396bd16ce..08dc052fa5cb8e8212e15d8b55a7123af9892d72 100644 (file)
--- a/apps/files_trashbin/lib/Storage.php
+++ b/apps/files_trashbin/lib/Storage.php
@@ -37,6 +37,7 @@ use OCP\Encryption\Exceptions\GenericEncryptionException;
 use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Node;
+use OCP\Files\Storage\IStorage;
 use OCP\ILogger;
 use OCP\IUserManager;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -60,6 +61,8 @@ class Storage extends Wrapper {
        /** @var ITrashManager */
        private $trashManager;
 
+       private $trashEnabled = true;
+
        /**
         * Storage constructor.
         *
@@ -95,14 +98,18 @@ class Storage extends Wrapper {
         * @return bool true if the operation succeeded, false otherwise
         */
        public function unlink($path) {
-               try {
-                       return $this->doDelete($path, 'unlink');
-               } catch (GenericEncryptionException $e) {
-                       // in case of a encryption exception we delete the file right away
-                       $this->logger->info(
-                               "Can't move file " . $path .
-                               " to the trash bin, therefore it was deleted right away");
+               if ($this->trashEnabled) {
+                       try {
+                               return $this->doDelete($path, 'unlink');
+                       } catch (GenericEncryptionException $e) {
+                               // in case of a encryption exception we delete the file right away
+                               $this->logger->info(
+                                       "Can't move file " . $path .
+                                       " to the trash bin, therefore it was deleted right away");
 
+                               return $this->storage->unlink($path);
+                       }
+               } else {
                        return $this->storage->unlink($path);
                }
        }
@@ -115,7 +122,11 @@ class Storage extends Wrapper {
         * @return bool true if the operation succeeded, false otherwise
         */
        public function rmdir($path) {
-               return $this->doDelete($path, 'rmdir');
+               if ($this->trashEnabled) {
+                       return $this->doDelete($path, 'rmdir');
+               } else {
+                       return $this->storage->rmdir($path);
+               }
        }
 
        /**
@@ -216,4 +227,36 @@ class Storage extends Wrapper {
        public function getMountPoint() {
                return $this->mountPoint;
        }
+
+       public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) {
+               $sourceIsTrashbin = $sourceStorage->instanceOfStorage(Storage::class);
+               try {
+                       // the fallback for moving between storage involves a copy+delete
+                       // we don't want to trigger the trashbin when doing the delete
+                       if ($sourceIsTrashbin) {
+                               /** @var Storage $sourceStorage */
+                               $sourceStorage->disableTrash();
+                       }
+                       $result = $this->getWrapperStorage()->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath);
+                       if ($sourceIsTrashbin) {
+                               /** @var Storage $sourceStorage */
+                               $sourceStorage->enableTrash();
+                       }
+                       return $result;
+               } catch (\Exception $e) {
+                       if ($sourceIsTrashbin) {
+                               /** @var Storage $sourceStorage */
+                               $sourceStorage->enableTrash();
+                       }
+                       throw $e;
+               }
+       }
+
+       protected function disableTrash() {
+               $this->trashEnabled = false;
+       }
+
+       protected function enableTrash() {
+               $this->trashEnabled = true;
+       }
 }

diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php
index f86e0d12a956255b517c0d9dc9079cbc3cb71ff5..53c1a09cc392890afc927d688485e24ab37e8776 100644 (file)
--- a/apps/files_trashbin/tests/StorageTest.php
+++ b/apps/files_trashbin/tests/StorageTest.php
@@ -33,6 +33,7 @@
 namespace OCA\Files_Trashbin\Tests;
 
 use OC\Files\Filesystem;
+use OC\Files\Storage\Common;
 use OC\Files\Storage\Temporary;
 use OCA\Files_Trashbin\AppInfo\Application;
 use OCA\Files_Trashbin\Events\MoveToTrashEvent;
@@ -44,11 +45,23 @@ use OCP\Files\Cache\ICache;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
+use OCP\Files\Storage\IStorage;
 use OCP\ILogger;
 use OCP\IUserManager;
 use OCP\Lock\ILockingProvider;
 use OCP\Share\IShare;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Test\Traits\MountProviderTrait;
+
+class TemporaryNoCross extends Temporary {
+       public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = null) {
+               return Common::copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime);
+       }
+
+       public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) {
+               return Common::moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath);
+       }
+}
 
 /**
  * Class Storage
@@ -58,6 +71,8 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface;
  * @package OCA\Files_Trashbin\Tests
  */
 class StorageTest extends \Test\TestCase {
+       use MountProviderTrait;
+
        /**
         * @var string
         */
@@ -645,4 +660,18 @@ class StorageTest extends \Test\TestCase {
                $this->assertEquals('foo', $this->rootView->file_get_contents($this->user . '/files_trashbin/files/test.txt.d1000'));
                $this->assertEquals('bar', $this->rootView->file_get_contents($this->user . '/files_trashbin/files/test.txt.d1001'));
        }
+
+       public function testMoveFromStoragePreserveFileId() {
+               $this->userView->file_put_contents('test.txt', 'foo');
+               $fileId = $this->userView->getFileInfo('test.txt')->getId();
+
+               $externalStorage = new TemporaryNoCross([]);
+               $externalStorage->getScanner()->scan('');
+               Filesystem::mount($externalStorage, [], "/" . $this->user . "/files/storage");
+
+               $this->assertTrue($this->userView->rename('test.txt', 'storage/test.txt'));
+               $this->assertTrue($externalStorage->file_exists('test.txt'));
+
+               $this->assertEquals($fileId, $this->userView->getFileInfo('storage/test.txt')->getId());
+       }
 }