[browser] By default do not monitor directories on drives we know

are slow Effectively this means that the browser no longer defaults to watching network and remote drives (on Windows) for changes. This is expensive to do and can result in large hangs in the QGIS application. Users can still manually opt-in to monitoring of these locations through the context menu of the directory in the browser panel.
qgis · May 31, 2021 · 8ff0efd · 8ff0efd
1 parent 75838d3
commit 8ff0efd
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 4 deletions.
diff --git a/cmake_templates/qgsconfig.h.in b/cmake_templates/qgsconfig.h.in
@@ -97,5 +97,7 @@
 #define PDAL_VERSION_MINOR "${PDAL_VERSION_MINOR}"
 #define PDAL_VERSION_MICRO "${PDAL_VERSION_MICRO}"
 
+#cmakedefine ENABLE_TESTS
+
 #endif
 
diff --git a/python/core/auto_generated/browser/qgsdirectoryitem.sip.in b/python/core/auto_generated/browser/qgsdirectoryitem.sip.in
@@ -142,8 +142,9 @@ All parent directories will be checked to determine whether they have monitoring
 manually enabled or disabled. As soon as a parent directory is found which has monitoring
 manually enabled or disabled then the corresponding value will be returned.
 
-Paths are monitored by default, so if no explicit setting is in place for a parent directory then
-the function will return ``True``.
+If no explicit setting is in place for a parent directory, then a check will be made to determine
+whether the path resides on a known slow drive. If so, monitoring is disabled by default and
+``False`` will be returned. Otherwise paths are monitored by default and the function will return ``True``.
 
 .. seealso:: :py:func:`isMonitored`
 

diff --git a/src/core/browser/qgsdirectoryitem.cpp b/src/core/browser/qgsdirectoryitem.cpp
@@ -23,6 +23,7 @@
 #include "qgsdataprovider.h"
 #include "qgszipitem.h"
 #include "qgsprojectitem.h"
+#include "qgsfileutils.h"
 #include <QFileSystemWatcher>
 #include <QDir>
 #include <QMouseEvent>
@@ -449,6 +450,11 @@ bool QgsDirectoryItem::pathShouldByMonitoredByDefault( const QString &path )
     }
   }
 
+  // else if we know that the path is on a slow device, we don't monitor by default
+  // as this can be very expensive and slow down QGIS
+  if ( QgsFileUtils::pathIsSlowDevice( path ) )
+    return false;
+
   // paths are monitored by default if no explicit setting is in place
   return true;
 }

diff --git a/src/core/browser/qgsdirectoryitem.h b/src/core/browser/qgsdirectoryitem.h
@@ -150,8 +150,9 @@ class CORE_EXPORT QgsDirectoryItem : public QgsDataCollectionItem
      * manually enabled or disabled. As soon as a parent directory is found which has monitoring
      * manually enabled or disabled then the corresponding value will be returned.
      *
-     * Paths are monitored by default, so if no explicit setting is in place for a parent directory then
-     * the function will return TRUE.
+     * If no explicit setting is in place for a parent directory, then a check will be made to determine
+     * whether the path resides on a known slow drive. If so, monitoring is disabled by default and
+     * FALSE will be returned. Otherwise paths are monitored by default and the function will return TRUE.
      *
      * \see isMonitored()
      * \see setMonitoring()

diff --git a/src/core/qgsfileutils.cpp b/src/core/qgsfileutils.cpp
@@ -15,6 +15,7 @@
 #include "qgsfileutils.h"
 #include "qgis.h"
 #include "qgsexception.h"
+#include "qgsconfig.h"
 #include <QObject>
 #include <QRegularExpression>
 #include <QFileInfo>
