← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

[Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-02 into lp:ubuntu-filemanager-app

 

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

Commit message:
Enumerator Locations moved from class LocationsFactory to class Location
Created Location::isThereDiskSpace() which by default returns true, all locations can reimplement this method.
Created Location::isRemote(), Location::isLocalDisk() and Location::isTrashDisk() to handle more locations

Requested reviews:
  Ubuntu File Manager Developers (ubuntu-filemanager-dev)

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

Location class is improved:
   * Some methods from FileSystemAction will migrate to Location class
   * Added methods to identify if a Location object is Local/Remote
-- 
Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-02 into lp:ubuntu-filemanager-app.
=== modified file 'src/plugin/folderlistmodel/diriteminfo.cpp'
--- src/plugin/folderlistmodel/diriteminfo.cpp	2015-06-28 18:07:28 +0000
+++ src/plugin/folderlistmodel/diriteminfo.cpp	2015-07-18 21:23:48 +0000
@@ -457,15 +457,15 @@
     //owner permissions
     if (statBuffer.st_mode & S_IRUSR)
     {
-        readPermission  |= QFile::ReadOwner;
+        readPermission  |= QFile::ReadOwner |  QFile::ReadUser;
     }
     if (statBuffer.st_mode & S_IWUSR)
     {
-        writePermission |= QFile::WriteOwner;
+        writePermission |= QFile::WriteOwner | QFile::WriteUser;
     }
     if (statBuffer.st_mode & S_IXUSR)
     {
-        execPermission  |= QFile::ExeOwner;
+        execPermission  |= QFile::ExeOwner |  QFile::ExeUser;
     }
     //group permissions
     if (statBuffer.st_mode & S_IRGRP)

=== modified file 'src/plugin/folderlistmodel/dirmodel.cpp'
--- src/plugin/folderlistmodel/dirmodel.cpp	2015-07-18 21:23:48 +0000
+++ src/plugin/folderlistmodel/dirmodel.cpp	2015-07-18 21:23:48 +0000
@@ -76,7 +76,7 @@
 
 #define IS_FILE_MANAGER_IDLE()            (!mAwaitingResults)
 
-#define IS_BROWSING_TRASH_ROOTDIR() (mCurLocation && mCurLocation->type() == LocationsFactory::TrashDisk && mCurLocation->isRoot())
+#define IS_BROWSING_TRASH_ROOTDIR() (mCurLocation && mCurLocation->isTrashDisk() && mCurLocation->isRoot())
 
 namespace {
     QHash<QByteArray, int> roleMapping;
@@ -412,7 +412,7 @@
             return fi.isBrowsable();
         case IsSharingAllowedRole:
             return     fi.isDir() && !fi.isSymLink() && !fi.isSharedDir()
-                    && mCurLocation->type() == LocationsFactory::LocalDisk
+                    && mCurLocation->isLocalDisk()
                     && fi.isWritable() && fi.isExecutable() && fi.isReadable();
         case IsSharedDirRole:
             return fi.isSharedDir();
@@ -591,7 +591,7 @@
     }
 
     //if current location is Trash only in the root is allowed to remove Items
-    if (mCurLocation->type() == LocationsFactory::TrashDisk)
+    if (mCurLocation->isTrashDisk())
     {
         if (IS_BROWSING_TRASH_ROOTDIR())
         {
@@ -788,8 +788,8 @@
 }
 ExternalFSWatcher * DirModel::getExternalFSWatcher() const
 {
-   const Location *l = mLocationFactory->availableLocations().at(LocationsFactory::LocalDisk);
-   const DiskLocation *disk = static_cast<const DiskLocation*> (l);
+   Location *l = mLocationFactory->getDiskLocation();
+   DiskLocation *disk = static_cast<DiskLocation*> (l);
    return disk->getExternalFSWatcher();
 }
 #endif
@@ -885,8 +885,8 @@
 
 void DirModel::paste()
 {
-    // Restrict pasting if in restricted directory
-    if (!allowAccess(mCurrentDir)) {
+    // Restrict pasting if in restricted directory when pasting on a local file system
+    if (!mCurLocation->isRemote() && !allowAccess(mCurrentDir)) {
         qDebug() << Q_FUNC_INFO << "access not allowed, pasting not done" << mCurrentDir;
         return;
     }
@@ -1682,10 +1682,10 @@
 
 void DirModel:: moveIndexesToTrash(const QList<int>& items)
 { 
-    if (mCurLocation->type() == LocationsFactory::LocalDisk)
+    if (mCurLocation->isLocalDisk())
     {
         const TrashLocation *trashLocation = static_cast<const TrashLocation*>
-                   (mLocationFactory->getLocation(LocationsFactory::TrashDisk));
+                   (mLocationFactory->getTrashLocation());
         ActionPathList  itemsAndTrashPath;
         int index = 0;
         for (int counter=0; counter < items.count(); ++counter)

=== modified file 'src/plugin/folderlistmodel/location.cpp'
--- src/plugin/folderlistmodel/location.cpp	2015-03-01 19:02:31 +0000
+++ src/plugin/folderlistmodel/location.cpp	2015-07-18 21:23:48 +0000
@@ -305,4 +305,27 @@
     }
 }
 
-
+/*
+ *   Each Location should have its implementation if it is possible
+ */
+bool Location::isThereDiskSpace(const QString &pathname, qint64 requiredSize)
+{
+    Q_UNUSED(pathname);
+    Q_UNUSED(requiredSize);
+    return true;
+}
+
+
+/*!
+ * \brief Location::currentInfo()
+ * \return  the updated information about the current path
+ */
+const DirItemInfo *Location::currentInfo()
+{
+    if (m_info == 0)
+    {
+        m_info = new DirItemInfo();
+    }
+    refreshInfo(); //update information
+    return m_info;
+}

=== modified file 'src/plugin/folderlistmodel/location.h'
--- src/plugin/folderlistmodel/location.h	2015-03-01 15:32:42 +0000
+++ src/plugin/folderlistmodel/location.h	2015-07-18 21:23:48 +0000
@@ -47,6 +47,24 @@
 {
    Q_OBJECT
 public:  
+
+    Q_ENUMS(Locations)
+    /*!
+     * \brief The Locations enum defines which Locations are supported
+     *
+     * \note Items also work as indexes for \a m_locations, they must be 0..(n-1)
+     */
+    enum Locations
+    {
+        LocalDisk=0,   //<! any mounted file system
+        TrashDisk,     //<! special trash location in the disk
+        NetSambaShare  //<! SAMBA or CIFS shares
+#if 0
+        NetFishShare   //<! FISH protocol over ssh that provides file sharing
+#endif
+    };
+
+public:
     virtual ~Location();
 protected:
     explicit Location( int type, QObject *parent=0);
@@ -93,6 +111,21 @@
 
 public:
     /*!
+     * \brief isThereDiskSpace()  Check if the filesystem has enough space to put a file with size \a requiredSize
+     *
+     *
+     * \param pathname  is the full pathname of the new file that is going to be created
+     *
+     * \param requiredSize  the size required
+     *
+     *
+     * \note  For remote locations if not is possible to get this value this function MUST return true
+     *
+     *        The default implementation just returns true and let the copy fail if there is enough space
+     */
+    virtual bool     isThereDiskSpace(const QString& pathname, qint64 requiredSize);
+
+    /*!
      * \brief fetchItems() gets the content of the Location
      *
      * \param dirFilter   current Filter
@@ -131,6 +164,13 @@
       */
      virtual DirItemInfo *       validateUrlPath(const QString& urlPath);
 
+    /*!
+      * \brief isRemote() It must return TRUE when type() is greater than Location::TrashDisk
+      * \return
+      */
+     inline  bool       isRemote()     const { return m_type > TrashDisk; }
+     inline  bool       isLocalDisk()  const { return m_type == LocalDisk;}
+     inline  bool       isTrashDisk()  const { return m_type == TrashDisk; }
 
 public: //virtual
     virtual void        fetchExternalChanges(const QString& urlPath,
@@ -153,6 +193,7 @@
 
     inline const DirItemInfo*  info() const  { return m_info; }
     inline int                 type() const  { return m_type; }
+    const DirItemInfo*         currentInfo(); //updated information about the current path
 
 protected:
      DirItemInfo *                m_info;

=== modified file 'src/plugin/folderlistmodel/locationsfactory.cpp'
--- src/plugin/folderlistmodel/locationsfactory.cpp	2015-06-28 18:07:28 +0000
+++ src/plugin/folderlistmodel/locationsfactory.cpp	2015-07-18 21:23:48 +0000
@@ -50,16 +50,9 @@
  , m_authDataStore(NetAuthenticationDataList::getInstance(this))
  , m_lastUrlNeedsAuthentication(false)
 {
-   m_locations.append(new DiskLocation(LocalDisk));
-   m_locations.append(new TrashLocation(TrashDisk));
-   SmbLocation * smblocation = new SmbLocation(NetSambaShare);
-   m_locations.append(smblocation);
-
-   // Qt::DirectConnection is used here
-   // it allows lastUrlNeedsAuthencation() to have the right flag
-   connect(smblocation, SIGNAL(needsAuthentication(QString,QString)),
-           this,        SLOT(onUrlNeedsAuthentication(QString,QString)),
-                        Qt::DirectConnection);
+   addLocation(new DiskLocation(Location::LocalDisk));
+   addLocation(new TrashLocation(Location::TrashDisk));
+   addLocation(new SmbLocation(Location::NetSambaShare));
 }
 
 LocationsFactory::~LocationsFactory()
@@ -92,7 +85,7 @@
 #if defined(Q_OS_UNIX)
         if (uPath.startsWith(LocationUrl::TrashRootURL.midRef(0,6)))
         {
-            type = TrashDisk;
+            type = Location::TrashDisk;
             m_tmpPath  = LocationUrl::TrashRootURL + DirItemInfo::removeExtraSlashes(uPath, index+1);
         }
         else
@@ -100,7 +93,7 @@
 #endif
         if (uPath.startsWith(LocationUrl::DiskRootURL.midRef(0,5)))
         {
-            type = LocalDisk;
+            type = Location::LocalDisk;
             m_tmpPath  = QDir::rootPath() + DirItemInfo::removeExtraSlashes(uPath, index+1);
         }
         else
@@ -108,14 +101,14 @@
              uPath.startsWith(LocationUrl::CifsURL.midRef(0,5))
            )
         {
-                type = NetSambaShare;
+                type = Location::NetSambaShare;
                 m_tmpPath  = LocationUrl::SmbURL + DirItemInfo::removeExtraSlashes(uPath, index+1);
         }
     }
     else
     {
         m_tmpPath = DirItemInfo::removeExtraSlashes(uPath, -1);
-        type    = LocalDisk;
+        type    = Location::LocalDisk;
         if (!m_tmpPath.startsWith(QDir::rootPath()) && m_curLoc)
         {
             //it can be any, check current location
@@ -127,7 +120,7 @@
         location = m_locations.at(type);       
     }
 #if DEBUG_MESSAGES
-    qDebug() << Q_FUNC_INFO << "input path:" << uPath  << "location result:" << location;
+    qDebug() << Q_FUNC_INFO << "input path:" << uPath  << "location:" << location << "type:" << type;
 #endif
     return location;
 }
@@ -246,3 +239,16 @@
     }
     return item;
 }
+
+
+void LocationsFactory::addLocation(Location *location)
+{
+     m_locations.append(location);
+
+     // Qt::DirectConnection is used here
+     // it allows lastUrlNeedsAuthencation() to have the right flag
+     connect(location,   SIGNAL(needsAuthentication(QString,QString)),
+             this,       SLOT(onUrlNeedsAuthentication(QString,QString)),
+             Qt::DirectConnection);
+}
+

=== modified file 'src/plugin/folderlistmodel/locationsfactory.h'
--- src/plugin/folderlistmodel/locationsfactory.h	2015-06-03 11:54:36 +0000
+++ src/plugin/folderlistmodel/locationsfactory.h	2015-07-18 21:23:48 +0000
@@ -22,11 +22,11 @@
 #ifndef LOCATIONSFACTORY_H
 #define LOCATIONSFACTORY_H
 
+#include "location.h"
+
 #include <QObject>
 #include <QList>
 
-
-class Location;
 class DirItemInfo;
 class NetAuthenticationDataList;
 class NetAuthenticationData;
@@ -57,19 +57,9 @@
     explicit LocationsFactory(QObject *parent = 0);
     ~LocationsFactory();
 
-    Q_ENUMS(Locations)
-    enum Locations
-    {
-        LocalDisk,     //<! any mounted file system
-        TrashDisk,     //<! special trash location in the disk
-        NetSambaShare  //<! SAMBA or CIFS shares
-#if 0
-        NetFishShare   //<! FISH protocol over ssh that provides file sharing
-#endif
-    };
-
-    inline const Location * getLocation(Locations index) const {return m_locations.at(index);}
-
+    inline  Location * getLocation(int index) const {return m_locations.at(index);}
+    inline  Location * getDiskLocation()  const { return getLocation(Location::LocalDisk); }
+    inline  Location * getTrashLocation() const { return getLocation(Location::TrashDisk); }
 
     /*!
      * \brief parse()  Just parses (does not set/change the current location) according to \a urlPath
@@ -100,7 +90,7 @@
      * \brief location()
      * \return The current location
      */
-    Location * location() const { return m_curLoc; }
+    Location * currentLocation() const { return m_curLoc; }
 
     /*!
      * \brief availableLocations()
@@ -155,6 +145,12 @@
      */
     DirItemInfo *validateCurrentUrl(Location *location, const NetAuthenticationData&);
 
+    /*!
+     * \brief addLocation() just appends the location in the list \ref m_locations and connect signals
+     * \param location
+     */
+    void        addLocation(Location * location);
+
 signals:
     void        locationChanged(const Location *old, const Location *current);
 

=== modified file 'src/plugin/test_folderlistmodel/regression/tst_folderlistmodel.cpp'
--- src/plugin/test_folderlistmodel/regression/tst_folderlistmodel.cpp	2015-06-28 18:07:28 +0000
+++ src/plugin/test_folderlistmodel/regression/tst_folderlistmodel.cpp	2015-07-18 21:23:48 +0000
@@ -2520,7 +2520,7 @@
 #if 0 // "sheme:/"  (single slash) is no longer supported
     location = factoryLocations.setNewPath("trash:/");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::TrashDisk);
+    QVERIFY(location->isTrashDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   validTrashURL);
     QCOMPARE(location->urlPath(), validTrashURL);
     QCOMPARE(location->isRoot(), true);
@@ -2528,21 +2528,21 @@
 
     location = factoryLocations.setNewPath("trash://");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::TrashDisk);
