Replace ArcGIS REST data source manager list with a view based on the…

… browser model This has MANY benefits: - Shows the merged feature service/map service view we use in browser - Avoids the HUGE main thread ui hang when connecting to servers with many layers (could be 20 mins+ for some services) - Avoids bombing out and showing incomplete lists when a server reports an error when connecting to any of the services it advertises (happens often)
qgis · Dec 23, 2020 · 8904919 · 8904919
1 parent 48b8e74
commit 8904919
Show file tree

Hide file tree

Showing 16 changed files with 327 additions and 184 deletions.
diff --git a/python/core/auto_generated/qgsbrowserproxymodel.sip.in b/python/core/auto_generated/qgsbrowserproxymodel.sip.in
@@ -192,6 +192,7 @@ Sets show layers to ``showLayers``
     virtual bool filterAcceptsRow( int sourceRow, const QModelIndex &sourceParent ) const;
 
 
+
   public:
     virtual bool hasChildren( const QModelIndex &parent = QModelIndex() ) const;
 

diff --git a/python/gui/auto_generated/qgsabstractdatasourcewidget.sip.in b/python/gui/auto_generated/qgsabstractdatasourcewidget.sip.in
@@ -34,6 +34,14 @@ Store a pointer to the map canvas to retrieve extent and CRS
 Used to select an appropriate CRS and possibly to retrieve data only in the current extent
 %End
 
+    void setBrowserModel( QgsBrowserModel *model );
+%Docstring
+Sets a browser ``model`` to use with the widget.
+
+.. seealso:: :py:func:`browserModel`
+
+.. versionadded:: 3.18
+%End
 
   public slots:
 
@@ -168,6 +176,13 @@ Returns the widget mode
     const QgsMapCanvas *mapCanvas() const;
 %Docstring
 Returns the map canvas (can be ``None``)
+%End
+
+    QgsBrowserModel *browserModel();
+%Docstring
+Returns the associated browser model (may be ``None``).
+
+.. versionadded:: 3.18
 %End
 
     void setupButtons( QDialogButtonBox *buttonBox );

diff --git a/src/core/qgsbrowserproxymodel.h b/src/core/qgsbrowserproxymodel.h
@@ -200,10 +200,14 @@ class CORE_EXPORT QgsBrowserProxyModel : public QSortFilterProxyModel
     // It would be better to apply the filer only to expanded (visible) items, but using mapFromSource() + view here was causing strange errors
     bool filterAcceptsRow( int sourceRow, const QModelIndex &sourceParent ) const override;
 
+    /**
+     * Reference to associated browser model.
+     */
+    QgsBrowserModel *mModel = nullptr;
+
   private:
     QStringList mHiddenDataItemsKeys;
     QStringList mShownDataItemsKeys;
-    QgsBrowserModel *mModel = nullptr;
     QString mFilter; //filter string provided
     QVector<QRegExp> mREList; //list of filters, separated by "|"
     FilterSyntax mPatternSyntax = Normal;

diff --git a/src/gui/qgsabstractdatasourcewidget.cpp b/src/gui/qgsabstractdatasourcewidget.cpp
@@ -35,6 +35,11 @@ const QgsMapCanvas *QgsAbstractDataSourceWidget::mapCanvas() const
   return mMapCanvas;
 }
 
+QgsBrowserModel *QgsAbstractDataSourceWidget::browserModel()
+{
+  return mBrowserModel;
+}
+
 void QgsAbstractDataSourceWidget::setupButtons( QDialogButtonBox *buttonBox )
 {
   buttonBox->setStandardButtons( QDialogButtonBox::Apply | QDialogButtonBox::Close | QDialogButtonBox::Help );
@@ -58,6 +63,11 @@ void QgsAbstractDataSourceWidget::setMapCanvas( const QgsMapCanvas *mapCanvas )
   mMapCanvas = mapCanvas;
 }
 
+void QgsAbstractDataSourceWidget::setBrowserModel( QgsBrowserModel *model )
+{
+  mBrowserModel = model;
+}
+
 void QgsAbstractDataSourceWidget::addButtonClicked()
 {
 }

