ubuntu-touch-coreapps-reviewers team mailing list archive

[Carlos Jose Mazieri <carlos.mazieri@xxxxxxxxx>]

Carlos Jose Mazieri has proposed merging lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-13 into lp:ubuntu-filemanager-app with lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-12 as a prerequisite.

Commit message:
Some DirItemInfo objects created by targetLocation in Actions
QFile is no longer used in Actions, instead the abstract LocationItemFile is created by the suitable Location object

Requested reviews:
  Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot): continuous-integration
  Ubuntu File Manager Developers (ubuntu-filemanager-dev)

For more details, see:
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-13/+merge/265390

All QFile usage replaced by LocationItemFile.

* MP resubmitted to force a new build.
-- 
Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-13 into lp:ubuntu-filemanager-app.

=== modified file 'src/plugin/folderlistmodel/filesystemaction.cpp'
--- src/plugin/folderlistmodel/filesystemaction.cpp	2015-07-21 12:21:26 +0000
+++ src/plugin/folderlistmodel/filesystemaction.cpp	2015-07-21 12:21:26 +0000
@@ -40,7 +40,7 @@
 #include "location.h"
 #include "locationsfactory.h"
 #include "locationitemdiriterator.h"
-
+#include "locationitemfile.h"
 
 #if defined(Q_OS_UNIX)
 #include <sys/statvfs.h>
@@ -669,7 +669,9 @@
         }
         else
         {
-            m_cancelCurrentAction = !QFile::remove(fi.absoluteFilePath());
+            LocationItemFile *qFile = m_curAction->sourceLocation->newFile(fi.absoluteFilePath());
+            m_cancelCurrentAction = !qFile->remove();
+            delete qFile;
         }
 #if DEBUG_REMOVE
         qDebug() << Q_FUNC_INFO << "remove ret=" << !m_cancelCurrentAction << fi.absoluteFilePath();
@@ -743,14 +745,17 @@
         const DirItemInfo &fi = entry->reversedOrder.at(entry->currItem);
         QString orig    = fi.absoluteFilePath();
         QString target = targetFrom(orig, entry);
+#if DEBUG_MESSAGES
+        qDebug() << "orig:" << orig << "target:" << target;
+#endif
         QString path(target);
         // do this here to allow progress send right item number, copySingleFile will emit progress()
         m_curAction->currItem++;
         //--
         if (fi.isFile() || fi.isSymLink())
         {
-            DirItemInfo  t(target);
-            path = t.path();
+            QScopedPointer <DirItemInfo> t(m_curAction->targetLocation->newItemInfo(target));
+            path = t->path();            
         }
         //check if the main item in the entry is a directory
         //if so it needs to appear on any attached view
@@ -763,7 +768,7 @@
             QDir entryDirObj(entryDir);
             if (!entryDirObj.exists() && entryDirObj.mkpath(entryDir))
             {
-                QScopedPointer <DirItemInfo> item(m_locationsFactory->currentLocation()->newItemInfo(entryDir));
+                QScopedPointer <DirItemInfo> item(m_curAction->targetLocation->newItemInfo(entryDir));
                 entry->added = true;
                 notifyActionOnItem(*item, ItemAdded);
             }