+    QVERIFY(location->isTrashDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   validTrashURL);
     QCOMPARE(location->urlPath(), validTrashURL);
     QCOMPARE(location->isRoot(), true);
 
     location = factoryLocations.setNewPath("trash:///");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::TrashDisk);
+    QVERIFY(location->isTrashDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   validTrashURL);
     QCOMPARE(location->urlPath(), validTrashURL);
     QCOMPARE(location->isRoot(), true);
 
     location = factoryLocations.setNewPath("trash://////");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::TrashDisk);
+    QVERIFY(location->isTrashDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   validTrashURL);
     QCOMPARE(location->urlPath(), validTrashURL);
     QCOMPARE(location->isRoot(), true);
@@ -2555,28 +2555,28 @@
 
     location = factoryLocations.setNewPath("file://////");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::LocalDisk);
+    QVERIFY(location->isLocalDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   QDir::rootPath());
     QCOMPARE(location->urlPath(), QDir::rootPath());
     QCOMPARE(location->isRoot(), true);
 
     location = factoryLocations.setNewPath("/");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::LocalDisk);
+    QVERIFY(location->isLocalDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   QDir::rootPath());
     QCOMPARE(location->urlPath(), QDir::rootPath());
     QCOMPARE(location->isRoot(), true);
 
     location = factoryLocations.setNewPath("//");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::LocalDisk);
+    QVERIFY(location->isLocalDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   QDir::rootPath());
     QCOMPARE(location->urlPath(), QDir::rootPath());
     QCOMPARE(location->isRoot(), true);
 
     location = factoryLocations.setNewPath("//bin");
     QVERIFY(location);
-    QVERIFY(location->type() == LocationsFactory::LocalDisk);
+    QVERIFY(location->isLocalDisk());
     QCOMPARE(location->info()->absoluteFilePath(),   QLatin1String("/bin"));
     QCOMPARE(location->urlPath(), QLatin1String("/bin"));
     QCOMPARE(location->isRoot(), false);


Follow ups