diff --git a/src/gui/qgsabstractdatasourcewidget.h b/src/gui/qgsabstractdatasourcewidget.h
@@ -29,6 +29,7 @@
 #include <QDialogButtonBox>
 
 class QgsMapCanvas;
+class QgsBrowserModel;
 
 
 /**
@@ -51,6 +52,13 @@ class GUI_EXPORT QgsAbstractDataSourceWidget : public QDialog
      */
     void setMapCanvas( const QgsMapCanvas *mapCanvas );
 
+    /**
+     * Sets a browser \a model to use with the widget.
+     *
+     * \see browserModel()
+     * \since QGIS 3.18
+     */
+    void setBrowserModel( QgsBrowserModel *model );
 
   public slots:
 
@@ -166,6 +174,13 @@ class GUI_EXPORT QgsAbstractDataSourceWidget : public QDialog
     //! Returns the map canvas (can be NULLPTR)
     const QgsMapCanvas *mapCanvas() const;
 
+    /**
+     * Returns the associated browser model (may be NULLPTR).
+     *
+     * \since QGIS 3.18
+     */
+    QgsBrowserModel *browserModel();
+
     //! Connect the ok and apply/add buttons to the slots
     void setupButtons( QDialogButtonBox *buttonBox );
 
@@ -176,6 +191,7 @@ class GUI_EXPORT QgsAbstractDataSourceWidget : public QDialog
     QPushButton *mAddButton  = nullptr;
     QgsProviderRegistry::WidgetMode mWidgetMode;
     QgsMapCanvas const *mMapCanvas = nullptr;
+    QgsBrowserModel *mBrowserModel = nullptr;
 
 };
 

