diff --git a/src/calamares/CalamaresApplication.cpp b/src/calamares/CalamaresApplication.cpp index 755808f5f..959e41fb1 100644 --- a/src/calamares/CalamaresApplication.cpp +++ b/src/calamares/CalamaresApplication.cpp @@ -39,6 +39,7 @@ #include #include #include +#include CalamaresApplication::CalamaresApplication( int& argc, char* argv[] ) @@ -354,7 +355,7 @@ CalamaresApplication::initView() connect( m_moduleManager, &Calamares::ModuleManager::modulesLoaded, this, &CalamaresApplication::initViewSteps ); connect( m_moduleManager, &Calamares::ModuleManager::modulesFailed, this, &CalamaresApplication::initFailed ); - m_moduleManager->loadModules(); + QTimer::singleShot( 0, m_moduleManager, &Calamares::ModuleManager::loadModules ); m_mainwindow->move( this->desktop()->availableGeometry().center() - m_mainwindow->rect().center() ); diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 87338ae6c..f942dbde1 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -207,6 +207,16 @@ if ( ECM_FOUND AND BUILD_TESTING ) Qt5::Test ) calamares_automoc( libcalamaresnetworktest ) + + ecm_add_test( + modulesystem/Tests.cpp + TEST_NAME + libcalamaresmodulesystemtest + LINK_LIBRARIES + calamares + Qt5::Test + ) + calamares_automoc( libcalamaresmodulesystemtest ) endif() if( BUILD_TESTING ) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index b55292bd4..456956430 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -169,14 +169,14 @@ interpretSequence( const YAML::Node& node, Settings::ModuleSequence& moduleSeque continue; } QString thisActionS = sequenceVListItem.toMap().firstKey(); - ModuleAction thisAction; + ModuleSystem::Action thisAction; if ( thisActionS == "show" ) { - thisAction = ModuleAction::Show; + thisAction = ModuleSystem::Action::Show; } else if ( thisActionS == "exec" ) { - thisAction = ModuleAction::Exec; + thisAction = ModuleSystem::Action::Exec; } else { diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index a52eac82e..4c2f2ed9d 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -46,7 +46,7 @@ public: using InstanceDescriptionList = QList< InstanceDescription >; InstanceDescriptionList customModuleInstances() const; - using ModuleSequence = QList< QPair< ModuleAction, QStringList > >; + using ModuleSequence = QList< QPair< ModuleSystem::Action, QStringList > >; ModuleSequence modulesSequence() const; QString brandingComponentName() const; diff --git a/src/libcalamares/modulesystem/Actions.h b/src/libcalamares/modulesystem/Actions.h index 06b5589c4..e1be0b867 100644 --- a/src/libcalamares/modulesystem/Actions.h +++ b/src/libcalamares/modulesystem/Actions.h @@ -22,13 +22,16 @@ namespace Calamares { +namespace ModuleSystem +{ -enum class ModuleAction : char +enum class Action : char { Show, Exec }; +} // namespace ModuleSystem } // namespace Calamares #endif diff --git a/src/libcalamares/modulesystem/InstanceKey.h b/src/libcalamares/modulesystem/InstanceKey.h new file mode 100644 index 000000000..35ad27c40 --- /dev/null +++ b/src/libcalamares/modulesystem/InstanceKey.h @@ -0,0 +1,111 @@ +/* === This file is part of Calamares - === + * + * Copyright 2014-2015, Teo Mrnjavac + * Copyright 2018-2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ +#ifndef MODULESYSTEM_INSTANCEKEY_H +#define MODULESYSTEM_INSTANCEKEY_H + +#include +#include +#include +#include + +namespace Logger +{ +class CLog; +} + +namespace Calamares +{ +namespace ModuleSystem +{ + +/** @brief A module instance's key (`module@id`) + * + * A module instance is identified by both the module's name + * (a Calamares module, e.g. `users`) and an instance id. + * Usually, the instance id is the same as the module name + * and the whole module instance key is `users@users`, but + * it is possible to use the same module more than once + * and then you distinguish those module instances by their + * secondary id (e.g. `users@one`). + * + * This is supported by the *instances* configuration entry + * in `settings.conf`. + */ +class InstanceKey : public QPair< QString, QString > +{ +public: + /// @brief Create an instance key from explicit module and id. + InstanceKey( const QString& module, const QString& id ) + : QPair( module, id ) + { + if ( second.isEmpty() ) + { + second = first; + } + validate(); + } + + /// @brief Create unusual, invalid instance key + InstanceKey() + : QPair( QString(), QString() ) + { + } + + /// @brief A valid module has both name and id + bool isValid() const { return !first.isEmpty() && !second.isEmpty(); } + + /// @brief A custom module has a non-default id + bool isCustom() const { return first != second; } + + QString module() const { return first; } + QString id() const { return second; } + + /// @brief Create instance key from stringified version + static InstanceKey fromString( const QString& s ) + { + QStringList moduleEntrySplit = s.split( '@' ); + if ( moduleEntrySplit.length() < 1 || moduleEntrySplit.length() > 2 ) + { + return InstanceKey(); + } + // For length 1, first == last + return InstanceKey( moduleEntrySplit.first(), moduleEntrySplit.last() ); + } + + QString toString() const + { + return first + '@' + second; + } + +private: + /** @brief Check validity and reset module and id if needed. */ + void validate() + { + if ( first.contains( '@' ) || second.contains( '@' ) ) + { + first = QString(); + second = QString(); + } + } +}; + +} // namespace ModuleSystem +} // namespace Calamares + +#endif diff --git a/src/libcalamares/modulesystem/Tests.cpp b/src/libcalamares/modulesystem/Tests.cpp new file mode 100644 index 000000000..8ef62a098 --- /dev/null +++ b/src/libcalamares/modulesystem/Tests.cpp @@ -0,0 +1,133 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "modulesystem/InstanceKey.h" + +#include + +using Calamares::ModuleSystem::InstanceKey; + +class ModuleSystemTests : public QObject +{ + Q_OBJECT +public: + ModuleSystemTests() {} + virtual ~ModuleSystemTests() {} + +private Q_SLOTS: + void initTestCase(); + + void testEmptyInstanceKey(); + void testCustomInstanceKey(); + void testFromStringInstanceKey(); + + void testBadSimpleCases(); + void testBadFromStringCases(); +}; + +void +ModuleSystemTests::initTestCase() +{ +} + +void +assert_is_invalid( const InstanceKey& k ) +{ + QVERIFY( !k.isValid() ); + QVERIFY( !k.isCustom() ); + QVERIFY( k.module().isEmpty() ); + QVERIFY( k.id().isEmpty() ); + QVERIFY( k.toString().isEmpty() ); +} + +void +ModuleSystemTests::testEmptyInstanceKey() +{ + InstanceKey k0; + assert_is_invalid( k0 ); +} + +void +ModuleSystemTests::testCustomInstanceKey() +{ + InstanceKey k0( "derp", "derp" ); + QVERIFY( k0.isValid() ); + QVERIFY( !k0.isCustom() ); + QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); + QCOMPARE( k0.toString(), QStringLiteral( "derp@derp" ) ); + + InstanceKey k1( "derp", "horse" ); + QVERIFY( k1.isValid() ); + QVERIFY( k1.isCustom() ); + QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); + QCOMPARE( k1.toString(), QStringLiteral( "derp@horse" ) ); + + InstanceKey k4( "derp", QString() ); + QVERIFY( k4.isValid() ); + QVERIFY( !k4.isCustom() ); + QCOMPARE( k4.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k4.id(), QStringLiteral( "derp" ) ); + QCOMPARE( k4.toString(), QStringLiteral( "derp@derp" ) ); +} + +void +ModuleSystemTests::testFromStringInstanceKey() +{ + InstanceKey k0 = InstanceKey::fromString( "derp@derp" ); + QVERIFY( k0.isValid() ); + QVERIFY( !k0.isCustom() ); + QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); + + InstanceKey k1 = InstanceKey::fromString( "derp@horse" ); + QVERIFY( k1.isValid() ); + QVERIFY( k1.isCustom() ); + QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); + + InstanceKey k2 = InstanceKey::fromString( "derp" ); + QVERIFY( k2.isValid() ); + QVERIFY( !k2.isCustom() ); + QCOMPARE( k2.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k2.id(), QStringLiteral( "derp" ) ); +} + +/// @brief These are expected to fail since they show bugs in the code +void +ModuleSystemTests::testBadSimpleCases() +{ + InstanceKey k4( "derp", "derp@derp" ); + assert_is_invalid( k4 ); +} + +void +ModuleSystemTests::testBadFromStringCases() +{ + InstanceKey k0 = InstanceKey::fromString( QString() ); + assert_is_invalid( k0 ); + + k0 = InstanceKey::fromString( "derp@derp@derp" ); + assert_is_invalid( k0 ); +} + + +QTEST_GUILESS_MAIN( ModuleSystemTests ) + +#include "Tests.moc" diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index a65f64108..78164ae18 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -137,7 +137,12 @@ ModuleManager::doInit() QStringList ModuleManager::loadedInstanceKeys() { - return m_loadedModulesByInstanceKey.keys(); + QStringList l; + for ( const auto& m : m_loadedModulesByInstanceKey.keys() ) + { + l << m.toString(); + } + return l; } @@ -150,7 +155,7 @@ ModuleManager::moduleDescriptor( const QString& name ) Module* ModuleManager::moduleInstance( const QString& instanceKey ) { - return m_loadedModulesByInstanceKey.value( instanceKey ); + return m_loadedModulesByInstanceKey.value( ModuleSystem::InstanceKey::fromString( instanceKey ) ); } @@ -160,12 +165,12 @@ ModuleManager::moduleInstance( const QString& instanceKey ) * @return -1 on failure, otherwise index of the instance that matches. */ static int -findCustomInstance( const Settings::InstanceDescriptionList& customInstances, const QString& module, const QString& id ) +findCustomInstance( const Settings::InstanceDescriptionList& customInstances, const ModuleSystem::InstanceKey& m ) { for ( int i = 0; i < customInstances.count(); ++i ) { const auto& thisInstance = customInstances[ i ]; - if ( thisInstance.value( "module" ) == module && thisInstance.value( "id" ) == id ) + if ( thisInstance.value( "module" ) == m.module() && thisInstance.value( "id" ) == m.id() ) { return i; } @@ -177,136 +182,131 @@ findCustomInstance( const Settings::InstanceDescriptionList& customInstances, co void ModuleManager::loadModules() { - QTimer::singleShot( 0, this, [this]() { - QStringList failedModules = checkDependencies(); - Settings::InstanceDescriptionList customInstances = Settings::instance()->customModuleInstances(); + QStringList failedModules = checkDependencies(); + Settings::InstanceDescriptionList customInstances = Settings::instance()->customModuleInstances(); - const auto modulesSequence - = failedModules.isEmpty() ? Settings::instance()->modulesSequence() : Settings::ModuleSequence(); - for ( const auto& modulePhase : modulesSequence ) + const auto modulesSequence + = failedModules.isEmpty() ? Settings::instance()->modulesSequence() : Settings::ModuleSequence(); + for ( const auto& modulePhase : modulesSequence ) + { + ModuleSystem::Action currentAction = modulePhase.first; + + foreach ( const QString& moduleEntry, modulePhase.second ) { - ModuleAction currentAction = modulePhase.first; - - foreach ( const QString& moduleEntry, modulePhase.second ) + auto instanceKey = ModuleSystem::InstanceKey::fromString( moduleEntry ); + if ( !instanceKey.isValid() ) { - QStringList moduleEntrySplit = moduleEntry.split( '@' ); - QString moduleName; - QString instanceId; - QString configFileName; - if ( moduleEntrySplit.length() < 1 || moduleEntrySplit.length() > 2 ) + cError() << "Wrong module entry format for module" << moduleEntry; + failedModules.append( moduleEntry ); + continue; + } + + + if ( !m_availableDescriptorsByModuleName.contains( instanceKey.module() ) + || m_availableDescriptorsByModuleName.value( instanceKey.module() ).isEmpty() ) + { + cError() << "Module" << instanceKey.toString() << "not found in module search paths." + << Logger::DebugList( m_paths ); + failedModules.append( instanceKey.toString() ); + continue; + } + + QString configFileName; + if ( instanceKey.isCustom() ) + { + int found = findCustomInstance( customInstances, instanceKey ); + + if ( found > -1 ) { - cError() << "Wrong module entry format for module" << moduleEntry; + configFileName = customInstances[ found ].value( "config" ); + } + else //ought to be a custom instance, but cannot find instance entry + { + cError() << "Custom instance" << moduleEntry << "not found in custom instances section."; failedModules.append( moduleEntry ); continue; } - moduleName = moduleEntrySplit.first(); - instanceId = moduleEntrySplit.last(); - configFileName = QString( "%1.conf" ).arg( moduleName ); + } + else + { + configFileName = QString( "%1.conf" ).arg( instanceKey.module() ); + } - if ( !m_availableDescriptorsByModuleName.contains( moduleName ) - || m_availableDescriptorsByModuleName.value( moduleName ).isEmpty() ) + // So now we can assume that the module entry is at least valid, + // that we have a descriptor on hand (and therefore that the + // module exists), and that the instance is either default or + // defined in the custom instances section. + // We still don't know whether the config file for the entry + // exists and is valid, but that's the only thing that could fail + // from this point on. -- Teo 8/2015 + Module* thisModule = m_loadedModulesByInstanceKey.value( instanceKey, nullptr ); + if ( thisModule && !thisModule->isLoaded() ) + { + cError() << "Module" << instanceKey.toString() << "exists but not loaded."; + failedModules.append( instanceKey.toString() ); + continue; + } + + if ( thisModule && thisModule->isLoaded() ) + { + cDebug() << "Module" << instanceKey.toString() << "already loaded."; + } + else + { + thisModule = Module::fromDescriptor( m_availableDescriptorsByModuleName.value( instanceKey.module() ), + instanceKey.id(), + configFileName, + m_moduleDirectoriesByModuleName.value( instanceKey.module() ) ); + if ( !thisModule ) { - cError() << "Module" << moduleName << "not found in module search paths." - << Logger::DebugList( m_paths ); - failedModules.append( moduleName ); + cError() << "Module" << instanceKey.toString() << "cannot be created from descriptor" << configFileName; + failedModules.append( instanceKey.toString() ); continue; } - if ( moduleName != instanceId ) //means this is a custom instance + if ( !checkDependencies( *thisModule ) ) { - int found = findCustomInstance( customInstances, moduleName, instanceId ); - - if ( found > -1 ) - { - configFileName = customInstances[ found ].value( "config" ); - } - else //ought to be a custom instance, but cannot find instance entry - { - cError() << "Custom instance" << moduleEntry << "not found in custom instances section."; - failedModules.append( moduleEntry ); - continue; - } - } - - // So now we can assume that the module entry is at least valid, - // that we have a descriptor on hand (and therefore that the - // module exists), and that the instance is either default or - // defined in the custom instances section. - // We still don't know whether the config file for the entry - // exists and is valid, but that's the only thing that could fail - // from this point on. -- Teo 8/2015 - - QString instanceKey = QString( "%1@%2" ).arg( moduleName ).arg( instanceId ); - - Module* thisModule = m_loadedModulesByInstanceKey.value( instanceKey, nullptr ); - if ( thisModule && !thisModule->isLoaded() ) - { - cError() << "Module" << instanceKey << "exists but not loaded."; - failedModules.append( instanceKey ); + // Error message is already printed + failedModules.append( instanceKey.toString() ); continue; } - if ( thisModule && thisModule->isLoaded() ) + // If it's a ViewModule, it also appends the ViewStep to the ViewManager. + thisModule->loadSelf(); + m_loadedModulesByInstanceKey.insert( instanceKey, thisModule ); + if ( !thisModule->isLoaded() ) { - cDebug() << "Module" << instanceKey << "already loaded."; - } - else - { - thisModule = Module::fromDescriptor( m_availableDescriptorsByModuleName.value( moduleName ), - instanceId, - configFileName, - m_moduleDirectoriesByModuleName.value( moduleName ) ); - if ( !thisModule ) - { - cError() << "Module" << instanceKey << "cannot be created from descriptor" << configFileName; - failedModules.append( instanceKey ); - continue; - } - - if ( !checkDependencies( *thisModule ) ) - { - // Error message is already printed - failedModules.append( instanceKey ); - continue; - } - - // If it's a ViewModule, it also appends the ViewStep to the ViewManager. - thisModule->loadSelf(); - m_loadedModulesByInstanceKey.insert( instanceKey, thisModule ); - if ( !thisModule->isLoaded() ) - { - cError() << "Module" << instanceKey << "loading FAILED."; - failedModules.append( instanceKey ); - continue; - } - } - - // At this point we most certainly have a pointer to a loaded module in - // thisModule. We now need to enqueue jobs info into an EVS. - if ( currentAction == ModuleAction::Exec ) - { - ExecutionViewStep* evs - = qobject_cast< ExecutionViewStep* >( Calamares::ViewManager::instance()->viewSteps().last() ); - if ( !evs ) // If the last step is not an EVS, we must create it. - { - evs = new ExecutionViewStep( ViewManager::instance() ); - ViewManager::instance()->addViewStep( evs ); - } - - evs->appendJobModuleInstanceKey( instanceKey ); + cError() << "Module" << instanceKey.toString() << "loading FAILED."; + failedModules.append( instanceKey.toString() ); + continue; } } + + // At this point we most certainly have a pointer to a loaded module in + // thisModule. We now need to enqueue jobs info into an EVS. + if ( currentAction == ModuleSystem::Action::Exec ) + { + ExecutionViewStep* evs + = qobject_cast< ExecutionViewStep* >( Calamares::ViewManager::instance()->viewSteps().last() ); + if ( !evs ) // If the last step is not an EVS, we must create it. + { + evs = new ExecutionViewStep( ViewManager::instance() ); + ViewManager::instance()->addViewStep( evs ); + } + + evs->appendJobModuleInstanceKey( instanceKey.toString() ); + } } - if ( !failedModules.isEmpty() ) - { - ViewManager::instance()->onInitFailed( failedModules ); - emit modulesFailed( failedModules ); - } - else - { - emit modulesLoaded(); - } - } ); + } + if ( !failedModules.isEmpty() ) + { + ViewManager::instance()->onInitFailed( failedModules ); + emit modulesFailed( failedModules ); + } + else + { + emit modulesLoaded(); + } } void diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index 8412e51d5..a4f28fab8 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -20,6 +20,8 @@ #ifndef MODULELOADER_H #define MODULELOADER_H +#include "modulesystem/InstanceKey.h" + #include "Requirement.h" #include @@ -77,8 +79,9 @@ public: Module* moduleInstance( const QString& instanceKey ); /** - * @brief loadModules initiates the asynchronous module loading operation. + * @brief loadModules does all of the module loading operation. * When this is done, the signal modulesLoaded is emitted. + * It is recommended to call this from a single-shot QTimer. */ void loadModules(); @@ -123,7 +126,7 @@ private: QMap< QString, QVariantMap > m_availableDescriptorsByModuleName; QMap< QString, QString > m_moduleDirectoriesByModuleName; - QMap< QString, Module* > m_loadedModulesByInstanceKey; + QMap< ModuleSystem::InstanceKey, Module* > m_loadedModulesByInstanceKey; const QStringList m_paths; static ModuleManager* s_instance;