From a9d6b04f776b80acd1a97f48c7ed10e3599618b6 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 9 Jun 2020 15:16:34 +1000 Subject: [PATCH] [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 --- .../auto_generated/qgsbrowsermodel.sip.in | 7 ++++--- python/core/auto_generated/qgsdataitem.sip.in | 14 ++++++++----- src/app/qgisapp.cpp | 20 ++++++++++++++++--- src/app/qgisapp.h | 4 ++++ .../providers/ogr/qgsgeopackagedataitems.cpp | 4 ++-- src/core/qgsbrowsermodel.cpp | 19 ++++++++++++++++-- src/core/qgsbrowsermodel.h | 8 +++++--- src/core/qgsdataitem.cpp | 9 +++++---- src/core/qgsdataitem.h | 16 ++++++++++----- 9 files changed, 74 insertions(+), 27 deletions(-) diff --git a/python/core/auto_generated/qgsbrowsermodel.sip.in b/python/core/auto_generated/qgsbrowsermodel.sip.in index d408695348f..8d8bf9ef339 100644 --- 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 index c44ca229b69..b449946e908 100644 --- 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 index 8facafa1b44..3e3385e8699 100644 --- a/src/app/qgisapp.cpp +++ b/src/app/qgisapp.cpp @@ -1218,8 +1218,17 @@ 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 ); @@ -1227,7 +1236,12 @@ QgisApp::QgisApp( QSplashScreen *splash, bool restorePlugins, bool skipVersionCh 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 index 3c570e903b8..d711ee0ba88 100644 --- 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 mGeometryValidationService; QgsGeometryValidationModel *mGeometryValidationModel = nullptr; QgsGeometryValidationDock *mGeometryValidationDock = nullptr; diff --git a/src/core/providers/ogr/qgsgeopackagedataitems.cpp b/src/core/providers/ogr/qgsgeopackagedataitems.cpp index 8b4deb68f91..70d33257166 100644 --- 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 index 948282ddfe6..9f510470cc6 100644 --- 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 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( 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 index e250d30cdc2..15f4ffc9d18 100644 --- 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 index 85d3de6394a..d446afa994c 100644 --- 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 index e697ecfdf15..778c1aae6b3 100644 --- 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: