[netinstall] Split off requesting netinstall data into a queue-manager

This is the actual "meat" of the branch, which makes the
netinstall module request one URL at a time until one succeeds.

src/modules/netinstall/CMakeLists.txt

@@ -8,6 +8,7 @@ calamares_add_plugin( netinstall
     EXPORT_MACRO PLUGINDLLEXPORT_PRO
     SOURCES
         Config.cpp
+        LoaderQueue.cpp
         NetInstallViewStep.cpp
         NetInstallPage.cpp
         PackageTreeItem.cpp

src/modules/netinstall/Config.cpp

@@ -12,15 +12,15 @@
 
 #include "Config.h"
 
+#include "LoaderQueue.h"
+
 #include "GlobalStorage.h"
 #include "JobQueue.h"
 #include "network/Manager.h"
 #include "packages/Globals.h"
 #include "utils/Logger.h"
-#include "utils/RAII.h"
 #include "utils/Retranslator.h"
 #include "utils/Variant.h"
-#include "utils/Yaml.h"
 
 #include <QNetworkReply>
 
@@ -86,106 +86,13 @@ void
 Config::loadGroupList( const QVariantList& groupData )
 {
     m_model->setupModelData( groupData );
+    if ( m_model->rowCount() < 1 )
+    {
+        cWarning() << "NetInstall groups data was empty.";
+    }
     emit statusReady();
 }
 
-void
-Config::loadGroupList( const QUrl& url )
-{
-    if ( !url.isValid() )
-    {
-        setStatus( Status::FailedBadConfiguration );
-    }
-
-    using namespace CalamaresUtils::Network;
-
-    cDebug() << "NetInstall loading groups from" << url;
-    QNetworkReply* reply = Manager::instance().asynchronousGet(
-        url,
-        RequestOptions( RequestOptions::FakeUserAgent | RequestOptions::FollowRedirect, std::chrono::seconds( 30 ) ) );
-
-    if ( !reply )
-    {
-        cDebug() << Logger::Continuation << "request failed immediately.";
-        setStatus( Status::FailedBadConfiguration );
-    }
-    else
-    {
-        m_reply = reply;
-        connect( reply, &QNetworkReply::finished, this, &Config::receivedGroupData );
-    }
-}
-
-void
-Config::receivedGroupData()
-{
-    if ( !m_reply || !m_reply->isFinished() )
-    {
-        cWarning() << "NetInstall data called too early.";
-        setStatus( Status::FailedInternalError );
-        return;
-    }
-
-    cDebug() << "NetInstall group data received" << m_reply->size() << "bytes from" << m_reply->url();
-
-    cqDeleter< QNetworkReply > d { m_reply };
-
-    // If m_required is *false* then we still say we're ready
-    // even if the reply is corrupt or missing.
-    if ( m_reply->error() != QNetworkReply::NoError )
-    {
-        cWarning() << "unable to fetch netinstall package lists.";
-        cDebug() << Logger::SubEntry << "Netinstall reply error: " << m_reply->error();
-        cDebug() << Logger::SubEntry << "Request for url: " << m_reply->url().toString()
-                 << " failed with: " << m_reply->errorString();
-        setStatus( Status::FailedNetworkError );
-        return;
-    }
-
-    QByteArray yamlData = m_reply->readAll();
-    try
-    {
-        YAML::Node groups = YAML::Load( yamlData.constData() );
-
-        if ( groups.IsSequence() )
-        {
-            loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) );
-        }
-        else if ( groups.IsMap() )
-        {
-            auto map = CalamaresUtils::yamlMapToVariant( groups );
-            loadGroupList( map.value( "groups" ).toList() );
-        }
-        else
-        {
-            cWarning() << "NetInstall groups data does not form a sequence.";
-        }
-        if ( m_model->rowCount() < 1 )
-        {
-            cWarning() << "NetInstall groups data was empty.";
-        }
-    }
-    catch ( YAML::Exception& e )
-    {
-        CalamaresUtils::explainYamlException( e, yamlData, "netinstall groups data" );
-        setStatus( Status::FailedBadData );
-    }
-}
-
-Config::SourceItem
-Config::SourceItem::makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap )
-{
-    if ( groupsUrl == QStringLiteral( "local" ) )
-    {
-        return SourceItem { QUrl(), configurationMap.value( "groups" ).toList() };
-    }
-    else
-    {
-        return SourceItem { QUrl { groupsUrl }, QVariantList() };
-    }
-}
-
-
 void
 Config::setConfigurationMap( const QVariantMap& configurationMap )
 {
@@ -213,31 +120,24 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     const auto& groupsUrlVariant = configurationMap.value( key );
     if ( groupsUrlVariant.type() == QVariant::String )
     {
-        m_urls.append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
+        m_queue = new LoaderQueue( this );
+        m_queue->append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
     }
     else if ( groupsUrlVariant.type() == QVariant::StringList )
     {
+        m_queue = new LoaderQueue( this );
         for ( const auto& s : groupsUrlVariant.toStringList() )
         {
-            m_urls.append( SourceItem::makeSourceItem( s, configurationMap ) );
+            m_queue->append( SourceItem::makeSourceItem( s, configurationMap ) );
         }
     }
-
+    if ( m_queue )
-    QString groupsUrl = CalamaresUtils::getString( configurationMap, "groupsUrl" );
-    if ( !groupsUrl.isEmpty() )
     {
-        // Keep putting groupsUrl into the global storage,
+        connect( m_queue, &LoaderQueue::done, [this]() {
-        // even though it's no longer used for in-module data-passing.
+            m_queue->deleteLater();
-        Calamares::JobQueue::instance()->globalStorage()->insert( "groupsUrl", groupsUrl );
+            m_queue = nullptr;
-        if ( groupsUrl == QStringLiteral( "local" ) )
+        } );
-        {
+        m_queue->fetchNext();
-            QVariantList l = configurationMap.value( "groups" ).toList();
-            loadGroupList( l );
-        }
-        else
-        {
-            loadGroupList( groupsUrl );
-        }
     }
 }
 

