Update "new connection dialog" to use new connectDb() call

qgis · Sep 9, 2021 · 4800b29 · 4800b29
1 parent 176a35e
commit 4800b29
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 74 deletions.
diff --git a/src/providers/mssql/qgsmssqlconnection.cpp b/src/providers/mssql/qgsmssqlconnection.cpp
@@ -219,24 +219,24 @@ QStringList QgsMssqlConnection::schemas( const QString &uri, QString *errorMessa
 {
   const QgsDataSourceUri dsUri( uri );
 
-// connect to database
+  // connect to database
   std::shared_ptr<QgsMssqlDatabase> db = QgsMssqlDatabase::connectDb( dsUri.service(), dsUri.host(), dsUri.database(), dsUri.username(), dsUri.password() );
 
-  return schemas( db->db(), errorMessage );
+  return schemas( db, errorMessage );
 }
 
-QStringList QgsMssqlConnection::schemas( QSqlDatabase &dataBase, QString *errorMessage )
+QStringList QgsMssqlConnection::schemas( std::shared_ptr<QgsMssqlDatabase> db, QString *errorMessage )
 {
-  if ( !QgsMssqlDatabase::openDatabase( dataBase ) )
+  if ( !db->isValid() )
   {
     if ( errorMessage )
-      *errorMessage = dataBase.lastError().text();
+      *errorMessage = db->errorText();
     return QStringList();
   }
 
   const QString sql = QStringLiteral( "select s.name as schema_name from sys.schemas s" );
 
-  QSqlQuery q = QSqlQuery( dataBase );
+  QSqlQuery q = QSqlQuery( db->db() );
   q.setForwardOnly( true );
   if ( !q.exec( sql ) )
   {

diff --git a/src/providers/mssql/qgsmssqlconnection.h b/src/providers/mssql/qgsmssqlconnection.h
@@ -26,6 +26,8 @@
 class QString;
 class QSqlDatabase;
 
+class QgsMssqlDatabase;
+
 /**
  * \class QgsMssqlConnection
  * Connection handler for SQL Server provider
@@ -186,7 +188,7 @@ class QgsMssqlConnection
      * Returns a list of all schemas on the \a dataBase.
      * \since QGIS 3.18
      */
-    static QStringList schemas( QSqlDatabase &dataBase, QString *errorMessage );
+    static QStringList schemas( std::shared_ptr<QgsMssqlDatabase> db, QString *errorMessage );
 
     /**
      * Returns true if the given \a schema is a system schema.

diff --git a/src/providers/mssql/qgsmssqldatabase.cpp b/src/providers/mssql/qgsmssqldatabase.cpp
@@ -33,30 +33,30 @@ QRecursiveMutex QgsMssqlDatabase::sMutex;
 QMap<QString, std::weak_ptr<QgsMssqlDatabase> > QgsMssqlDatabase::sConnections;
 
 
-QString _connName( const QString &service, const QString &host, const QString &database, bool transaction )
+QString QgsMssqlDatabase::connectionName( const QString &service, const QString &host, const QString &database, bool transaction )
 {
-  QString connectionName;
+  QString connName;
   if ( service.isEmpty() )
   {
     if ( !host.isEmpty() )
-      connectionName = host + '.';
+      connName = host + '.';
 
     if ( database.isEmpty() )
     {
       QgsDebugMsg( QStringLiteral( "QgsMssqlProvider database name not specified" ) );
       return QString();
     }
 
-    connectionName += database;
+    connName += database;
   }
   else
-    connectionName = service;
+    connName = service;
 
   if ( !transaction )
-    connectionName += QStringLiteral( ":0x%1" ).arg( reinterpret_cast<quintptr>( QThread::currentThread() ), 2 * QT_POINTER_SIZE, 16, QLatin1Char( '0' ) );
+    connName += QStringLiteral( ":0x%1" ).arg( reinterpret_cast<quintptr>( QThread::currentThread() ), 2 * QT_POINTER_SIZE, 16, QLatin1Char( '0' ) );
   else
-    connectionName += ":transaction";
-  return connectionName;
+    connName += ":transaction";
+  return connName;
 }
 
 
@@ -72,12 +72,12 @@ std::shared_ptr<QgsMssqlDatabase> QgsMssqlDatabase::connectDb( const QString &se
 
   QMutexLocker locker( &sMutex );
 
-  QString connName = _connName( service, host, database, transaction );
+  QString connName = connectionName( service, host, database, transaction );
 
   if ( sConnections.contains( connName ) && !sConnections[connName].expired() )
     return sConnections[connName].lock();
 
-  QSqlDatabase db = QgsMssqlDatabase::getDatabase( service, host, database, username, password, transaction );
+  QSqlDatabase db = getDatabase( service, host, database, username, password, transaction );
 
   std::shared_ptr<QgsMssqlDatabase> c( new QgsMssqlDatabase( db, transaction ) );
 
@@ -104,9 +104,12 @@ QgsMssqlDatabase::QgsMssqlDatabase( const QSqlDatabase &db, bool transaction )
 #endif
   }
 
-  if ( !QgsMssqlDatabase::openDatabase( mDB ) )
+  if ( !mDB.isOpen() )
   {
-    QgsDebugMsg( "Failed to open MSSQL database: " + mDB.lastError().text() );
+    if ( !mDB.open() )
+    {
+      QgsDebugMsg( "Failed to open MSSQL database: " + mDB.lastError().text() );
+    }
   }
 }
 
@@ -116,8 +119,6 @@ QgsMssqlDatabase::~QgsMssqlDatabase()
   if ( mDB.isOpen() )
   {
     mDB.close();
-
-    // TODO: also remove DB?
   }
 }
 
@@ -133,7 +134,7 @@ QSqlDatabase QgsMssqlDatabase::getDatabase( const QString &service, const QStrin
   // that the connection cleanup on thread finalization happens in a predictable order
   QMutexLocker locker( &sMutex );
 
-  const QString threadSafeConnectionName = _connName( service, host, database, transaction ); //dbConnectionName( connectionName );
+  const QString threadSafeConnectionName = connectionName( service, host, database, transaction );
 
   if ( !QSqlDatabase::contains( threadSafeConnectionName ) )
   {
@@ -218,15 +219,3 @@ QSqlDatabase QgsMssqlDatabase::getDatabase( const QString &service, const QStrin
   // QgsDebugMsg( connectionString );
   return db;
 }
-
-bool QgsMssqlDatabase::openDatabase( QSqlDatabase &db )
-{
-  if ( !db.isOpen() )
-  {
-    if ( !db.open() )
-    {
-      return false;
-    }
-  }
-  return true;
-}
diff --git a/src/providers/mssql/qgsmssqldatabase.h b/src/providers/mssql/qgsmssqldatabase.h
@@ -56,16 +56,6 @@ class QgsMssqlDatabase
     static std::shared_ptr<QgsMssqlDatabase> connectDb( const QString &uri, bool transaction = false );
     static std::shared_ptr<QgsMssqlDatabase> connectDb( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, bool transaction = false );
 
-    /**
-     * Returns a QSqlDatabase object for queries to SQL Server.
-     *
-     * The database may not be open -- openDatabase() should be called to
-     * ensure that it is ready for use.
-     */
-    static QSqlDatabase getDatabase( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, bool transaction = false );
-
-    static bool openDatabase( QSqlDatabase &db );
-
     /////
 
     ~QgsMssqlDatabase();
@@ -93,6 +83,16 @@ class QgsMssqlDatabase
 
     friend class QgsMssqlQuery;
 
+    static QString connectionName( const QString &service, const QString &host, const QString &database, bool transaction );
+
+    /**
+     * Returns a QSqlDatabase object for queries to SQL Server.
+     *
+     * The database may not be open -- openDatabase() should be called to
+     * ensure that it is ready for use.
+     */
+    static QSqlDatabase getDatabase( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, bool transaction = false );
+
     static QMap<QString, std::weak_ptr<QgsMssqlDatabase> > sConnections;
 
 #if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)

diff --git a/src/providers/mssql/qgsmssqlnewconnection.cpp b/src/providers/mssql/qgsmssqlnewconnection.cpp
@@ -224,15 +224,12 @@ bool QgsMssqlNewConnection::testConnection( const QString &testDatabase )
     return false;
   }
 
-  QSqlDatabase db = getDatabase( testDatabase );
+  std::shared_ptr<QgsMssqlDatabase> db = getDatabase( testDatabase );
 
-  if ( db.isOpen() )
-    db.close();
-
-  if ( !db.open() )
+  if ( !db->isValid() )
   {
     bar->clearWidgets();
-    bar->pushWarning( tr( "Error opening connection" ), db.lastError().text() );
+    bar->pushWarning( tr( "Error opening connection" ), db->errorText() );
     return false;
   }
   else
@@ -252,11 +249,11 @@ void QgsMssqlNewConnection::listDatabases()
   listDatabase->clear();
   const QString queryStr = QStringLiteral( "SELECT name FROM master..sysdatabases WHERE name NOT IN ('master', 'tempdb', 'model', 'msdb')" );
 
-  QSqlDatabase db = getDatabase( QStringLiteral( "master" ) );
+  std::shared_ptr<QgsMssqlDatabase> db = getDatabase( QStringLiteral( "master" ) );
 
-  if ( db.open() )
+  if ( db->isValid() )
   {
-    QSqlQuery query = QSqlQuery( db );
+    QSqlQuery query = QSqlQuery( db->db() );
     query.setForwardOnly( true );
     ( void )query.exec( queryStr );
 
@@ -274,7 +271,6 @@ void QgsMssqlNewConnection::listDatabases()
       }
       listDatabase->setCurrentRow( 0 );
     }
-    db.close();
   }
 
   for ( int i = 0; i < listDatabase->count(); ++i )
