[browser] Refine refreshConnections method to avoid triggering a full

refresh of ALL browser content Instead limit refresh to the provider associated with the item only (and provide a means for items to refresh a different provider, e.g. to allow the geopackage connection item to be refreshed when a new connection is added through a directory item) Fixes #37007
qgis · Jun 9, 2020 · a9d6b04 · a9d6b04
1 parent e516231
commit a9d6b04
Show file tree

Hide file tree

Showing 9 changed files with 74 additions and 27 deletions.
diff --git a/python/core/auto_generated/qgsbrowsermodel.sip.in b/python/core/auto_generated/qgsbrowsermodel.sip.in
@@ -170,10 +170,11 @@ and on Linux the "/" root directory.
 Emitted when item children fetch was finished
 %End
 
-    void connectionsChanged();
+    void connectionsChanged( const QString &providerKey );
 %Docstring
-Connections changed in the browser, forwarded to the widget and used to
-notify the provider dialogs of a changed connection
+Emitted when connections for the specified ``providerKey`` have changed in the browser.
+
+Forwarded to the widget and used to notify the provider dialogs of a changed connection.
 %End
 
   public slots:

diff --git a/python/core/auto_generated/qgsdataitem.sip.in b/python/core/auto_generated/qgsdataitem.sip.in
@@ -420,9 +420,12 @@ Remove children recursively and set as not populated. This is used when refreshi
 
     virtual void refresh();
 
-    virtual void refreshConnections();
+    virtual void refreshConnections( const QString &providerKey = QString() );
 %Docstring
-Refresh connections: update GUI and emit signal
+Causes a data item provider to refresh all registered connections.
+
+If ``provider`` is specified then only the matching provider will be refreshed. Otherwise,
+all providers will be refreshed (which is potentially very expensive!).
 %End
 
     virtual void childrenCreated();
@@ -435,12 +438,13 @@ Refresh connections: update GUI and emit signal
     void dataChanged( QgsDataItem *item );
     void stateChanged( QgsDataItem *item, QgsDataItem::State oldState );
 
-    void connectionsChanged();
+    void connectionsChanged( const QString &providerKey = QString() );
 %Docstring
-Emitted when the provider's connections of the child items have changed
+Emitted when the connections of the provider with the specified key have changed.
+
 This signal is normally forwarded to the app in order to refresh the connection
 item in the provider dialogs and to refresh the connection items in the other