src/modules/netinstall/Config.h

@@ -18,11 +18,11 @@
 #include "modulesystem/InstanceKey.h"
 
 #include <QObject>
-#include <QQueue>
-#include <QUrl>
 #include <QVariantMap>
 
-class QNetworkReply;
+#include <memory>
+
+class LoaderQueue;
 
 class Config : public QObject
 {
@@ -61,13 +61,6 @@ public:
     QString sidebarLabel() const;
     QString titleLabel() const;
 
-    /** @brief Retrieves the groups, with name, description and packages
-     *
-     * Loads data from the given URL. Once done, the data is parsed
-     * and passed on to the other loadGroupList() method.
-     */
-    void loadGroupList( const QUrl& url );
-
     /** @brief Fill model from parsed data.
      *
      * Fills the model with a list of groups -- which can contain
@@ -82,44 +75,20 @@ public:
      */
     void finalizeGlobalStorage( const Calamares::ModuleSystem::InstanceKey& key );
 
-signals:
+Q_SIGNALS:
     void statusChanged( QString status );  ///< Something changed
     void sidebarLabelChanged( QString label );
     void titleLabelChanged( QString label );
     void statusReady();  ///< Loading groups is complete
 
-private slots:
+private Q_SLOTS:
-    void receivedGroupData();  ///< From async-loading group data
     void retranslate();
 
 private:
-    /** @brief Data about an entry in *groupsUrl*
-     *
-     * This can be a specific URL, or "local" which uses data stored
-     * in the configuration file itself.
-     */
-    struct SourceItem
-    {
-        QUrl url;
-        QVariantList data;
-
-        bool isUrl() const { return url.isValid(); }
-        bool isLocal() const { return !data.isEmpty(); }
-        bool isValid() const { return isUrl() || isLocal(); }
-        /** @brief Create a SourceItem
-         *
-         * If the @p groupsUrl is @c "local" then the *groups* key in
-         * the @p configurationMap is used as the source; otherwise the
-         * string is used as an actual URL.
-         */
-        static SourceItem makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap );
-    };
-
-    QQueue< SourceItem > m_urls;
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel = nullptr;  // As it appears in the sidebar
     CalamaresUtils::Locale::TranslatedString* m_titleLabel = nullptr;
     PackageModel* m_model = nullptr;