@@ -293,7 +289,7 @@ void QgsMssqlNewConnection::showHelp()
   QgsHelp::openHelp( QStringLiteral( "managing_data_source/opening_data.html#connecting-to-mssql-spatial" ) );
 }
 
-QSqlDatabase QgsMssqlNewConnection::getDatabase( const QString &name ) const
+std::shared_ptr<QgsMssqlDatabase> QgsMssqlNewConnection::getDatabase( const QString &name ) const
 {
   QString database;
   QListWidgetItem *item = listDatabase->currentItem();
@@ -306,11 +302,11 @@ QSqlDatabase QgsMssqlNewConnection::getDatabase( const QString &name ) const
     database = item->text();
   }
 
-  return QgsMssqlDatabase::getDatabase( txtService->text().trimmed(),
-                                        txtHost->text().trimmed(),
-                                        database,
-                                        txtUsername->text().trimmed(),
-                                        txtPassword->text().trimmed() );
+  return QgsMssqlDatabase::connectDb( txtService->text().trimmed(),
+                                      txtHost->text().trimmed(),
+                                      database,
+                                      txtUsername->text().trimmed(),
+                                      txtPassword->text().trimmed() );
 }
 
 
@@ -331,7 +327,7 @@ void QgsMssqlNewConnection::onCurrentDataBaseChange()
   if ( listDatabase->currentItem() )
     databaseName = listDatabase->currentItem()->text();
 