-open browsers
+open browsers.
 %End
 
   protected slots:

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -1218,16 +1218,30 @@ QgisApp::QgisApp( QSplashScreen *splash, bool restorePlugins, bool skipVersionCh
   addDockWidget( Qt::LeftDockWidgetArea, mBrowserWidget );
   mBrowserWidget->hide();
   // Only connect the first widget: the model is shared, there is no need to refresh multiple times.
-  connect( this, &QgisApp::connectionsChanged, mBrowserWidget, &QgsBrowserDockWidget::refresh );
-  connect( mBrowserWidget, &QgsBrowserDockWidget::connectionsChanged, this, &QgisApp::connectionsChanged );
+  connect( this, &QgisApp::connectionsChanged, mBrowserWidget, [ = ]
+  {
+    if ( !mBlockBrowser1Refresh && !mBlockBrowser2Refresh )
+      mBrowserWidget->refresh();
+  } );
+  connect( mBrowserWidget, &QgsBrowserDockWidget::connectionsChanged, this, [ = ]
+  {
+    mBlockBrowser1Refresh++;
+    emit connectionsChanged();
+    mBlockBrowser1Refresh--;
+  } );
   connect( mBrowserWidget, &QgsBrowserDockWidget::openFile, this, &QgisApp::openFile );
   connect( mBrowserWidget, &QgsBrowserDockWidget::handleDropUriList, this, &QgisApp::handleDropUriList );
 
   mBrowserWidget2 = new QgsBrowserDockWidget( tr( "Browser (2)" ), mBrowserModel, this );
   mBrowserWidget2->setObjectName( QStringLiteral( "Browser2" ) );
   addDockWidget( Qt::LeftDockWidgetArea, mBrowserWidget2 );
   mBrowserWidget2->hide();
-  connect( mBrowserWidget2, &QgsBrowserDockWidget::connectionsChanged, this, &QgisApp::connectionsChanged );
+  connect( mBrowserWidget2, &QgsBrowserDockWidget::connectionsChanged, this, [ = ]
+  {
+    mBlockBrowser2Refresh++;
+    emit connectionsChanged();
+    mBlockBrowser2Refresh--;
+  } );
   connect( mBrowserWidget2, &QgsBrowserDockWidget::openFile, this, &QgisApp::openFile );
   connect( mBrowserWidget2, &QgsBrowserDockWidget::handleDropUriList, this, &QgisApp::handleDropUriList );
 

diff --git a/src/app/qgisapp.h b/src/app/qgisapp.h
@@ -2531,6 +2531,10 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
     //! True if we are blocking the activeLayerChanged signal from being emitted
     bool mBlockActiveLayerChanged = false;
 
+    int mBlockBrowser1Refresh = 0;
+    int mBlockBrowser2Refresh = 0;
+    int mDataSourceManagerRefresh = 0;
+
     std::unique_ptr<QgsGeometryValidationService> mGeometryValidationService;
     QgsGeometryValidationModel *mGeometryValidationModel = nullptr;
     QgsGeometryValidationDock *mGeometryValidationDock = nullptr;

diff --git a/src/core/providers/ogr/qgsgeopackagedataitems.cpp b/src/core/providers/ogr/qgsgeopackagedataitems.cpp
@@ -214,13 +214,13 @@ void QgsGeoPackageCollectionItem::addConnection()
   QgsOgrDbConnection connection( mName, QStringLiteral( "GPKG" ) );
   connection.setPath( mPath );
   connection.save();
-  mParent->refreshConnections();
+  mParent->refreshConnections( QStringLiteral( "GPKG" ) );
 }
 
 void QgsGeoPackageCollectionItem::deleteConnection()
 {
   QgsOgrDbConnection::deleteConnection( name(), QStringLiteral( "GPKG" ) );
-  mParent->refreshConnections();
+  mParent->refreshConnections( QStringLiteral( "GPKG" ) );
 }
 
 bool QgsGeoPackageCollectionItem::vacuumGeoPackageDb( const QString &name, const QString &path, QString &errCause )

diff --git a/src/core/qgsbrowsermodel.cpp b/src/core/qgsbrowsermodel.cpp
@@ -198,6 +198,21 @@ void QgsBrowserModel::dataItemProviderWillBeRemoved( QgsDataItemProvider *provid
   }
 }
 
+void QgsBrowserModel::onConnectionsChanged( const QString &providerKey )
+{
+  // refresh the matching provider
+  for ( QgsDataItem *item : qgis::as_const( mRootItems ) )
+  {
+    if ( item->providerKey() == providerKey )
+    {
+      item->refresh();
+      break;  // assuming there is max. 1 root item per provider
+    }
+  }
+
+  emit connectionsChanged( providerKey );
+}
+
 QMap<QString, QgsDirectoryItem *> QgsBrowserModel::driveItems() const
 {
   return mDriveItems;
@@ -596,7 +611,7 @@ void QgsBrowserModel::setupItemConnections( QgsDataItem *item )
   // if it's a collection item, also forwards connectionsChanged
   QgsDataCollectionItem *collectionItem = qobject_cast<QgsDataCollectionItem *>( item );
   if ( collectionItem )
-    connect( collectionItem, &QgsDataCollectionItem::connectionsChanged, this, &QgsBrowserModel::connectionsChanged );
+    connect( collectionItem, &QgsDataCollectionItem::connectionsChanged, this, &QgsBrowserModel::onConnectionsChanged );
 }
 
 QStringList QgsBrowserModel::mimeTypes() const