@@ -789,8 +794,10 @@
         else
         if (fi.isDir())
         {
+            LocationItemFile *qFile = m_curAction->targetLocation->newFile(target);
             m_cancelCurrentAction = !
-                 QFile(target).setPermissions(fi.permissions());
+                 qFile->setPermissions(fi.permissions());
+            delete qFile;
             if (m_cancelCurrentAction)
             {
                 m_errorTitle = QObject::tr("Could not set permissions to dir");
@@ -803,7 +810,7 @@
         {
             qint64 needsSize = 0;
             m_curAction->copyFile.clear();
-            m_curAction->copyFile.source = new QFile(orig);
+            m_curAction->copyFile.source = m_curAction->sourceLocation->newFile(orig);
             m_cancelCurrentAction = !m_curAction->copyFile.source->open(QFile::ReadOnly);
             if (m_cancelCurrentAction)
             {               
@@ -814,7 +821,7 @@
             {
                 needsSize = m_curAction->copyFile.source->size();
                 //create destination
-                m_curAction->copyFile.target = new QFile(target);             
+                m_curAction->copyFile.target = m_curAction->targetLocation->newFile(target);
                 m_curAction->copyFile.targetName = target;
                 //first open it read-only to get its size if exists
                 if (m_curAction->copyFile.target->open(QFile::ReadOnly))
@@ -823,7 +830,7 @@
                     m_curAction->copyFile.target->close();
                 }
                 //check if there is disk space to copy source to target               
-                if (needsSize > 0 && !m_locationsFactory->currentLocation()->isThereDiskSpace(entry->itemPaths.targetPath(), needsSize))
+                if (needsSize > 0 && !m_curAction->targetLocation->isThereDiskSpace(entry->itemPaths.targetPath(), needsSize))
                 {
                     m_cancelCurrentAction = true;
                     m_errorTitle = QObject::tr("There is no space to copy");
@@ -848,7 +855,7 @@
                 //depending on the file size it may take longer, the view needs to be informed
                 if (m_curAction->copyFile.isEntryItem && !m_cancelCurrentAction)
                 {
-                    QScopedPointer <DirItemInfo> item(m_locationsFactory->currentLocation()->newItemInfo(target));
+                    QScopedPointer <DirItemInfo> item(m_curAction->targetLocation->newItemInfo(target));
                     if (!entry->alreadyExists)
                     {                       
                        entry->added = true;
@@ -877,9 +884,7 @@
  * \param entry
  */
 void FileSystemAction::moveEntry(ActionEntry *entry)
-{
-    QFile file;
-
+{   
     for(; !m_cancelCurrentAction                          &&
           entry->currStep       < STEP_FILES              &&
           m_curAction->currItem < m_curAction->totalItems &&
@@ -889,36 +894,38 @@
 
     {
         const DirItemInfo &fi = entry->reversedOrder.at(entry->currItem);
-        file.setFileName(fi.absoluteFilePath());
-        DirItemInfo targetInfo(entry->itemPaths.target());
+        QScopedPointer<LocationItemFile> file(m_curAction->sourceLocation->newFile(fi.absoluteFilePath()));
+        QScopedPointer<DirItemInfo> targetInfo(m_curAction->targetLocation->newItemInfo(entry->itemPaths.target()));
         //rename will fail
-        if (targetInfo.exists())
+        if (targetInfo->exists())
         {
             //will not emit removed() neither added()
             entry->added = true;
-            if (targetInfo.isFile() || targetInfo.isSymLink())
+            if (targetInfo->isFile() || targetInfo->isSymLink())
             {
-                if (!QFile::remove(targetInfo.absoluteFilePath()))
+                QScopedPointer<LocationItemFile>
+                               targetFile(m_curAction->sourceLocation->newFile(targetInfo->absoluteFilePath()));
+                if (!targetFile->remove())
                 {
                     m_cancelCurrentAction = true;
                     m_errorTitle = QObject::tr("Could not remove the directory/file ") +
-                                      targetInfo.absoluteFilePath();
+                                      targetInfo->absoluteFilePath();
                     m_errorMsg   = ::strerror(errno);
                 }
             }
-            else
-            if (targetInfo.isDir())
+            else                      //only for local disk operations
+            if (targetInfo->isDir() && !m_curAction->isRemote())
             {
                //move target to /tmp and remove it later by creating an Remove action
                //this will emit removed()
-               moveDirToTempAndRemoveItLater(targetInfo.absoluteFilePath());
+               moveDirToTempAndRemoveItLater(targetInfo->absoluteFilePath());
             }
         }
-        if (!m_cancelCurrentAction && !file.rename(entry->itemPaths.target()))
+        if (!m_cancelCurrentAction && !file->rename(entry->itemPaths.target()))
         {
             m_cancelCurrentAction = true;
             m_errorTitle = QObject::tr("Could not move the directory/file ") +
-                                     targetInfo.absoluteFilePath();
+                                     targetInfo->absoluteFilePath();
             m_errorMsg   = ::strerror(errno);
         }
     }//for
@@ -1386,7 +1393,10 @@
 #if defined(DEBUG_MESSAGES) || defined(REGRESSION_TEST_FOLDERLISTMODEL)
     qDebug() << Q_FUNC_INFO << dir <<  "being moved to" << tempDir;
 #endif
-    if (QFile::rename(dir, tempDir))
+    LocationItemFile *qFile = m_curAction->targetLocation->newFile(dir);
+    bool removed = qFile->rename(tempDir);
+    delete qFile;
+    if (removed)
     {
         if (m_curAction->auxAction == 0)
         {   // this new action as Remove will remove all dirs
@@ -1424,7 +1434,7 @@
     {
         const DirItemInfo& fi =
               entry->reversedOrder.at(entry->reversedOrder.count() -1);
-        DirItemInfo backuped;
+        QScopedPointer<DirItemInfo> backuped(m_curAction->targetLocation->newItemInfo(QLatin1String(0)));
         int counter=0;
         QString name;
         do
@@ -1447,12 +1457,12 @@
                 }
             }
             name.insert(pos,copy);
-            backuped.setFile(fi.absolutePath(), name);
-        } while (backuped.exists() && counter < 100);
+            backuped->setFile(fi.absolutePath(), name);
+        } while (backuped->exists() && counter < 100);
         if (counter < 100)
         {
-            entry->newName = new QString(backuped.fileName());
-            entry->itemPaths.setTargetFullName( backuped.absoluteFilePath() );
+            entry->newName = new QString(backuped->fileName());
+            entry->itemPaths.setTargetFullName( backuped->absoluteFilePath() );
             ret = true;
         }
     }

=== modified file 'src/plugin/folderlistmodel/filesystemaction.h'
--- src/plugin/folderlistmodel/filesystemaction.h	2015-07-21 12:21:26 +0000
+++ src/plugin/folderlistmodel/filesystemaction.h	2015-07-21 12:21:26 +0000
@@ -49,7 +49,7 @@
 #define AMOUNT_COPIED_TO_REFRESH_ITEM_INFO  50000000
 
 class DirModelMimeData;
-class QFile;
+class LocationItemFile;
 class QTemporaryFile;
 class Location;
 class LocationsFactory;
@@ -157,12 +157,12 @@
        CopyFile();
        ~CopyFile();
        void clear();
-       qint64            bytesWritten;           // set 0 when reach  bytesToNotify, notify progress
-       QFile          *  source;
-       QFile          *  target;
-       QString           targetName;
-       bool              isEntryItem;  //true when the file being copied is at toplevel of the copy/cut operation
-       qint64            amountSavedToRefresh;
+       qint64             bytesWritten;           // set 0 when reach  bytesToNotify, notify progress
+       LocationItemFile * source;
+       LocationItemFile * target;
+       QString            targetName;
+       bool               isEntryItem;  //true when the file being copied is at toplevel of the copy/cut operation
+       qint64             amountSavedToRefresh;
    };
 
    /*!