-  QSqlDatabase db = getDatabase();
+  std::shared_ptr<QgsMssqlDatabase> db = getDatabase();
 
   QStringList schemasList = QgsMssqlConnection::schemas( db, nullptr );
   int i = 0;
@@ -377,33 +373,27 @@ void QgsMssqlNewConnection::onPrimaryKeyFromGeometryToggled( bool checked )
 
 bool QgsMssqlNewConnection::testExtentInGeometryColumns() const
 {
-  QSqlDatabase db = getDatabase();
-
-  if ( !QgsMssqlDatabase::openDatabase( db ) )
+  std::shared_ptr<QgsMssqlDatabase> db = getDatabase();
+  if ( !db->isValid() )
     return false;
 
   const QString queryStr = QStringLiteral( "SELECT qgis_xmin,qgis_xmax,qgis_ymin,qgis_ymax FROM geometry_columns" );
-  QSqlQuery query = QSqlQuery( db );
+  QSqlQuery query = QSqlQuery( db->db() );
   const bool test = query.exec( queryStr );
 
-  db.close();
-
   return test;
 }
 
 bool QgsMssqlNewConnection::testPrimaryKeyInGeometryColumns() const
 {
-  QSqlDatabase db = getDatabase();
-
-  if ( !QgsMssqlDatabase::openDatabase( db ) )
+  std::shared_ptr<QgsMssqlDatabase> db = getDatabase();
+  if ( !db->isValid() )
     return false;
 
   const QString queryStr = QStringLiteral( "SELECT qgis_pkey FROM geometry_columns" );
-  QSqlQuery query = QSqlQuery( db );
+  QSqlQuery query = QSqlQuery( db->db() );
   const bool test = query.exec( queryStr );
 
-  db.close();
-
   return test;
 }
 

diff --git a/src/providers/mssql/qgsmssqlnewconnection.h b/src/providers/mssql/qgsmssqlnewconnection.h
@@ -23,6 +23,8 @@
 
 #include <QSqlDatabase>
 
+class QgsMssqlDatabase;
+
 /**
  * \class QgsMssqlNewConnection
  * \brief Dialog to allow the user to configure and save connection
@@ -99,7 +101,7 @@ class QgsMssqlNewConnection : public QDialog, private Ui::QgsMssqlNewConnectionB
     QVariantMap mSchemaSettings; //store the schema settings edited during this QDialog life time
     SchemaModel mSchemaModel;
 
-    QSqlDatabase getDatabase( const QString &name = QString() ) const;
+    std::shared_ptr<QgsMssqlDatabase> getDatabase( const QString &name = QString() ) const;
 
     bool testExtentInGeometryColumns() const;