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/new-protocols-support-02 into lp:ubuntu-filemanager-app with lp:~carlos-mazieri/ubuntu-filemanager-app/new-protocols-support-01 as a prerequisite.

Commit message:
LocationItemDirIterator descendants classes now perform the loading inside the constructor according to the new constructor flag LocationItemDirIterator::LoadMode.
LocationItemDirIterator::LoadLater allows LocationItemDirIterator::load() be called later, that is how Network Locations should put the loading inside the secondary thread specific for loading.
For Disk loading, that means in DiskLocationItemDirIterator class it was not implemented because QDir is used directly inside the secondary thread.

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

For more details, see:
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/new-protocols-support-02/+merge/280388

LocationItemDirIterator descendants classes now perform the loading inside the constructor according to the new constructor flag LocationItemDirIterator::LoadMode.
LocationItemDirIterator::LoadLater allows LocationItemDirIterator::load() be called later, that is how Network Locations should put the loading inside the secondary thread specific for loading.
For Disk loading, that means in DiskLocationItemDirIterator class it was not implemented because QDir is used directly inside the secondary thread.
-- 
Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~carlos-mazieri/ubuntu-filemanager-app/new-protocols-support-02 into lp:ubuntu-filemanager-app.

=== modified file 'src/plugin/folderlistmodel/disk/disklocation.cpp'
--- src/plugin/folderlistmodel/disk/disklocation.cpp	2015-12-08 16:55:41 +0000
+++ src/plugin/folderlistmodel/disk/disklocation.cpp	2015-12-12 15:44:23 +0000
@@ -192,8 +192,10 @@
 LocationItemDirIterator *
 DiskLocation::newDirIterator(const QString &path,
                              QDir::Filters filters,
-                             QDirIterator::IteratorFlags flags)
+                             QDirIterator::IteratorFlags flags,
+                             LocationItemDirIterator::LoadMode loadmode)
 {
+    Q_UNUSED(loadmode);
     return  new DiskLocationItemDirIterator(path, filters, flags);
 }
 

=== modified file 'src/plugin/folderlistmodel/disk/disklocation.h'
--- src/plugin/folderlistmodel/disk/disklocation.h	2015-12-08 16:55:41 +0000
+++ src/plugin/folderlistmodel/disk/disklocation.h	2015-12-12 15:44:23 +0000
@@ -65,7 +65,8 @@
                                           const bool isRecursive);
     virtual LocationItemDirIterator * newDirIterator(const QString & path,
                                                      QDir::Filters filters,
-                                                     QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags);
+                                                     QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                                                     LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
     virtual LocationItemFile   * newFile(const QString & path);
     virtual LocationItemDir    * newDir(const QString & dir = QLatin1String(0));
     virtual bool        isThereDiskSpace(const QString& pathname, qint64 requiredSize);

=== modified file 'src/plugin/folderlistmodel/location.h'
--- src/plugin/folderlistmodel/location.h	2015-12-08 16:55:41 +0000
+++ src/plugin/folderlistmodel/location.h	2015-12-12 15:44:23 +0000
@@ -23,6 +23,7 @@
 #define LOCATION_H
 
 #include "diriteminfo.h"
+#include "locationitemdiriterator.h"
 
 #include <QObject>
 #include <QDirIterator>