-    QNetworkReply* m_reply = nullptr;  // For fetching data
+    LoaderQueue* m_queue;
     Status m_status = Status::Ok;
     bool m_required = false;
 };

src/modules/netinstall/LoaderQueue.cpp

@@ -0,0 +1,174 @@
+/*
+ *   SPDX-FileCopyrightText: 2016 Luca Giambonini <almack@chakraos.org>
+ *   SPDX-FileCopyrightText: 2016 Lisa Vitolo     <shainer@chakraos.org>
+ *   SPDX-FileCopyrightText: 2017 Kyle Robbertze  <krobbertze@gmail.com>
+ *   SPDX-FileCopyrightText: 2017-2018 2020, Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ */
+
+#include "LoaderQueue.h"
+
+#include "Config.h"
+#include "network/Manager.h"
+#include "utils/Logger.h"
+#include "utils/RAII.h"
+#include "utils/Yaml.h"
+
+#include <QNetworkReply>
+#include <QTimer>
+
+SourceItem
+SourceItem::makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap )
+{
+    if ( groupsUrl == QStringLiteral( "local" ) )
+    {
+        return SourceItem { QUrl(), configurationMap.value( "groups" ).toList() };
+    }
+    else
+    {
+        return SourceItem { QUrl { groupsUrl }, QVariantList() };
+    }
+}
+
+LoaderQueue::LoaderQueue( Config* parent )
+    : QObject( parent )
+    , m_config( parent )
+{
+}
+
+void
+LoaderQueue::append( SourceItem&& i )
+{
+    m_queue.append( std::move( i ) );
+}
+
+void
+LoaderQueue::fetchNext()
+{
+    if ( m_queue.isEmpty() )
+    {
+        m_config->setStatus( Config::Status::FailedBadData );
+        emit done();
+        return;
+    }
+
+    auto source = m_queue.takeFirst();
+    if ( source.isLocal() )
+    {
+        m_config->loadGroupList( source.data );
+        emit done();
+    }
+    else
+    {
+        fetch( source.url );
+    }
+}
+
+
+void
+LoaderQueue::fetch( const QUrl& url )
+{
+    if ( !url.isValid() )
+    {
+        m_config->setStatus( Config::Status::FailedBadConfiguration );
+    }
+
+    using namespace CalamaresUtils::Network;
+
+    cDebug() << "NetInstall loading groups from" << url;
+    QNetworkReply* reply = Manager::instance().asynchronousGet(
+        url,
+        RequestOptions( RequestOptions::FakeUserAgent | RequestOptions::FollowRedirect, std::chrono::seconds( 30 ) ) );
+
+    if ( !reply )
+    {
+        cDebug() << Logger::Continuation << "request failed immediately.";
+        m_config->setStatus( Config::Status::FailedBadConfiguration );
+    }
+    else
+    {
+        m_reply = reply;
+        connect( reply, &QNetworkReply::finished, this, &LoaderQueue::dataArrived );
+    }
+}
+
+class FetchNextUnless
+{
+public:
+    FetchNextUnless( LoaderQueue* q )
+        : m_q( q )
+    {
+    }
+    ~FetchNextUnless()
+    {
+        if ( m_q )
+        {
+            QMetaObject::invokeMethod( m_q, "fetchNext", Qt::QueuedConnection );
+        }
+    }
+    void release() { m_q = nullptr; }
+
+private:
+    LoaderQueue* m_q = nullptr;
+};
+
+void
+LoaderQueue::dataArrived()
+{
+    FetchNextUnless finished( this );
+
+    if ( !m_reply || !m_reply->isFinished() )
+    {
+        cWarning() << "NetInstall data called too early.";
+        m_config->setStatus( Config::Status::FailedInternalError );
+        return;
+    }
+
+    cDebug() << "NetInstall group data received" << m_reply->size() << "bytes from" << m_reply->url();
+
+    cqDeleter< QNetworkReply > d { m_reply };
+
+    // If m_required is *false* then we still say we're ready
+    // even if the reply is corrupt or missing.
+    if ( m_reply->error() != QNetworkReply::NoError )
+    {
+        cWarning() << "unable to fetch netinstall package lists.";
+        cDebug() << Logger::SubEntry << "Netinstall reply error: " << m_reply->error();
+        cDebug() << Logger::SubEntry << "Request for url: " << m_reply->url().toString()
+                 << " failed with: " << m_reply->errorString();
+        m_config->setStatus( Config::Status::FailedNetworkError );
+        return;
+    }
+
+    QByteArray yamlData = m_reply->readAll();
+    try
+    {
+        YAML::Node groups = YAML::Load( yamlData.constData() );
+
+        if ( groups.IsSequence() )
+        {
+            finished.release();
+            m_config->loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) );
+            emit done();
+        }
+        else if ( groups.IsMap() )
+        {
+            finished.release();
+            auto map = CalamaresUtils::yamlMapToVariant( groups );
+            m_config->loadGroupList( map.value( "groups" ).toList() );
+            emit done();
+        }
+        else
+        {
+            cWarning() << "NetInstall groups data does not form a sequence.";
+        }
+    }
+    catch ( YAML::Exception& e )
+    {
+        CalamaresUtils::explainYamlException( e, yamlData, "netinstall groups data" );
+        m_config->setStatus( Config::Status::FailedBadData );
+    }
+}

