[libcalamares] Make InstanceDescription a class

- switch from dumb struct to a class; use a structured InstanceKey
- expand testing of InstanceKey and InstanceDescription

src/libcalamares/Settings.cpp

@@ -4,6 +4,7 @@
  *   SPDX-FileCopyrightText: 2019 Gabriel Craciunescu <crazy@frugalware.org>
  *   SPDX-FileCopyrightText: 2019 Dominic Hayes <ferenosdev@outlook.com>
  *   SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -18,9 +19,6 @@
  *   You should have received a copy of the GNU General Public License
  *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
  *
- *   SPDX-License-Identifier: GPL-3.0-or-later
- *   License-Filename: LICENSE
- *
  */
 
 #include "Settings.h"
@@ -75,22 +73,31 @@ requireBool( const YAML::Node& config, const char* key, bool d )
 namespace Calamares
 {
 
-InstanceDescription::InstanceDescription( const QVariantMap& m )
+InstanceDescription::InstanceDescription( Calamares::ModuleSystem::InstanceKey&& key, int weight )
-    : module( m.value( "module" ).toString() )
+    : m_instanceKey( key )
-    , id( m.value( "id" ).toString() )
+    , m_weight( qBound( 1, weight, 100 ) )
-    , config( m.value( "config" ).toString() )
-    , weight( m.value( "weight" ).toInt() )
 {
-    if ( id.isEmpty() )
+    if ( !isValid() )
     {
-        id = module;
+        m_weight = 0;
     }
-    if ( config.isEmpty() )
-    {
-        config = module + QStringLiteral( ".conf" );
 }
 
-    weight = qBound( 1, weight, 100 );
+InstanceDescription
+InstanceDescription::fromSettings( const QVariantMap& m )
+{
+    InstanceDescription r(
+        Calamares::ModuleSystem::InstanceKey( m.value( "module" ).toString(), m.value( "id" ).toString() ),
+        m.value( "weight" ).toInt() );
+    if ( r.isValid() )
+    {
+        r.m_configFileName = m.value( "config" ).toString();
+        if ( r.m_configFileName.isEmpty() )
+        {
+            r.m_configFileName = r.key().module() + QStringLiteral( ".conf" );
+        }
+    }
+    return r;
 }
 
 Settings* Settings::s_instance = nullptr;
@@ -156,7 +163,7 @@ interpretInstances( const YAML::Node& node, Settings::InstanceDescriptionList& c
                 {
                     continue;
                 }
-                customInstances.append( InstanceDescription( instancesVListItem.toMap() ) );
+                customInstances.append( InstanceDescription::fromSettings( instancesVListItem.toMap() ) );
             }
         }
     }

src/libcalamares/Settings.h

@@ -4,6 +4,7 @@
  *   SPDX-FileCopyrightText: 2019 Gabriel Craciunescu <crazy@frugalware.org>
  *   SPDX-FileCopyrightText: 2019 Dominic Hayes <ferenosdev@outlook.com>
  *   SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -18,9 +19,6 @@
  *   You should have received a copy of the GNU General Public License
  *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
  *
- *   SPDX-License-Identifier: GPL-3.0-or-later
- *   License-Filename: LICENSE
- *
  */
 
 #ifndef SETTINGS_H
@@ -28,6 +26,7 @@
 
 #include "DllMacro.h"
 #include "modulesystem/Actions.h"
+#include "modulesystem/InstanceKey.h"
 
 #include <QObject>
 #include <QStringList>