@@ -124,7 +125,8 @@
      */
      virtual LocationItemDirIterator * newDirIterator(const QString & path,
                                                      QDir::Filters filters,
-                                                     QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags)  = 0;
+                                                     QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                                                     LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor)  = 0;
     /*!
       * \brief newFile() creates a LocationItemFile object which is similar to Qt QFile object
       *

=== modified file 'src/plugin/folderlistmodel/locationitemdiriterator.cpp'
--- src/plugin/folderlistmodel/locationitemdiriterator.cpp	2015-03-01 15:32:42 +0000
+++ src/plugin/folderlistmodel/locationitemdiriterator.cpp	2015-12-12 15:44:23 +0000
@@ -21,19 +21,40 @@
 
 #include "locationitemdiriterator.h"
 
-LocationItemDirIterator::LocationItemDirIterator(const QString &, const QStringList &, QDir::Filters, QDirIterator::IteratorFlags )
-{
-}
-
-
-LocationItemDirIterator::LocationItemDirIterator(const QString &, QDir::Filters , QDirIterator::IteratorFlags)
-{
-
-}
-
-LocationItemDirIterator::LocationItemDirIterator(const QString &, QDirIterator::IteratorFlags )
-{
-
+LocationItemDirIterator::LocationItemDirIterator(const QString &path,
+                                                 const QStringList &nameFilters,
+                                                 QDir::Filters filters,
+                                                 QDirIterator::IteratorFlags flags,
+                                                 LocationItemDirIterator::LoadMode loadmode)
+                                                 : m_path(path)
+                                                 , m_nameFilters(nameFilters)
+                                                 , m_filters(filters)
+                                                 , m_flags(flags)
+
+{
+    Q_UNUSED(loadmode);
+}
+
+
+LocationItemDirIterator::LocationItemDirIterator(const QString &path,
+                                                 QDir::Filters filters,
+                                                 QDirIterator::IteratorFlags flags,
+                                                 LocationItemDirIterator::LoadMode loadmode)
+                                                 : m_path(path)
+                                                 , m_filters(filters)
+                                                 , m_flags(flags)
+{
+     Q_UNUSED(loadmode);
+}
+
+LocationItemDirIterator::LocationItemDirIterator(const QString &path,
+                         QDirIterator::IteratorFlags flags,
+                         LocationItemDirIterator::LoadMode loadmode)
+                         : m_path(path)
+                         , m_filters(QDir::NoFilter)
+                         , m_flags(flags)
+{
+     Q_UNUSED(loadmode);
 }
 
 LocationItemDirIterator::~LocationItemDirIterator()
@@ -42,3 +63,22 @@
 }
 
 
+QDirIterator::IteratorFlags LocationItemDirIterator::flags() const
+{
+    return m_flags;
+}
+
+QDir::Filters LocationItemDirIterator::filters() const
+{
+    return m_filters;
+}
+
+QString LocationItemDirIterator::path() const
+{
+    return m_path;
+}
+
+void LocationItemDirIterator::load()
+{
+    //default implementation does nothing
+}

=== modified file 'src/plugin/folderlistmodel/locationitemdiriterator.h'
--- src/plugin/folderlistmodel/locationitemdiriterator.h	2015-03-01 15:32:42 +0000
+++ src/plugin/folderlistmodel/locationitemdiriterator.h	2015-12-12 15:44:23 +0000
@@ -29,14 +29,22 @@
  * \brief The LocationItemDirIterator class is an abstract similar to Qt QDirIterator
  *
  *   Different protocols supported by filemanager (different Locations) must provide a class like that.
+ *
+ *   The \ref LoadLater can used in the constructor to indicate to the constructor to NOT load the path/url, instead \ref load() can called later to do that.
  */
 
 class LocationItemDirIterator
 {
 public:
+   enum LoadMode
+   {
+     LoadOnConstructor,  //!< loads the entire directory or url in the constructor as Qt QDirIterator does
+     LoadLater           //!< do NOT load the entire directory or url, \ref load() method should be responsabile to do that.
+   };
+
    virtual ~LocationItemDirIterator();
 public:
-   virtual bool	        hasNext()  const = 0;
+   virtual bool         hasNext()  const = 0;
    virtual QString	    next()           = 0;
 
    virtual DirItemInfo	fileInfo() const = 0;
@@ -50,18 +58,43 @@
     * \brief filePath()
     * \return the full pathname of the current item
     */
-   virtual QString	    filePath() const = 0;
+   virtual QString          filePath() const = 0;
 
     /*!
     * \brief path()
     * \return  the base directory of the iterator path (not the current item)
     */
-   virtual QString	    path()     const = 0;
-
-protected:
-   LocationItemDirIterator(const QString & path, QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags);
-   LocationItemDirIterator(const QString & path, QDir::Filters filters, QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags);
-   LocationItemDirIterator(const QString & path, const QStringList & nameFilters, QDir::Filters filters = QDir::NoFilter, QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags);
+   virtual QString          path()     const;
+
+   /*!
+    * \brief load()  reponsable to load the entire directory or url when \ref LoadLater is passed to the constructor
+    */
+   virtual void         load() ;
+
+   QDir::Filters        filters()  const;
+   QDirIterator::IteratorFlags flags() const;
+
+protected:
+   LocationItemDirIterator(const QString & path,
+                           QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                           LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
+
+   LocationItemDirIterator(const QString & path,
+                           QDir::Filters filters,
+                           QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                           LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
+
+   LocationItemDirIterator(const QString & path,
+                           const QStringList & nameFilters,
+                           QDir::Filters filters = QDir::NoFilter,
+                           QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                           LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
+
+protected:
+   QString                     m_path;
+   QStringList                 m_nameFilters;
+   QDir::Filters               m_filters;
+   QDirIterator::IteratorFlags m_flags;
 };
 
 

=== modified file 'src/plugin/folderlistmodel/smb/qsambaclient/src/smblocationdiriterator.cpp'
--- src/plugin/folderlistmodel/smb/qsambaclient/src/smblocationdiriterator.cpp	2015-07-13 20:41:48 +0000
+++ src/plugin/folderlistmodel/smb/qsambaclient/src/smblocationdiriterator.cpp	2015-12-12 15:44:23 +0000
@@ -32,35 +32,44 @@
                                                const QStringList &nameFilters,
                                                QDir::Filters filters,
                                                QDirIterator::IteratorFlags flags,
-                                               Const_SmbUtil_Ptr smb)
-        : LocationItemDirIterator(path, nameFilters, filters, flags)
+                                               Const_SmbUtil_Ptr smb,
+                                               LocationItemDirIterator::LoadMode loadmode)
+        : LocationItemDirIterator(path, nameFilters, filters, flags, loadmode)
         , INIT_ATTR(path, smb)
 {
-    bool recursive = flags == QDirIterator::Subdirectories ? true : false;
-    m_urlItems = smbObj()->listContent(path, recursive, filters, nameFilters);
+   if (loadmode == LocationItemDirIterator::LoadOnConstructor)
+   {
+       load();
+   }
 }
 
 
 SmbLocationDirIterator::SmbLocationDirIterator(const QString &path,
                                                QDir::Filters filters,
                                                QDirIterator::IteratorFlags flags,
-                                               Const_SmbUtil_Ptr smb)
-     : LocationItemDirIterator(path,filters,flags)
+                                               Const_SmbUtil_Ptr smb,
+                                               LocationItemDirIterator::LoadMode loadmode)
+     : LocationItemDirIterator(path,filters,flags, loadmode)
      , INIT_ATTR(path, smb)
 {
-    bool recursive = flags == QDirIterator::Subdirectories ? true : false;
-    m_urlItems = smbObj()->listContent(path, recursive, filters);
+    if (loadmode == LocationItemDirIterator::LoadOnConstructor)
+    {
+        load();
+    }
 }
 
 
 SmbLocationDirIterator::SmbLocationDirIterator(const QString &path,
                                                QDirIterator::IteratorFlags flags,
-                                               Const_SmbUtil_Ptr smb)
-       : LocationItemDirIterator(path,flags)
+                                               Const_SmbUtil_Ptr smb,
+                                               LocationItemDirIterator::LoadMode loadmode)
+       : LocationItemDirIterator(path,flags, loadmode)
        , INIT_ATTR(path, smb)
 {
-    bool recursive = flags == QDirIterator::Subdirectories ? true : false;
-    m_urlItems = smbObj()->listContent(path, recursive, QDir::NoFilter);
+    if (loadmode == LocationItemDirIterator::LoadOnConstructor)
+    {
+        load();
+    }
 }
 
 
