From 53eb6c614aea3fdcc4c2d99f7de9ef8700845ee9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 10:31:12 +0200 Subject: [PATCH] [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 | 41 ++++--- src/libcalamares/Settings.h | 45 ++++++-- src/libcalamares/Tests.cpp | 197 ++++++++++++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 27 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 3c45c2e43..c4e7a3b4c 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -1,9 +1,10 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2019 Gabriel Craciunescu * SPDX-FileCopyrightText: 2019 Dominic Hayes * SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot + * 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 . * - * 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 ) - : module( m.value( "module" ).toString() ) - , id( m.value( "id" ).toString() ) - , config( m.value( "config" ).toString() ) - , weight( m.value( "weight" ).toInt() ) +InstanceDescription::InstanceDescription( Calamares::ModuleSystem::InstanceKey&& key, int weight ) + : m_instanceKey( key ) + , m_weight( qBound( 1, weight, 100 ) ) { - if ( id.isEmpty() ) + if ( !isValid() ) { - id = module; - } - if ( config.isEmpty() ) - { - config = module + QStringLiteral( ".conf" ); + m_weight = 0; } +} - 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() ) ); } } } diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 098e010e5..6aa308dc5 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -1,9 +1,10 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2019 Gabriel Craciunescu * SPDX-FileCopyrightText: 2019 Dominic Hayes * SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot + * 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 . * - * 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 #include @@ -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") - QString id; ///< Id, to distinguish multiple instances (e.g. "one", for "welcome@one") - QString config; ///< Config-file name (for multiple instances) - int weight; + PRIVATETEST : InstanceDescription( InstanceKey&& key, int weight ); + +public: + /** @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 diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 53bedc544..a9f53dc56 100644 --- a/src/libcalamares/Tests.cpp +++ b/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( "" ) ); // .. 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 )