Refactor to simply and avoid QObject based QgsSslErrorHandler

Author: nyalldawson

src/app/CMakeLists.txt

@@ -252,7 +252,6 @@ SET (QGIS_APP_MOC_HDRS
   qgsalignrasterdialog.h
   qgsappbrowserproviders.h
   qgsappscreenshots.h
-  qgsappsslerrorhandler.h
   qgsjoindialog.h
   qgsaddtaborgroup.h
   qgsannotationwidget.h

src/app/qgsappsslerrorhandler.h

@@ -19,9 +19,8 @@
 
 class QgsAppSslErrorHandler : public QgsSslErrorHandler
 {
-    Q_OBJECT
 
-  public slots:
+  public:
 
     void handleSslErrors( QNetworkReply *reply, const QList<QSslError> &errors ) override;
 

src/core/qgsnetworkaccessmanager.cpp

@@ -127,10 +127,7 @@ QgsNetworkAccessManager::QgsNetworkAccessManager( QObject *parent )
 void QgsNetworkAccessManager::setSslErrorHandler( std::unique_ptr<QgsSslErrorHandler> handler )
 {
   Q_ASSERT( sMainNAM == this );
-
   mSslErrorHandler = std::move( handler );
-  connect( this, &QgsNetworkAccessManager::sslErrorsOccurred, mSslErrorHandler.get(), &QgsSslErrorHandler::onSslErrors );
-  connect( mSslErrorHandler.get(), &QgsSslErrorHandler::sslErrorsHandled, this, &QgsNetworkAccessManager::afterSslErrorHandled );
 }
 
 void QgsNetworkAccessManager::insertProxyFactory( QNetworkProxyFactory *factory )
@@ -256,11 +253,13 @@ QNetworkReply *QgsNetworkAccessManager::createRequest( QNetworkAccessManager::Op
   return reply;
 }
 
+#ifndef QT_NO_SSL
 void QgsNetworkAccessManager::unlockAfterSslErrorHandled()
 {
   Q_ASSERT( QThread::currentThread() == QApplication::instance()->thread() );
   mSslErrorWaitCondition.wakeOne();
 }
+#endif
 
 void QgsNetworkAccessManager::abortRequest()
 {
@@ -346,6 +345,12 @@ void QgsNetworkAccessManager::afterSslErrorHandled( QNetworkReply *reply )
     qobject_cast< QgsNetworkAccessManager *>( reply->manager() )->unlockAfterSslErrorHandled(); // safe to call directly - the other thread will be stuck waiting for us
   }
 }
+
+void QgsNetworkAccessManager::handleSslErrors( QNetworkReply *reply, const QList<QSslError> &errors )
+{
+  mSslErrorHandler->handleSslErrors( reply, errors );
+  afterSslErrorHandled( reply );
+}
 #endif
 
 QString QgsNetworkAccessManager::cacheLoadControlName( QNetworkRequest::CacheLoadControl control )
@@ -422,14 +427,17 @@ void QgsNetworkAccessManager::setupDefaultProxyAndCache( Qt::ConnectionType conn
              connectionType );
 
     connect( this, &QgsNetworkAccessManager::requestEncounteredSslErrors, sMainNAM, &QgsNetworkAccessManager::requestEncounteredSslErrors );
-    connect( this, &QgsNetworkAccessManager::sslErrorsOccurred, sMainNAM, &QgsNetworkAccessManager::sslErrorsOccurred );
 #endif
   }
   else
   {
+#ifndef QT_NO_SSL
     setSslErrorHandler( qgis::make_unique< QgsSslErrorHandler >() );
+#endif
   }
-
+#ifndef QT_NO_SSL
+  connect( this, &QgsNetworkAccessManager::sslErrorsOccurred, sMainNAM, &QgsNetworkAccessManager::handleSslErrors );
+#endif
   connect( this, &QNetworkAccessManager::finished, this, &QgsNetworkAccessManager::onReplyFinished );
 
   // check if proxy is enabled
@@ -539,14 +547,8 @@ QgsNetworkRequestParameters::QgsNetworkRequestParameters( QNetworkAccessManager:
 // QgsSslErrorHandler
 //
 
-void QgsSslErrorHandler::onSslErrors( QNetworkReply *reply, const QList<QSslError> &errors )
-{
-  Q_ASSERT( QThread::currentThread() == QApplication::instance()->thread() );
-  handleSslErrors( reply, errors );
-  emit sslErrorsHandled( reply );
-}
-
 void QgsSslErrorHandler::handleSslErrors( QNetworkReply *reply, const QList<QSslError> & )
 {
+  Q_UNUSED( reply );
   QgsDebugMsg( QStringLiteral( "SSL errors occurred accessing URL:\n%1" ).arg( reply->request().url().toString() ) );
 }

src/core/qgsnetworkaccessmanager.h

@@ -162,22 +162,14 @@ class QgsNetworkAccessManager;
  * SSL error handler is set by calling QgsNetworkAccessManager::setSslErrorHandler().
  * By default an instance of the logging-only QgsSslErrorHandler base class is used.
  *
- * \since 3.6
+ * \since QGIS 3.6
  */
-class CORE_EXPORT QgsSslErrorHandler : public QObject
+class CORE_EXPORT QgsSslErrorHandler
 {
-    Q_OBJECT
-
-///@cond PRIVATE
-  public slots:
 
-    /**
-     * Called whenever SSL \a errors are encountered during a network \a reply.
-     */
-    void onSslErrors( QNetworkReply *reply, const QList<QSslError> &errors );
-///@endcond
+  public:
 
-  protected:
+    virtual ~QgsSslErrorHandler() = default;
 
     /**
      * Called whenever SSL \a errors are encountered during a network \a reply.
@@ -191,17 +183,6 @@ class CORE_EXPORT QgsSslErrorHandler : public QObject
      */
     virtual void handleSslErrors( QNetworkReply *reply, const QList<QSslError> &errors );
 
-  signals:
-
-    /**
-     * Emitted when the SSL errors for a \a reply have been handled (i.e. the
-     * subclass' handleSslErrors() implementation has completed).
-     *
-     * It is not necessary for subclasses to emit this signal manually, it will
-     * automatically be emitted at the correct time.
-     */
-    void sslErrorsHandled( QNetworkReply *reply );
-
 };
 #endif
 
@@ -414,16 +395,17 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager
     void onReplyDownloadProgress( qint64 bytesReceived, qint64 bytesTotal );
 #ifndef QT_NO_SSL
     void onReplySslErrors( const QList<QSslError> &errors );
-#endif
-
-    void afterSslErrorHandled( QNetworkReply *reply );
 
+    void handleSslErrors( QNetworkReply *reply, const QList<QSslError> &errors );
+#endif
   protected:
     QNetworkReply *createRequest( QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *outgoingData = nullptr ) override;
 
   private:
+#ifndef QT_NO_SSL
     void unlockAfterSslErrorHandled();
-
+    void afterSslErrorHandled( QNetworkReply *reply );
+#endif
     QList<QNetworkProxyFactory *> mProxyFactories;
     QNetworkProxy mFallbackProxy;
     QStringList mExcludedURLs;

tests/src/core/testqgsnetworkaccessmanager.cpp

@@ -61,9 +61,7 @@ class BackgroundRequest : public QThread
 
 class TestSslErrorHandler : public QgsSslErrorHandler
 {
-    Q_OBJECT
-
-  protected:
+  public:
 
     void handleSslErrors( QNetworkReply *reply, const QList<QSslError> &errors ) override
     {