@@ -36,14 +35,40 @@
 namespace Calamares
 {
 
-struct DLLEXPORT InstanceDescription
+/** @brief Description of an instance as named in `settings.conf`
+ *
+ * An instance is an intended-step-in-sequence; it is not yet
+ * a loaded module. The instances have config-files and weights
+ * which are used by the module manager when loading modules
+ * and creating jobs.
+ */
+class DLLEXPORT InstanceDescription
 {
-    InstanceDescription( const QVariantMap& );
+    using InstanceKey = Calamares::ModuleSystem::InstanceKey;
 
-    QString module;  ///< Module name (e.g. "welcome")
+    PRIVATETEST : InstanceDescription( InstanceKey&& key, int weight );
-    QString id;  ///< Id, to distinguish multiple instances (e.g. "one", for "welcome@one")
+
-    QString config;  ///< Config-file name (for multiple instances)
+public:
-    int weight;
+    /** @brief An invalid InstanceDescription
+     *
+     * Use `fromSettings()` to populate an InstanceDescription and
+     * check its validity.
+     */
+    InstanceDescription() = default;
+
+    static InstanceDescription fromSettings( const QVariantMap& );
+
+    bool isValid() const { return m_instanceKey.isValid(); }
+
+    const InstanceKey& key() const { return m_instanceKey; }
+    QString configFileName() const { return m_configFileName; }
+    bool isCustom() const { return m_instanceKey.isCustom(); }
+    int weight() const { return m_weight; }
+
+private:
+    InstanceKey m_instanceKey;
+    QString m_configFileName;
+    int m_weight = 0;
 };
 
 class DLLEXPORT Settings : public QObject

src/libcalamares/Tests.cpp

@@ -20,6 +20,8 @@
  */
 
 #include "GlobalStorage.h"
+#include "Settings.h"
+#include "modulesystem/InstanceKey.h"
 
 #include "utils/Logger.h"
 
@@ -39,6 +41,9 @@ private Q_SLOTS:
     void testGSLoadSave();
     void testGSLoadSave2();
     void testGSLoadSaveYAMLStringList();
+
+    void testInstanceKey();
+    void testInstanceDescription();
 };
 
 void
@@ -177,6 +182,198 @@ TestLibCalamares::testGSLoadSaveYAMLStringList()
     QCOMPARE( gs2.value( "dwarfs" ).toString(), QStringLiteral( "<QStringList>" ) );  // .. they're gone
 }
 