@@ -764,7 +779,7 @@ QgsDataItem *QgsBrowserModel::addProviderRootItem( QgsDataItemProvider *pr )
     // make sure the top level key is set always
     item->setProviderKey( pr->name() );
     // Forward the signal from the root items to the model (and then to the app)
-    connect( item, &QgsDataItem::connectionsChanged, this, &QgsBrowserModel::connectionsChanged );
+    connect( item, &QgsDataItem::connectionsChanged, this, &QgsBrowserModel::onConnectionsChanged );
     QgsDebugMsgLevel( "Add new top level item : " + item->name(), 4 );
     setupItemConnections( item );
   }

diff --git a/src/core/qgsbrowsermodel.h b/src/core/qgsbrowsermodel.h
@@ -184,10 +184,11 @@ class CORE_EXPORT QgsBrowserModel : public QAbstractItemModel
     void stateChanged( const QModelIndex &index, QgsDataItem::State oldState );
 
     /**
-     * Connections changed in the browser, forwarded to the widget and used to
-     * notify the provider dialogs of a changed connection
+     * Emitted when connections for the specified \a providerKey have changed in the browser.
+     *
+     * Forwarded to the widget and used to notify the provider dialogs of a changed connection.
      */
-    void connectionsChanged();
+    void connectionsChanged( const QString &providerKey );
 
   public slots:
     //! Reload the whole model
@@ -257,6 +258,7 @@ class CORE_EXPORT QgsBrowserModel : public QAbstractItemModel
   private slots:
     void dataItemProviderAdded( QgsDataItemProvider *provider );
     void dataItemProviderWillBeRemoved( QgsDataItemProvider *provider );
+    void onConnectionsChanged( const QString &providerKey );
 
   private:
     bool mInitialized = false;

diff --git a/src/core/qgsdataitem.cpp b/src/core/qgsdataitem.cpp
@@ -384,17 +384,18 @@ void QgsDataItem::refresh()
   }
 }
 
-void QgsDataItem::refreshConnections()
+void QgsDataItem::refreshConnections( const QString &key )
 {
   // Walk up until the root node is reached
   if ( mParent )
   {
-    mParent->refreshConnections();
+    mParent->refreshConnections( key );
   }
   else
   {
-    refresh();
-    emit connectionsChanged();
+    // if a specific key was specified then we use that -- otherwise we assume the connections
+    // changed belong to the same provider as this item
+    emit connectionsChanged( key.isEmpty() ? providerKey() : key );
   }
 }
 

diff --git a/src/core/qgsdataitem.h b/src/core/qgsdataitem.h
@@ -443,8 +443,13 @@ class CORE_EXPORT QgsDataItem : public QObject
 
     virtual void refresh();
 
-    //! Refresh connections: update GUI and emit signal
-    virtual void refreshConnections();
+    /**
+     * Causes a data item provider to refresh all registered connections.
+     *
+     * If \a provider is specified then only the matching provider will be refreshed. Otherwise,
+     * all providers will be refreshed (which is potentially very expensive!).
+     */
+    virtual void refreshConnections( const QString &providerKey = QString() );
 
     virtual void childrenCreated();
 
@@ -457,12 +462,13 @@ class CORE_EXPORT QgsDataItem : public QObject
     void stateChanged( QgsDataItem *item, QgsDataItem::State oldState );
 
     /**
-     * Emitted when the provider's connections of the child items have changed
+     * Emitted when the connections of the provider with the specified key have changed.
+     *
      * This signal is normally forwarded to the app in order to refresh the connection
      * item in the provider dialogs and to refresh the connection items in the other
-     * open browsers
+     * open browsers.
      */
-    void connectionsChanged();
+    void connectionsChanged( const QString &providerKey = QString() );
 
   protected slots: