[BUGFIX] Copy and move folders between storages is broken 94/19194/4
authorNicole Cordes <typo3@cordes.co>
Fri, 22 Mar 2013 17:23:39 +0000 (18:23 +0100)
committerAndreas Wolf <andreas.wolf@typo3.org>
Sat, 23 Mar 2013 11:21:54 +0000 (12:21 +0100)
If you try to copy a folder from one storage into another one it leads to
a copy in the same (source) storage. This happens because the action is
executed in the source storage and only works with the source folder
object. This is fixed by calling the move method on the target storage
instead.

Besides the copyFolderBetweenStorages function raises an exception which
might be moved into the driver class as the driver should be responsible
for the copy work. Therefore the AbstractDriver should support
(abstract) methods to copy a folder between storages and the local
driver should raises the exception.

Same applies for moving folders between storages.

Change-Id: Ib282e351f39abca21d57f5d621b4bd999f8419d9
Fixes: #46564
Releases: 6.1, 6.0
Reviewed-on: https://review.typo3.org/19194
Reviewed-by: Andreas Wolf
Tested-by: Andreas Wolf
typo3/sysext/core/Classes/Resource/Driver/AbstractDriver.php
typo3/sysext/core/Classes/Resource/Driver/LocalDriver.php
typo3/sysext/core/Classes/Resource/Folder.php
typo3/sysext/core/Classes/Resource/ResourceStorage.php
typo3/sysext/core/Classes/Utility/File/ExtendedFileUtility.php