+void
+TestLibCalamares::testInstanceKey()
+{
+    using InstanceKey = Calamares::ModuleSystem::InstanceKey;
+    {
+        InstanceKey k;
+        QVERIFY( !k.isValid() );
+        QVERIFY( !k.isCustom() );
+        QVERIFY( k.module().isEmpty() );
+    }
+    {
+        InstanceKey k( QStringLiteral( "welcome" ), QString() );
+        QVERIFY( k.isValid() );
+        QVERIFY( !k.isCustom() );
+        QCOMPARE( k.module(), QStringLiteral( "welcome" ) );
+        QCOMPARE( k.id(), QStringLiteral( "welcome" ) );
+    }
+    {
+        InstanceKey k( QStringLiteral( "shellprocess" ), QStringLiteral( "zfssetup" ) );
+        QVERIFY( k.isValid() );
+        QVERIFY( k.isCustom() );
+        QCOMPARE( k.module(), QStringLiteral( "shellprocess" ) );
+        QCOMPARE( k.id(), QStringLiteral( "zfssetup" ) );
+    }
+
+    {
+        // This is a bad idea, names and ids with odd punctuation
+        InstanceKey k( QStringLiteral( " o__O " ), QString() );
+        QVERIFY( k.isValid() );
+        QVERIFY( !k.isCustom() );
+        QCOMPARE( k.module(), QStringLiteral( " o__O " ) );
+    }
+    {
+        // .. but @ is disallowed
+        InstanceKey k( QStringLiteral( "welcome@hi" ), QString() );
+        QVERIFY( !k.isValid() );
+        QVERIFY( !k.isCustom() );
+        QVERIFY( k.module().isEmpty() );
+    }
+
+    {
+        InstanceKey k = InstanceKey::fromString( "welcome" );
+        QVERIFY( k.isValid() );
+        QVERIFY( !k.isCustom() );
+        QCOMPARE( k.module(), QStringLiteral( "welcome" ) );
+        QCOMPARE( k.id(), QStringLiteral( "welcome" ) );
+    }
+    {
+        InstanceKey k = InstanceKey::fromString( "welcome@welcome" );
+        QVERIFY( k.isValid() );
+        QVERIFY( !k.isCustom() );
+        QCOMPARE( k.module(), QStringLiteral( "welcome" ) );
+        QCOMPARE( k.id(), QStringLiteral( "welcome" ) );
+    }
+
+    {
+        InstanceKey k = InstanceKey::fromString( "welcome@hi" );
+        QVERIFY( k.isValid() );
+        QVERIFY( k.isCustom() );
+        QCOMPARE( k.module(), QStringLiteral( "welcome" ) );
+        QCOMPARE( k.id(), QStringLiteral( "hi" ) );
+    }
+    {
+        InstanceKey k = InstanceKey::fromString( "welcome@hi@hi" );
+        QVERIFY( !k.isValid() );
+        QVERIFY( !k.isCustom() );
+        QVERIFY( k.module().isEmpty() );
+        QVERIFY( k.id().isEmpty() );
+    }
+}
+
+void
+TestLibCalamares::testInstanceDescription()
+{
+    using InstanceDescription = Calamares::InstanceDescription;
+    using InstanceKey = Calamares::ModuleSystem::InstanceKey;
+
+    // With invalid keys
+    //
+    //
+    {
+        InstanceDescription d;
+        QVERIFY( !d.isValid() );
+        QVERIFY( !d.isCustom() );
+        QCOMPARE( d.weight(), 0 );
+        QVERIFY( d.configFileName().isEmpty() );
+    }
+
+    {
+        InstanceDescription d( InstanceKey(), 0 );
+        QVERIFY( !d.isValid() );
+        QVERIFY( !d.isCustom() );
+        QCOMPARE( d.weight(), 0 );
+        QVERIFY( d.configFileName().isEmpty() );
+    }
+
+    {
+        InstanceDescription d( InstanceKey(), 100 );
+        QVERIFY( !d.isValid() );
+        QVERIFY( !d.isCustom() );
+        QCOMPARE( d.weight(), 0 );
+        QVERIFY( d.configFileName().isEmpty() );
+    }
+
+    // Private constructor
+    //
+    // This doesn't set up the config file yet.
+    {
+        InstanceDescription d( InstanceKey::fromString( "welcome" ), 0 );
+        QVERIFY( d.isValid() );
+        QVERIFY( !d.isCustom() );
+        QCOMPARE( d.weight(), 1 );  // **now** the constraints kick in
+        QVERIFY( d.configFileName().isEmpty() );
+    }
+
+    {
+        InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 0 );
+        QVERIFY( d.isValid() );
+        QVERIFY( d.isCustom() );
+        QCOMPARE( d.weight(), 1 );  // **now** the constraints kick in
+        QVERIFY( d.configFileName().isEmpty() );
+    }
+
+    {
+        InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 75 );
+        QCOMPARE( d.weight(), 75 );
+    }
+    {
+        InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 105 );
+        QCOMPARE( d.weight(), 100 );
+    }
+
+
+    // From settings, normal program flow
+    //
+    //
+    {
+        QVariantMap m;
+
+        InstanceDescription d = InstanceDescription::fromSettings( m );
+        QVERIFY( !d.isValid() );
+    }
+    {
+        QVariantMap m;
+        m.insert( "name", "welcome" );
+
+        InstanceDescription d = InstanceDescription::fromSettings( m );
+        QVERIFY( !d.isValid() );
+    }
+    {
+        QVariantMap m;
+        m.insert( "module", "welcome" );
+
+        InstanceDescription d = InstanceDescription::fromSettings( m );
+        QVERIFY( d.isValid() );
+        QVERIFY( !d.isCustom() );
+        QCOMPARE( d.weight(), 1 );
+        QCOMPARE( d.key().module(), QString( "welcome" ) );
+        QCOMPARE( d.key().id(), QString( "welcome" ) );
+        QCOMPARE( d.configFileName(), QString( "welcome.conf" ) );
+    }
+    {
+        QVariantMap m;
+        m.insert( "module", "welcome" );
+        m.insert( "id", "hi" );
+        m.insert( "weight", "17" );  // String, that's kind of bogus
+
+        InstanceDescription d = InstanceDescription::fromSettings( m );
+        QVERIFY( d.isValid() );
+        QVERIFY( d.isCustom() );
+        QCOMPARE( d.weight(), 17 );
+        QCOMPARE( d.key().module(), QString( "welcome" ) );
+        QCOMPARE( d.key().id(), QString( "hi" ) );
+        QCOMPARE( d.configFileName(), QString( "welcome.conf" ) );
+    }
+    {
+        QVariantMap m;
+        m.insert( "module", "welcome" );
+        m.insert( "id", "hi" );
+        m.insert( "weight", 134 );
+        m.insert( "config", "hi.conf" );
+
+        InstanceDescription d = InstanceDescription::fromSettings( m );
+        QVERIFY( d.isValid() );
+        QVERIFY( d.isCustom() );
+        QCOMPARE( d.weight(), 100 );
+        QCOMPARE( d.key().module(), QString( "welcome" ) );
+        QCOMPARE( d.key().id(), QString( "hi" ) );
+        QCOMPARE( d.configFileName(), QString( "hi.conf" ) );
+    }
+}
+
 
 QTEST_GUILESS_MAIN( TestLibCalamares )