@@ -337,6 +338,11 @@ Qgis::DriveType QgsFileUtils::driveType( const QString &path )
 
 bool QgsFileUtils::pathIsSlowDevice( const QString &path )
 {
+#ifdef ENABLE_TESTS
+  if ( path.contains( QLatin1String( "fake_slow_path_for_unit_tests" ) ) )
+    return true;
+#endif
+
   try
   {
     const Qgis::DriveType type = driveType( path );

diff --git a/tests/src/core/testqgsdataitem.cpp b/tests/src/core/testqgsdataitem.cpp
@@ -53,6 +53,7 @@ class TestQgsDataItem : public QObject
     void testDirItem();
     void testDirItemChildren();
     void testDirItemMonitoring();
+    void testDirItemMonitoringSlowDrive();
     void testLayerItemType();
     void testProjectItemCreation();
 
@@ -341,6 +342,57 @@ void TestQgsDataItem::testDirItemMonitoring()
   QVERIFY( !childItem3->mFileSystemWatcher );
 }
 
+void TestQgsDataItem::testDirItemMonitoringSlowDrive()
+{
+  QTemporaryDir dir;
+
+  // fake a directory on a slow drive -- this is a special hardcoded path which always reports to be on a slow drive. See QgsFileUtils::pathIsSlowDevice
+  const QString parentDir = dir.path() + QStringLiteral( "/fake_slow_path_for_unit_tests" );
+  const QString child1 = parentDir + QStringLiteral( "/child1" );
+  QVERIFY( QDir().mkpath( child1 ) );
+  const QString child2 = parentDir + QStringLiteral( "/child2" );
+  QVERIFY( QDir().mkpath( child2 ) );
+  const QString child2child = child2 + QStringLiteral( "/child" );
+  QVERIFY( QDir().mkpath( child2child ) );
+
+  QVERIFY( !QgsDirectoryItem::pathShouldByMonitoredByDefault( parentDir ) );
+  QVERIFY( !QgsDirectoryItem::pathShouldByMonitoredByDefault( child1 ) );
+  QVERIFY( !QgsDirectoryItem::pathShouldByMonitoredByDefault( child2 ) );
+  QVERIFY( !QgsDirectoryItem::pathShouldByMonitoredByDefault( child2child ) );
+
+  std::unique_ptr< QgsDirectoryItem > dirItem = std::make_unique< QgsDirectoryItem >( nullptr, QStringLiteral( "parent name" ), parentDir, QStringLiteral( "/" ) + parentDir );
+  // user has not explicitly set the path to be monitored or not, so Default should be returned here:
+  QCOMPARE( dirItem->monitoring(), Qgis::BrowserDirectoryMonitoring::Default );
+  // but directory should NOT be monitored
+  QVERIFY( !dirItem->isMonitored() );
+
+  dirItem->populate( true );
+  QVERIFY( !dirItem->mFileSystemWatcher );
+
+  QCOMPARE( dirItem->rowCount(), 2 );
+  QPointer< QgsDirectoryItem > childItem1( qobject_cast< QgsDirectoryItem * >( dirItem->children().at( 0 ) ) );
+  QPointer< QgsDirectoryItem > childItem2( qobject_cast< QgsDirectoryItem * >( dirItem->children().at( 1 ) ) );
+  QVERIFY( childItem1 );
+  QVERIFY( childItem2 );
+  // neither of these should be monitored either!
+  QVERIFY( !childItem1->isMonitored() );
+  QVERIFY( !childItem2->isMonitored() );
+  childItem1->populate( true );
+  childItem2->populate( true );
+  QVERIFY( !childItem1->mFileSystemWatcher );
+  QVERIFY( !childItem2->mFileSystemWatcher );
+
+  // explicitly opt in to monitoring a directory on a slow drive
+  childItem2->setMonitoring( Qgis::BrowserDirectoryMonitoring::AlwaysMonitor );
+  QVERIFY( childItem2->mFileSystemWatcher );
+
+  // this means that subdirectories of childItem2 should now return true to being monitored by default
+  QVERIFY( !QgsDirectoryItem::pathShouldByMonitoredByDefault( parentDir ) );
+  QVERIFY( !QgsDirectoryItem::pathShouldByMonitoredByDefault( child1 ) );
+  QVERIFY( !QgsDirectoryItem::pathShouldByMonitoredByDefault( child2 ) );
+  QVERIFY( QgsDirectoryItem::pathShouldByMonitoredByDefault( child2child ) );
+}
+
 void TestQgsDataItem::testLayerItemType()
 {
   std::unique_ptr< QgsMapLayer > layer = std::make_unique< QgsVectorLayer >( mTestDataDir + "polys.shp",