src/modules/netinstall/LoaderQueue.h

@@ -0,0 +1,67 @@
+/*
+ *   SPDX-FileCopyrightText: 2016 Luca Giambonini <almack@chakraos.org>
+ *   SPDX-FileCopyrightText: 2016 Lisa Vitolo     <shainer@chakraos.org>
+ *   SPDX-FileCopyrightText: 2017 Kyle Robbertze  <krobbertze@gmail.com>
+ *   SPDX-FileCopyrightText: 2017-2018 2020, Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ */
+
+#ifndef NETINSTALL_LOADERQUEUE_H
+#define NETINSTALL_LOADERQUEUE_H
+
+#include <QQueue>
+#include <QUrl>
+#include <QVariantList>
+
+class Config;
+class QNetworkReply;
+
+/** @brief Data about an entry in *groupsUrl*
+ *
+ * This can be a specific URL, or "local" which uses data stored
+ * in the configuration file itself.
+ */
+struct SourceItem
+{
+    QUrl url;
+    QVariantList data;
+
+    bool isUrl() const { return url.isValid(); }
+    bool isLocal() const { return !data.isEmpty(); }
+    bool isValid() const { return isUrl() || isLocal(); }
+    /** @brief Create a SourceItem
+     *
+     * If the @p groupsUrl is @c "local" then the *groups* key in
+     * the @p configurationMap is used as the source; otherwise the
+     * string is used as an actual URL.
+     */
+    static SourceItem makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap );
+};
+
+
+class LoaderQueue : public QObject
+{
+    Q_OBJECT
+public:
+    LoaderQueue( Config* parent );
+
+    void append( SourceItem&& i );
+    void fetchNext();
+
+public Q_SLOTS:
+    void fetch( const QUrl& url );
+    void dataArrived();
+
+Q_SIGNALS:
+    void done();
+
+private:
+    QQueue< SourceItem > m_queue;
+    Config* m_config = nullptr;
+    QNetworkReply* m_reply = nullptr;
+};
+
+#endif