@@ -113,18 +122,6 @@
 }
 
 
-QString SmbLocationDirIterator::path() const
-{
-    QString ret;
-    QStringList separated =  UrlItemInfo::separatePathFilename(cleanUrl());
-    if (separated.count() > 0)
-    {
-        ret = separated.at(0);
-    }
-    return ret;
-}
-
-
 QString SmbLocationDirIterator::filePath() const
 {
     QString fullpathname;
@@ -134,3 +131,10 @@
     }
     return fullpathname;
 }
+
+
+void SmbLocationDirIterator::load()
+{
+    bool recursive = m_flags == QDirIterator::Subdirectories ? true : false;
+    m_urlItems = smbObj()->listContent(path(), recursive, m_filters, m_nameFilters);
+}

=== modified file 'src/plugin/folderlistmodel/smb/qsambaclient/src/smblocationdiriterator.h'
--- src/plugin/folderlistmodel/smb/qsambaclient/src/smblocationdiriterator.h	2015-05-20 16:18:07 +0000
+++ src/plugin/folderlistmodel/smb/qsambaclient/src/smblocationdiriterator.h	2015-12-12 15:44:23 +0000
@@ -32,14 +32,26 @@
 public:
    virtual DirItemInfo	fileInfo() const;
    virtual QString	    fileName() const;