index 416ef51..6dacc94 100644 (file)
@@ -524,12 +524,41 @@ abstract class AbstractDriver {
        /**
         * Folder equivalent to copyFileWithinStorage().
         *
-        * @param \TYPO3\CMS\Core\Resource\Folder $folderToMove
+        * @param \TYPO3\CMS\Core\Resource\Folder $folderToCopy
         * @param \TYPO3\CMS\Core\Resource\Folder $targetFolder
         * @param string $newFileName
         * @return boolean
         */
-       abstract public function copyFolderWithinStorage(\TYPO3\CMS\Core\Resource\Folder $folderToMove, \TYPO3\CMS\Core\Resource\Folder $targetFolder, $newFileName);
+       abstract public function copyFolderWithinStorage(\TYPO3\CMS\Core\Resource\Folder $folderToCopy, \TYPO3\CMS\Core\Resource\Folder $targetFolder, $newFileName);
+
+       /**
+        * Move a folder from another storage.
+        *
+        * @param \TYPO3\CMS\Core\Resource\Folder $folderToMove
+        * @param \TYPO3\CMS\Core\Resource\Folder $targetParentFolder
+        * @param string $newFolderName
+        * @throws \BadMethodCallException
+        * @return boolean
+        */
+       public function moveFolderBetweenStorages(\TYPO3\CMS\Core\Resource\Folder $folderToMove, \TYPO3\CMS\Core\Resource\Folder $targetParentFolder, $newFolderName) {
+               // This is not implemented for now as moving files between storages might cause quite some headaches when
+               // something goes wrong. It is also not that common of a use case, so it does not hurt that much to leave it out
+               // for now.
+               throw new \BadMethodCallException('Moving folders between storages is not implemented.');
+       }
+
+       /**
+        * Copy a folder from another storage.
+        *
+        * @param \TYPO3\CMS\Core\Resource\Folder $folderToCopy
+        * @param \TYPO3\CMS\Core\Resource\Folder $targetParentFolder
+        * @param string $newFolderName
+        * @throws \BadMethodCallException
+        * @return boolean
+        */
+       public function copyFolderBetweenStorages(\TYPO3\CMS\Core\Resource\Folder $folderToCopy, \TYPO3\CMS\Core\Resource\Folder $targetParentFolder, $newFolderName) {
+               throw new \BadMethodCallException('Not yet implemented!', 1330262731);
+       }
 
        /**
         * Removes a file from this storage. This does not check if the file is
index 4d8af56..72d0ef6 100644 (file)
@@ -860,6 +860,32 @@ class LocalDriver extends \TYPO3\CMS\Core\Resource\Driver\AbstractDriver {
        }
 
        /**
+        * Move a folder from another storage.
+        *
+        * @param \TYPO3\CMS\Core\Resource\Folder $folderToMove
+        * @param \TYPO3\CMS\Core\Resource\Folder $targetParentFolder
+        * @param string $newFolderName
+        * @return boolean
+        */
+       public function moveFolderBetweenStorages(\TYPO3\CMS\Core\Resource\Folder $folderToMove, \TYPO3\CMS\Core\Resource\Folder $targetParentFolder, $newFolderName) {
+               // TODO implement a clever shortcut here if both storages are of type local
+               return parent::moveFolderBetweenStorages($folderToMove, $targetParentFolder, $newFolderName);
+       }
+
+       /**
+        * Copy a folder from another storage.
+        *
+        * @param \TYPO3\CMS\Core\Resource\Folder $folderToCopy
+        * @param \TYPO3\CMS\Core\Resource\Folder $targetParentFolder
+        * @param string $newFolderName
+        * @return boolean
+        */
+       public function copyFolderBetweenStorages(\TYPO3\CMS\Core\Resource\Folder $folderToCopy, \TYPO3\CMS\Core\Resource\Folder $targetParentFolder, $newFolderName) {
+               // TODO implement a clever shortcut here if both storages are of type local
+               return parent::copyFolderBetweenStorages($folderToCopy, $targetParentFolder, $newFolderName);
+       }
+
+       /**
         * Renames a file in this storage.
         *
         * @param \TYPO3\CMS\Core\Resource\FileInterface $file
index 1ac0a46..b4e4b02 100644 (file)
@@ -340,7 +340,7 @@ class Folder implements FolderInterface {
         * @return Folder New (copied) folder object.
         */
        public function copyTo(Folder $targetFolder, $targetFolderName = NULL, $conflictMode = 'renameNewFile') {
-               return $this->storage->copyFolder($this, $targetFolder, $targetFolderName, $conflictMode);
+               return $targetFolder->getStorage()->copyFolder($this, $targetFolder, $targetFolderName, $conflictMode);
        }
 
        /**
@@ -352,7 +352,7 @@ class Folder implements FolderInterface {
         * @return Folder New (copied) folder object.
         */
        public function moveTo(Folder $targetFolder, $targetFolderName = NULL, $conflictMode = 'renameNewFile') {
-               return $this->storage->moveFolder($this, $targetFolder, $targetFolderName, $conflictMode);
+               return $targetFolder->getStorage()->moveFolder($this, $targetFolder, $targetFolderName, $conflictMode);
        }
 
        /**
index 1113fc4..63aedac 100644 (file)
@@ -1391,14 +1391,10 @@ class ResourceStorage {
         * @param Folder $targetParentFolder
         * @param string $newFolderName
         *
-        * @throws \BadMethodCallException
-        * @return array Mapping of old file identifiers to new ones
+        * @return boolean
         */
-       protected function moveFolderBetweenStorages(Folder $folderToMove, Folder $targetParentFolder, $newFolderName = NULL) {
-               // This is not implemented for now as moving files between storages might cause quite some headaches when
-               // something goes wrong. It is also not that common of a use case, so it does not hurt that much to leave it out
-               // for now.
-               throw new \BadMethodCallException('Moving folders between storages is not implemented.');
+       protected function moveFolderBetweenStorages(Folder $folderToMove, Folder $targetParentFolder, $newFolderName) {
+               return $this->getDriver()->moveFolderBetweenStorages($folderToMove, $targetParentFolder, $newFolderName);
        }
 
        /**
@@ -1434,17 +1430,16 @@ class ResourceStorage {
        }
 
        /**
-        * Moves files between storages
+        * Copy folders between storages
         *
-        * @param Folder $folderToMove
+        * @param Folder $folderToCopy
         * @param Folder $targetParentFolder
-        * @param null $newFolderName
+        * @param string $newFolderName
         *
-        * @throws \RuntimeException
-        * @return void
+        * @return boolean
         */
-       protected function copyFolderBetweenStorages(Folder $folderToMove, Folder $targetParentFolder, $newFolderName = NULL) {
-               throw new \RuntimeException('Not yet implemented!', 1330262731);
+       protected function copyFolderBetweenStorages(Folder $folderToCopy, Folder $targetParentFolder, $newFolderName) {
+               return $this->getDriver()->copyFolderBetweenStorages($folderToCopy, $targetParentFolder, $newFolderName);
        }
 
        /**
index c4e20c3..ed9e269 100644 (file)
@@ -508,6 +508,8 @@ class ExtendedFileUtility extends \TYPO3\CMS\Core\Utility\File\BasicFileUtility
                                $this->writelog(2, 1, 111, 'Extension of file name "%s" is not allowed in "%s"!', array($sourceFileObject->getIdentifier(), $targetFolderObject->getIdentifier()));
                        } catch (\TYPO3\CMS\Core\Resource\Exception\ExistingTargetFileNameException $e) {
                                $this->writelog(2, 1, 112, 'File "%s" already exists in folder "%s"!', array($sourceFileObject->getIdentifier(), $targetFolderObject->getIdentifier()));
+                       } catch (\BadMethodCallException $e) {
+                               $this->writelog(3, 1, 128, 'The function to copy a file between storages is not yet implemented', array());
                        } catch (\RuntimeException $e) {
                                $this->writelog(2, 2, 109, 'File "%s" WAS NOT copied to "%s"! Write-permission problem?', array($sourceFileObject->getIdentifier(), $targetFolderObject->getIdentifier()));
                        }
@@ -530,6 +532,8 @@ class ExtendedFileUtility extends \TYPO3\CMS\Core\Utility\File\BasicFileUtility
                                $this->writelog(2, 1, 122, 'Destination cannot be inside the target! D="%s", T="%s"', array($targetFolderObject->getIdentifier(), $sourceFolderObject->getIdentifier()));
                        } catch (\TYPO3\CMS\Core\Resource\Exception\ExistingTargetFolderException $e) {
                                $this->writelog(2, 1, 123, 'Target "%s" already exists!', array($targetFolderObject->getIdentifier()));
+                       } catch (\BadMethodCallException $e) {
+                               $this->writelog(3, 1, 129, 'The function to copy a folder between storages is not yet implemented', array());
                        } catch (\RuntimeException $e) {
                                $this->writelog(2, 2, 119, 'Directory "%s" WAS NOT copied to "%s"! Write-permission problem?', array($sourceFolderObject->getIdentifier(), $targetFolderObject->getIdentifier()));
                        }
@@ -584,6 +588,8 @@ class ExtendedFileUtility extends \TYPO3\CMS\Core\Utility\File\BasicFileUtility
                                $this->writelog(3, 1, 111, 'Extension of file name "%s" is not allowed in "%s"!', array($sourceFileObject->getIdentifier(), $targetFolderObject->getIdentifier()));
                        } catch (\TYPO3\CMS\Core\Resource\Exception\ExistingTargetFileNameException $e) {
                                $this->writelog(3, 1, 112, 'File "%s" already exists in folder "%s"!', array($sourceFileObject->getIdentifier(), $targetFolderObject->getIdentifier()));
+                       } catch (\BadMethodCallException $e) {
+                               $this->writelog(3, 1, 126, 'The function to move a file between storages is not yet implemented', array());
                        } catch (\RuntimeException $e) {
                                $this->writelog(3, 2, 109, 'File "%s" WAS NOT copied to "%s"! Write-permission problem?', array($sourceFileObject->getIdentifier(), $targetFolderObject->getIdentifier()));
                        }
@@ -609,6 +615,8 @@ class ExtendedFileUtility extends \TYPO3\CMS\Core\Utility\File\BasicFileUtility
                                $this->writelog(3, 1, 122, 'Destination cannot be inside the target! D="%s", T="%s"', array($targetFolderObject->getIdentifier(), $sourceFolderObject->getIdentifier()));
                        } catch (\TYPO3\CMS\Core\Resource\Exception\ExistingTargetFolderException $e) {
                                $this->writelog(3, 1, 123, 'Target "%s" already exists!', array($targetFolderObject->getIdentifier()));
+                       } catch (\BadMethodCallException $e) {
+                               $this->writelog(3, 1, 127, 'The function to move a folder between storages is not yet implemented', array());
                        } catch (\RuntimeException $e) {
                                $this->writelog(3, 2, 119, 'Directory "%s" WAS NOT moved to "%s"! Write-permission problem?', array($sourceFolderObject->getIdentifier(), $targetFolderObject->getIdentifier()));
                        }