diff --git a/src/gui/qgsdatasourcemanagerdialog.cpp b/src/gui/qgsdatasourcemanagerdialog.cpp
@@ -35,6 +35,7 @@ QgsDataSourceManagerDialog::QgsDataSourceManagerDialog( QgsBrowserGuiModel *brow
   , ui( new Ui::QgsDataSourceManagerDialog )
   , mPreviousRow( -1 )
   , mMapCanvas( canvas )
+  , mBrowserModel( browserModel )
 {
   ui->setupUi( this );
   ui->verticalLayout_2->setSpacing( 6 );
@@ -53,7 +54,7 @@ QgsDataSourceManagerDialog::QgsDataSourceManagerDialog( QgsBrowserGuiModel *brow
   connect( ui->mOptionsListWidget, &QListWidget::currentRowChanged, this, &QgsDataSourceManagerDialog::setCurrentPage );
 
   // BROWSER Add the browser widget to the first stacked widget page
-  mBrowserWidget = new QgsBrowserDockWidget( QStringLiteral( "Browser" ), browserModel, this );
+  mBrowserWidget = new QgsBrowserDockWidget( QStringLiteral( "Browser" ), mBrowserModel, this );
   mBrowserWidget->setFeatures( QDockWidget::NoDockWidgetFeatures );
   ui->mOptionsStackedWidget->addWidget( mBrowserWidget );
   mPageNames.append( QStringLiteral( "browser" ) );
@@ -65,7 +66,7 @@ QgsDataSourceManagerDialog::QgsDataSourceManagerDialog( QgsBrowserGuiModel *brow
 
   // Add provider dialogs
   const QList<QgsSourceSelectProvider *> sourceSelectProviders = QgsGui::sourceSelectProviderRegistry()->providers( );
-  for ( const auto &provider : sourceSelectProviders )
+  for ( QgsSourceSelectProvider *provider : sourceSelectProviders )
   {
     QgsAbstractDataSourceWidget *dlg = provider->createDataSourceWidget( this );
     if ( !dlg )
@@ -157,6 +158,8 @@ void QgsDataSourceManagerDialog::addProviderDialog( QgsAbstractDataSourceWidget
   {
     dlg->setMapCanvas( mMapCanvas );
   }
+  dlg->setBrowserModel( mBrowserModel );
+
   connect( dlg, &QgsAbstractDataSourceWidget::rejected, this, &QgsDataSourceManagerDialog::reject );
   connect( dlg, &QgsAbstractDataSourceWidget::accepted, this, &QgsDataSourceManagerDialog::accept );
   makeConnections( dlg, providerKey );

diff --git a/src/gui/qgsdatasourcemanagerdialog.h b/src/gui/qgsdatasourcemanagerdialog.h
@@ -167,6 +167,7 @@ class GUI_EXPORT QgsDataSourceManagerDialog : public QgsOptionsDialogBase, priva
     // Map canvas
     QgsMapCanvas *mMapCanvas = nullptr;
     QgsMessageBar *mMessageBar = nullptr;
+    QgsBrowserGuiModel *mBrowserModel = nullptr;
 
 };
 

diff --git a/src/providers/arcgisrest/CMakeLists.txt b/src/providers/arcgisrest/CMakeLists.txt
@@ -49,7 +49,7 @@ if (WITH_GUI)
     qgsafsprovidergui.cpp
     qgsafssourceselect.cpp
     qgsarcgisrestdataitemguiprovider.cpp
-    qgsarcgisservicesourceselect.cpp
+    qgsarcgisrestsourceselect.cpp
     qgsnewarcgisrestconnection.cpp
   )
 endif()
@@ -88,29 +88,13 @@ set (AMS_SRCS
   qgsamsprovider.cpp
 )
 
-if (WITH_GUI)
-  set(AMS_SRCS ${AMS_SRCS}
-    qgsamsprovidergui.cpp
-    qgsamssourceselect.cpp
-    qgsarcgisservicesourceselect.cpp
-    qgsnewarcgisrestconnection.cpp
-  )
-endif()
-
 add_library(arcgismapserverprovider MODULE ${AMS_SRCS})
 
 target_link_libraries(arcgismapserverprovider
   qgis_core
   ${QCA_LIBRARY}
 )
 
-if (WITH_GUI)
-  target_link_libraries(arcgismapserverprovider
-    qgis_gui
-  )
-  add_dependencies(arcgismapserverprovider ui)
-endif()
-
 install (TARGETS arcgismapserverprovider
   RUNTIME DESTINATION ${QGIS_PLUGIN_DIR}
   LIBRARY DESTINATION ${QGIS_PLUGIN_DIR})
diff --git a/src/providers/arcgisrest/qgsafssourceselect.cpp b/src/providers/arcgisrest/qgsafssourceselect.cpp
@@ -28,12 +28,13 @@
 
 
 QgsAfsSourceSelect::QgsAfsSourceSelect( QWidget *parent, Qt::WindowFlags fl, QgsProviderRegistry::WidgetMode widgetMode )
-  : QgsArcGisServiceSourceSelect( QStringLiteral( "ARCGISFEATURESERVER" ), QgsArcGisServiceSourceSelect::FeatureService, parent, fl, widgetMode )
+  : QgsArcGisRestSourceSelect( QStringLiteral( "ARCGISFEATURESERVER" ), QgsArcGisRestSourceSelect::FeatureService, parent, fl, widgetMode )
 {
 }
 
 bool QgsAfsSourceSelect::connectToService( const QgsOwsConnection &connection )
 {
+#if 0
   QString errorTitle, errorMessage;
 
   const QString authcfg = connection.uri().authConfigId();
@@ -148,6 +149,7 @@ bool QgsAfsSourceSelect::connectToService( const QgsOwsConnection &connection )
 
   if ( !visitItemsRecursive( baseUrl, nullptr ) )
     QMessageBox::warning( this, tr( "Error" ), tr( "Failed to retrieve service capabilities:\n%1: %2" ).arg( errorTitle, errorMessage ) );
+#endif
 
   return true;
 }
@@ -182,7 +184,7 @@ void QgsAfsSourceSelect::buildQuery( const QgsOwsConnection &connection, const Q
   if ( d.exec() == QDialog::Accepted )
   {
     QgsDebugMsg( "Expression text = " + w->expressionText() );
-    mModelProxy->setData( filterIndex, QVariant( w->expressionText() ) );
+    //mModelProxy->setData( filterIndex, QVariant( w->expressionText() ) );
   }
 }
 

diff --git a/src/providers/arcgisrest/qgsafssourceselect.h b/src/providers/arcgisrest/qgsafssourceselect.h
@@ -20,11 +20,11 @@
 
 #include "qgsguiutils.h"
 #include "qgsproviderregistry.h"
-#include "qgsarcgisservicesourceselect.h"
+#include "qgsarcgisrestsourceselect.h"
 
 class QCheckBox;
 
-class QgsAfsSourceSelect: public QgsArcGisServiceSourceSelect
+class QgsAfsSourceSelect: public QgsArcGisRestSourceSelect
 {
     Q_OBJECT
 

diff --git a/src/providers/arcgisrest/qgsamssourceselect.cpp b/src/providers/arcgisrest/qgsamssourceselect.cpp
@@ -27,14 +27,14 @@
 
 
 QgsAmsSourceSelect::QgsAmsSourceSelect( QWidget *parent, Qt::WindowFlags fl, QgsProviderRegistry::WidgetMode widgetMode )
-  : QgsArcGisServiceSourceSelect( QStringLiteral( "ARCGISMAPSERVER" ), QgsArcGisServiceSourceSelect::MapService, parent, fl, widgetMode )
+  : QgsArcGisRestSourceSelect( QStringLiteral( "ARCGISMAPSERVER" ), QgsArcGisRestSourceSelect::MapService, parent, fl, widgetMode )
 {
 }
 
 bool QgsAmsSourceSelect::connectToService( const QgsOwsConnection &connection )
 {
   QString errorTitle, errorMessage;
-
+#if 0
   const QString authcfg = connection.uri().authConfigId();
   const QString baseUrl = connection.uri().param( QStringLiteral( "url" ) );
   const QString referer = connection.uri().param( QStringLiteral( "referer" ) );
@@ -149,6 +149,7 @@ bool QgsAmsSourceSelect::connectToService( const QgsOwsConnection &connection )
 
   if ( !visitItemsRecursive( baseUrl, nullptr ) )
     QMessageBox::warning( this, tr( "Error" ), tr( "Failed to retrieve service capabilities:\n%1: %2" ).arg( errorTitle, errorMessage ) );
+#endif
 
   return true;
 }

diff --git a/src/providers/arcgisrest/qgsamssourceselect.h b/src/providers/arcgisrest/qgsamssourceselect.h
@@ -18,12 +18,12 @@
 #ifndef QGSAMSSOURCESELECT_H
 #define QGSAMSSOURCESELECT_H
 
-#include "qgsarcgisservicesourceselect.h"
+#include "qgsarcgisrestsourceselect.h"
 #include "qgsproviderregistry.h"
 
 class QCheckBox;
 
-class QgsAmsSourceSelect: public QgsArcGisServiceSourceSelect
+class QgsAmsSourceSelect: public QgsArcGisRestSourceSelect
 {
     Q_OBJECT
 

diff --git a/src/providers/arcgisrest/qgsarcgisrestdataitems.cpp b/src/providers/arcgisrest/qgsarcgisrestdataitems.cpp
@@ -55,7 +55,7 @@ QVector<QgsDataItem *> QgsArcGisRestRootItem::createChildren()
 QWidget *QgsArcGisRestRootItem::paramWidget()
 {
   QgsAfsSourceSelect *select = new QgsAfsSourceSelect( nullptr, Qt::WindowFlags(), QgsProviderRegistry::WidgetMode::Manager );
-  connect( select, &QgsArcGisServiceSourceSelect::connectionsChanged, this, &QgsArcGisRestRootItem::onConnectionsChanged );
+  connect( select, &QgsArcGisRestSourceSelect::connectionsChanged, this, &QgsArcGisRestRootItem::onConnectionsChanged );
   return select;
 }