From 136381c8e45397d3fb118aa6491ea8f200fab12a Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Mon, 7 Jun 2021 14:49:22 +0200 Subject: [PATCH] [Oracle] Fixes #40001 : Use QgsTask to retrieve available tables so we can properly cancel it --- src/providers/oracle/CMakeLists.txt | 2 +- .../oracle/qgsoraclecolumntypetask.cpp | 92 +++++++++++++++++++ .../oracle/qgsoraclecolumntypetask.h | 67 ++++++++++++++ src/providers/oracle/qgsoracledataitems.cpp | 62 ++++++------- src/providers/oracle/qgsoracledataitems.h | 9 +- .../oracle/qgsoraclesourceselect.cpp | 47 ++++------ src/providers/oracle/qgsoraclesourceselect.h | 12 +-- 7 files changed, 218 insertions(+), 73 deletions(-) create mode 100644 src/providers/oracle/qgsoraclecolumntypetask.cpp create mode 100644 src/providers/oracle/qgsoraclecolumntypetask.h diff --git a/src/providers/oracle/CMakeLists.txt b/src/providers/oracle/CMakeLists.txt index e6b100eddb55..97823b63f7bc 100644 --- a/src/providers/oracle/CMakeLists.txt +++ b/src/providers/oracle/CMakeLists.txt @@ -10,7 +10,7 @@ set(ORACLE_SRCS qgsoraclenewconnection.cpp qgsoracletablecache.cpp qgsoracletablemodel.cpp - qgsoraclecolumntypethread.cpp + qgsoraclecolumntypetask.cpp qgsoraclefeatureiterator.cpp qgsoracleconnpool.cpp qgsoracleexpressioncompiler.cpp diff --git a/src/providers/oracle/qgsoraclecolumntypetask.cpp b/src/providers/oracle/qgsoraclecolumntypetask.cpp new file mode 100644 index 000000000000..c02cecc479c0 --- /dev/null +++ b/src/providers/oracle/qgsoraclecolumntypetask.cpp @@ -0,0 +1,92 @@ +/*************************************************************************** + qgscolumntypetask.cpp - lookup oracle geometry type and srid in a thread + ------------------- +begin : 3.1.2012 +copyright : (C) 2012 by Juergen E. Fischer +email : jef at norbit dot de + ***************************************************************************/ + +/*************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ + +#include "qgsoraclecolumntypetask.h" +#include "qgslogger.h" +#include "qgsoracleconnpool.h" + +#include + +QgsOracleColumnTypeTask::QgsOracleColumnTypeTask( const QString &name, const QString &limitToSchema, bool useEstimatedMetadata, bool allowGeometrylessTables ) + : QgsTask( tr( "Scanning tables for %1" ).arg( name ) ) + , mName( name ) + , mSchema( limitToSchema ) + , mUseEstimatedMetadata( useEstimatedMetadata ) + , mAllowGeometrylessTables( allowGeometrylessTables ) +{ + qRegisterMetaType( "QgsOracleLayerProperty" ); +} + +bool QgsOracleColumnTypeTask::run() +{ + QString conninfo = QgsOracleConn::toPoolName( QgsOracleConn::connUri( mName ) ); + QgsOracleConn *conn = QgsOracleConnPool::instance()->acquireConnection( conninfo ); + if ( !conn ) + { + QgsDebugMsg( "Connection failed - " + conninfo ); + return false; + } + + emit progressMessage( tr( "Retrieving tables of %1…" ).arg( mName ) ); + QVector layerProperties; + if ( !conn->supportedLayers( layerProperties, + mSchema, + QgsOracleConn::geometryColumnsOnly( mName ), + QgsOracleConn::userTablesOnly( mName ), + mAllowGeometrylessTables ) || + layerProperties.isEmpty() ) + { + return false; + } + + int i = 0, n = layerProperties.size(); + for ( QVector::iterator it = layerProperties.begin(), + end = layerProperties.end(); + it != end; ++it ) + { + QgsOracleLayerProperty &layerProperty = *it; + if ( !isCanceled() ) + { + setProgress( ( i * 100. ) / n ); + emit progressMessage( tr( "Scanning column %1.%2.%3…" ) + .arg( layerProperty.ownerName, + layerProperty.tableName, + layerProperty.geometryColName ) ); + conn->retrieveLayerTypes( layerProperty, mUseEstimatedMetadata, QgsOracleConn::onlyExistingTypes( mName ) ); + } + + if ( isCanceled() ) + { + layerProperty.types.clear(); + layerProperty.srids.clear(); + } + + // Now tell the layer list dialog box... + emit setLayerType( layerProperty ); + } + + // store the list for later use (cache) + if ( !isCanceled() ) + mLayerProperties = layerProperties; + + setProgress( 100 ); + emit progressMessage( tr( "Table retrieval finished." ) ); + + QgsOracleConnPool::instance()->releaseConnection( conn ); + + return true; +} diff --git a/src/providers/oracle/qgsoraclecolumntypetask.h b/src/providers/oracle/qgsoraclecolumntypetask.h new file mode 100644 index 000000000000..0aa9a03e3619 --- /dev/null +++ b/src/providers/oracle/qgsoraclecolumntypetask.h @@ -0,0 +1,67 @@ +/*************************************************************************** + qgsoraclecolumntypetask.h - lookup oracle geometry type and srid in a thread + ------------------- + begin : 12.12.2012 + copyright : (C) 2012 by Juergen E. Fischer + email : jef at norbit dot de + ***************************************************************************/ + +/*************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ +#ifndef QGSORACLECOLUMNTYPETASK_H +#define QGSORACLECOLUMNTYPETASK_H + +#include "qgstaskmanager.h" +#include "qgsoracleconn.h" + +// A class that determines the geometry type of a given database +// schema.table.column, with the option of doing so in a separate +// thread. + +class QgsOracleColumnTypeTask : public QgsTask +{ + Q_OBJECT + public: + + /** + * + * \param connName + * \param limitToSchema If specified, only tables from this schema will be scanned + * \param useEstimatedMetaData + * \param allowGeometrylessTables + */ + QgsOracleColumnTypeTask( const QString &connName, + const QString &limitToSchema, + bool useEstimatedMetaData, + bool allowGeometrylessTables ); + + // These functions get the layer types and pass that information out + // by emitting the setLayerType() signal. + bool run() override; + + QVector layerProperties() const { return mLayerProperties; } + QString connectionName() const { return mName; } + bool useEstimatedMetadata() const { return mUseEstimatedMetadata; } + bool allowGeometrylessTables() const { return mAllowGeometrylessTables; } + + signals: + void setLayerType( const QgsOracleLayerProperty &layerProperty ); + void progressMessage( const QString &message ); + + private: + QgsOracleColumnTypeTask() = default; + + QString mName; + QString mSchema; + bool mUseEstimatedMetadata = false; + bool mAllowGeometrylessTables = false; + QVector mLayerProperties; +}; + +#endif // QGSORACLECOLUMNTYPETASK_H diff --git a/src/providers/oracle/qgsoracledataitems.cpp b/src/providers/oracle/qgsoracledataitems.cpp index 57d2cd65293a..8f0009dcafe6 100644 --- a/src/providers/oracle/qgsoracledataitems.cpp +++ b/src/providers/oracle/qgsoracledataitems.cpp @@ -16,7 +16,7 @@ #include "qgsoracletablemodel.h" #include "qgsoraclenewconnection.h" -#include "qgsoraclecolumntypethread.h" +#include "qgsoraclecolumntypetask.h" #include "qgsoracleprovider.h" #include "qgslogger.h" @@ -30,6 +30,7 @@ #include #include #include +#include bool deleteLayer( const QString &uri, QString &errCause ) { @@ -125,7 +126,7 @@ QgsOracleConnectionItem::QgsOracleConnectionItem( QgsDataItem *parent, const QSt : QgsDataCollectionItem( parent, name, path, QStringLiteral( "ORACLE" ) ) { mIconName = QStringLiteral( "mIconConnect.svg" ); - mCapabilities |= Qgis::BrowserItemCapability::Collapse; + mCapabilities |= Qgis::BrowserItemCapability::Collapse | Qgis::BrowserItemCapability::Fast; } QgsOracleConnectionItem::~QgsOracleConnectionItem() @@ -135,12 +136,14 @@ QgsOracleConnectionItem::~QgsOracleConnectionItem() void QgsOracleConnectionItem::stop() { - if ( mColumnTypeThread ) + if ( mColumnTypeTask ) { - mColumnTypeThread->stop(); - mColumnTypeThread->wait(); - delete mColumnTypeThread; - mColumnTypeThread = nullptr; + mColumnTypeTask->cancel(); + disconnect( mColumnTypeTask, nullptr, this, nullptr ); + disconnect( mColumnTypeTask, nullptr, QgsOracleRootItem::sMainWindow, nullptr ); + + // don't delete the task, taskManager takes ownership of it + mColumnTypeTask = nullptr; } } @@ -182,56 +185,49 @@ QVector QgsOracleConnectionItem::createChildren() if ( deferredDelete() ) return QVector(); - if ( !mColumnTypeThread ) + if ( !mColumnTypeTask ) { - mColumnTypeThread = new QgsOracleColumnTypeThread( mName, + mColumnTypeTask = new QgsOracleColumnTypeTask( mName, QgsOracleConn::restrictToSchema( mName ), /* useEstimatedMetadata */ true, QgsOracleConn::allowGeometrylessTables( mName ) ); - mColumnTypeTask = new QgsProxyProgressTask( tr( "Scanning tables for %1" ).arg( mName ) ); - QgsApplication::taskManager()->addTask( mColumnTypeTask ); - connect( mColumnTypeThread, &QgsOracleColumnTypeThread::setLayerType, + connect( mColumnTypeTask, &QgsOracleColumnTypeTask::setLayerType, this, &QgsOracleConnectionItem::setLayerType ); - connect( mColumnTypeThread, &QThread::started, this, &QgsOracleConnectionItem::threadStarted ); - connect( mColumnTypeThread, &QThread::finished, this, &QgsOracleConnectionItem::threadFinished ); + connect( mColumnTypeTask, &QgsTask::begun, this, &QgsOracleConnectionItem::taskStarted ); + connect( mColumnTypeTask, &QgsTask::taskCompleted, this, &QgsOracleConnectionItem::taskFinished ); + connect( mColumnTypeTask, &QgsTask::taskTerminated, this, &QgsOracleConnectionItem::taskFinished ); if ( QgsOracleRootItem::sMainWindow ) { - connect( mColumnTypeThread, &QgsOracleColumnTypeThread::progress, - mColumnTypeTask, [ = ]( int i, int n ) + connect( mColumnTypeTask, &QgsOracleColumnTypeTask::progressMessage, + QgsOracleRootItem::sMainWindow->statusBar(), [ = ]( const QString & message ) { - mColumnTypeTask->setProxyProgress( 100.0 * static_cast< double >( i ) / n ); + QgsOracleRootItem::sMainWindow->statusBar()->showMessage( message ); } ); - connect( mColumnTypeThread, SIGNAL( progressMessage( QString ) ), - QgsOracleRootItem::sMainWindow, SLOT( showStatusMessage( QString ) ) ); } - } - if ( mColumnTypeThread ) - { - mColumnTypeThread->start(); - } - else - { - setAllAsPopulated(); + QgsApplication::taskManager()->addTask( mColumnTypeTask ); } return QVector(); } -void QgsOracleConnectionItem::threadStarted() +void QgsOracleConnectionItem::taskStarted() { QgsDebugMsgLevel( QStringLiteral( "Entering." ), 3 ); } -void QgsOracleConnectionItem::threadFinished() +void QgsOracleConnectionItem::taskFinished() { - mColumnTypeTask->finalize( true ); - mColumnTypeTask = nullptr; - QgsDebugMsgLevel( QStringLiteral( "Entering." ), 3 ); - setAllAsPopulated(); + + if ( mColumnTypeTask->status() == QgsTask::Complete ) + setAllAsPopulated(); + else + setState( Qgis::BrowserItemState::NotPopulated ); + + mColumnTypeTask = nullptr; } void QgsOracleConnectionItem::setLayerType( const QgsOracleLayerProperty &layerProperty ) diff --git a/src/providers/oracle/qgsoracledataitems.h b/src/providers/oracle/qgsoracledataitems.h index e3fba7284f24..85282df695a4 100644 --- a/src/providers/oracle/qgsoracledataitems.h +++ b/src/providers/oracle/qgsoracledataitems.h @@ -33,7 +33,7 @@ class QgsOracleRootItem; class QgsOracleConnectionItem; class QgsOracleOwnerItem; class QgsOracleLayerItem; -class QgsProxyProgressTask; +class QgsOracleColumnTypeTask; class QgsOracleRootItem : public QgsConnectionsRootItem { @@ -82,14 +82,13 @@ class QgsOracleConnectionItem : public QgsDataCollectionItem void setLayerType( const QgsOracleLayerProperty &layerProperty ); - void threadStarted(); - void threadFinished(); + void taskStarted(); + void taskFinished(); private: void stop(); QMap mOwnerMap; - QgsOracleColumnTypeThread *mColumnTypeThread = nullptr; - QgsProxyProgressTask *mColumnTypeTask = nullptr; + QgsOracleColumnTypeTask *mColumnTypeTask = nullptr; void setAllAsPopulated(); }; diff --git a/src/providers/oracle/qgsoraclesourceselect.cpp b/src/providers/oracle/qgsoraclesourceselect.cpp index c08c022bdd3f..99d8c068b218 100644 --- a/src/providers/oracle/qgsoraclesourceselect.cpp +++ b/src/providers/oracle/qgsoraclesourceselect.cpp @@ -27,7 +27,7 @@ email : jef at norbit dot de #include "qgsquerybuilder.h" #include "qgsdatasourceuri.h" #include "qgsvectorlayer.h" -#include "qgsoraclecolumntypethread.h" +#include "qgsoraclecolumntypetask.h" #include "qgssettings.h" #include "qgsproxyprogresstask.h" #include "qgsgui.h" @@ -421,9 +421,9 @@ void QgsOracleSourceSelect::setLayerType( const QgsOracleLayerProperty &layerPro QgsOracleSourceSelect::~QgsOracleSourceSelect() { - if ( mColumnTypeThread ) + if ( mColumnTypeTask ) { - mColumnTypeThread->stop(); + mColumnTypeTask->cancel(); finishList(); } @@ -489,9 +489,9 @@ void QgsOracleSourceSelect::on_btnConnect_clicked() { cbxAllowGeometrylessTables->setEnabled( true ); - if ( mColumnTypeThread ) + if ( mColumnTypeTask ) { - mColumnTypeThread->stop(); + mColumnTypeTask->cancel(); return; } @@ -505,27 +505,23 @@ void QgsOracleSourceSelect::on_btnConnect_clicked() mIsConnected = true; mTablesTreeDelegate->setConnectionInfo( uri ); - mColumnTypeThread = new QgsOracleColumnTypeThread( cmbConnections->currentText(), + mColumnTypeTask = new QgsOracleColumnTypeTask( cmbConnections->currentText(), QgsOracleConn::restrictToSchema( cmbConnections->currentText() ), uri.useEstimatedMetadata(), cbxAllowGeometrylessTables->isChecked() ); - mColumnTypeTask = new QgsProxyProgressTask( tr( "Scanning tables for %1" ).arg( cmbConnections->currentText() ) ); - QgsApplication::taskManager()->addTask( mColumnTypeTask ); - connect( mColumnTypeThread, &QgsOracleColumnTypeThread::setLayerType, + connect( mColumnTypeTask, &QgsOracleColumnTypeTask::setLayerType, this, &QgsOracleSourceSelect::setLayerType ); - connect( mColumnTypeThread, &QThread::finished, - this, &QgsOracleSourceSelect::columnThreadFinished ); - connect( mColumnTypeThread, &QgsOracleColumnTypeThread::progress, - mColumnTypeTask, [ = ]( int i, int n ) - { - mColumnTypeTask->setProxyProgress( 100.0 * static_cast< double >( i ) / n ); - } ); - connect( mColumnTypeThread, &QgsOracleColumnTypeThread::progressMessage, + connect( mColumnTypeTask, &QgsTask::taskCompleted, + this, &QgsOracleSourceSelect::columnTaskFinished ); + connect( mColumnTypeTask, &QgsTask::taskTerminated, + this, &QgsOracleSourceSelect::columnTaskFinished ); + connect( mColumnTypeTask, &QgsOracleColumnTypeTask::progressMessage, this, &QgsAbstractDataSourceWidget::progressMessage ); btnConnect->setText( tr( "Stop" ) ); - mColumnTypeThread->start(); + + QgsApplication::taskManager()->addTask( mColumnTypeTask ); } void QgsOracleSourceSelect::finishList() @@ -552,19 +548,16 @@ static QgsOracleTableCache::CacheFlags _currentFlags( const QString &connName, b return flags; } -void QgsOracleSourceSelect::columnThreadFinished() +void QgsOracleSourceSelect::columnTaskFinished() { - if ( !mColumnTypeThread->isStopped() ) + if ( mColumnTypeTask->status() == QgsTask::Complete ) { - QString connName = mColumnTypeThread->connectionName(); - QgsOracleTableCache::CacheFlags flags = _currentFlags( connName, mColumnTypeThread->useEstimatedMetadata(), mColumnTypeThread->allowGeometrylessTables() ); - QgsOracleTableCache::saveToCache( connName, flags, mColumnTypeThread->layerProperties() ); + QString connName = mColumnTypeTask->connectionName(); + QgsOracleTableCache::CacheFlags flags = _currentFlags( connName, mColumnTypeTask->useEstimatedMetadata(), mColumnTypeTask->allowGeometrylessTables() ); + QgsOracleTableCache::saveToCache( connName, flags, mColumnTypeTask->layerProperties() ); } - delete mColumnTypeThread; - mColumnTypeThread = nullptr; - - mColumnTypeTask->finalize( true ); + // don't delete the task, taskManager takes ownership of it mColumnTypeTask = nullptr; btnConnect->setText( tr( "Connect" ) ); diff --git a/src/providers/oracle/qgsoraclesourceselect.h b/src/providers/oracle/qgsoraclesourceselect.h index 992609cbd033..cbc94a7cf1cd 100644 --- a/src/providers/oracle/qgsoraclesourceselect.h +++ b/src/providers/oracle/qgsoraclesourceselect.h @@ -33,10 +33,9 @@ class QPushButton; class QStringList; -class QgsOracleColumnTypeThread; +class QgsOracleColumnTypeTask; class QgisApp; class QgsOracleSourceSelect; -class QgsProxyProgressTask; class QgsOracleSourceSelectDelegate : public QItemDelegate { @@ -132,7 +131,7 @@ class QgsOracleSourceSelect : public QgsAbstractDataSourceWidget, private Ui::Qg //!Sets a new regular expression to the model void setSearchExpression( const QString ®exp ); - void columnThreadFinished(); + void columnTaskFinished(); private: typedef QPair geomPair; @@ -141,7 +140,7 @@ class QgsOracleSourceSelect : public QgsAbstractDataSourceWidget, private Ui::Qg //! try to load list of tables from local cache void loadTableFromCache(); - // queue another query for the thread + // queue another query for the task void addSearchGeometryColumn( QgsOracleLayerProperty layerProperty ); // Set the position of the database connection list to the last @@ -152,9 +151,8 @@ class QgsOracleSourceSelect : public QgsAbstractDataSourceWidget, private Ui::Qg QString fullDescription( const QString &schema, const QString &table, const QString &column, const QString &type ); // The column labels QStringList mColumnLabels; - // Our thread for doing long running queries - QgsOracleColumnTypeThread *mColumnTypeThread = nullptr; - QgsProxyProgressTask *mColumnTypeTask = nullptr; + // Our task for doing long running queries + QgsOracleColumnTypeTask *mColumnTypeTask = nullptr; QgsDataSourceUri mConnInfo; QStringList mSelectedTables; // Storage for the range of layer type icons