-   virtual QString	    filePath() const;
-   virtual bool	        hasNext()  const;
-   virtual QString	    next()          ;
-   virtual QString	    path()     const;
+   virtual QString          filePath() const;
+   virtual bool         hasNext()  const;
+   virtual QString          next()          ;
+   virtual void         load();
 public:
-   SmbLocationDirIterator(const QString & path, QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags, Const_SmbUtil_Ptr  smb  = 0);
-   SmbLocationDirIterator(const QString & path, QDir::Filters filters, QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags, Const_SmbUtil_Ptr  smb  = 0);
-   SmbLocationDirIterator(const QString & path, const QStringList & nameFilters, QDir::Filters filters = QDir::NoFilter, QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags, Const_SmbUtil_Ptr  smb  = 0);
+   SmbLocationDirIterator(const QString & path,
+                          QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                          Const_SmbUtil_Ptr  smb  = 0,
+                          LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
+
+   SmbLocationDirIterator(const QString & path, QDir::Filters filters,
+                          QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                          Const_SmbUtil_Ptr  smb  = 0,
+                          LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
+
+   SmbLocationDirIterator(const QString & path, const QStringList & nameFilters,
+                          QDir::Filters filters = QDir::NoFilter,
+                          QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                          Const_SmbUtil_Ptr  smb  = 0,
+                          LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
 private:
    QStringList  m_urlItems;
    int          m_curItem;

=== modified file 'src/plugin/folderlistmodel/smb/smblocation.cpp'
--- src/plugin/folderlistmodel/smb/smblocation.cpp	2015-12-08 16:55:41 +0000
+++ src/plugin/folderlistmodel/smb/smblocation.cpp	2015-12-12 15:44:23 +0000
@@ -111,9 +111,10 @@
 LocationItemDirIterator *
 SmbLocation::newDirIterator(const QString &path,
                             QDir::Filters filters,
-                            QDirIterator::IteratorFlags flags)
+                            QDirIterator::IteratorFlags flags,
+                            LocationItemDirIterator::LoadMode loadmode)
 {
-    return new SmbLocationDirIterator(path, filters, flags, m_smb);
+    return new SmbLocationDirIterator(path, filters, flags, m_smb, loadmode);
 }
 
 

=== modified file 'src/plugin/folderlistmodel/smb/smblocation.h'
--- src/plugin/folderlistmodel/smb/smblocation.h	2015-12-08 16:55:41 +0000
+++ src/plugin/folderlistmodel/smb/smblocation.h	2015-12-12 15:44:23 +0000
@@ -41,7 +41,8 @@
                                           const bool isRecursive);
      virtual LocationItemDirIterator * newDirIterator(const QString & path,
                                                       QDir::Filters filters,
-                                                      QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags);
+                                                      QDirIterator::IteratorFlags flags = QDirIterator::NoIteratorFlags,
+                                                      LocationItemDirIterator::LoadMode loadmode = LocationItemDirIterator::LoadOnConstructor);
     virtual LocationItemFile   * newFile(const QString & path);   
     virtual LocationItemDir    * newDir(const QString & dir = QLatin1String(0));
      virtual bool       isThereDiskSpace(const QString& pathname, qint64 requiredSize);