diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 8e209f8a3..ff341bb56 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -1,6 +1,7 @@ # === This file is part of Calamares - === # # SPDX-FileCopyrightText: 2020 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 @@ -15,12 +16,12 @@ # 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 -# + # # libcalamares is the non-GUI part of Calamares, which includes handling -# translations, configurations, logging, utilities, global storage, and (non-GUI) jobs. +# translations, configurations, logging, utilities, global storage, and +# (non-GUI) jobs. +# add_definitions( -DDLLEXPORT_PRO ) include_directories( ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR} ) @@ -217,18 +218,24 @@ endforeach() # calamares_add_test( libcalamarestest + SOURCES + Tests.cpp +) + +calamares_add_test( + libcalamaresutilstest SOURCES utils/Tests.cpp ) calamares_add_test( - libcalamarestestpaths + libcalamaresutilspathstest SOURCES utils/TestPaths.cpp ) calamares_add_test( - geoiptest + libcalamaresgeoiptest SOURCES geoip/GeoIPTests.cpp ${geoip_src} diff --git a/src/libcalamares/GlobalStorage.cpp b/src/libcalamares/GlobalStorage.cpp index 341fc3892..253a4d6ad 100644 --- a/src/libcalamares/GlobalStorage.cpp +++ b/src/libcalamares/GlobalStorage.cpp @@ -2,6 +2,7 @@ * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * 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 @@ -16,13 +17,9 @@ * 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 "GlobalStorage.h" -#include "JobQueue.h" #include "utils/Logger.h" #include "utils/Units.h" @@ -30,12 +27,35 @@ #include #include +#include using CalamaresUtils::operator""_MiB; namespace Calamares { +class GlobalStorage::ReadLock : public QMutexLocker +{ +public: + ReadLock( const GlobalStorage* gs ) + : QMutexLocker( &gs->m_mutex ) + { + } +}; + +class GlobalStorage::WriteLock : public QMutexLocker +{ +public: + WriteLock( GlobalStorage* gs ) + : QMutexLocker( &gs->m_mutex ) + , m_gs( gs ) + { + } + ~WriteLock() { m_gs->changed(); } + + GlobalStorage* m_gs; +}; + GlobalStorage::GlobalStorage( QObject* parent ) : QObject( parent ) { @@ -45,6 +65,7 @@ GlobalStorage::GlobalStorage( QObject* parent ) bool GlobalStorage::contains( const QString& key ) const { + ReadLock l( this ); return m.contains( key ); } @@ -52,6 +73,7 @@ GlobalStorage::contains( const QString& key ) const int GlobalStorage::count() const { + ReadLock l( this ); return m.count(); } @@ -59,14 +81,15 @@ GlobalStorage::count() const void GlobalStorage::insert( const QString& key, const QVariant& value ) { + WriteLock l( this ); m.insert( key, value ); - emit changed(); } QStringList GlobalStorage::keys() const { + ReadLock l( this ); return m.keys(); } @@ -74,8 +97,8 @@ GlobalStorage::keys() const int GlobalStorage::remove( const QString& key ) { + WriteLock l( this ); int nItems = m.remove( key ); - emit changed(); return nItems; } @@ -83,21 +106,25 @@ GlobalStorage::remove( const QString& key ) QVariant GlobalStorage::value( const QString& key ) const { + ReadLock l( this ); return m.value( key ); } void GlobalStorage::debugDump() const { + ReadLock l( this ); + cDebug() << "GlobalStorage" << Logger::Pointer( this ) << m.count() << "items"; for ( auto it = m.cbegin(); it != m.cend(); ++it ) { - cDebug() << it.key() << '\t' << it.value(); + cDebug() << Logger::SubEntry << it.key() << '\t' << it.value(); } } bool -GlobalStorage::save( const QString& filename ) +GlobalStorage::saveJson( const QString& filename ) const { + ReadLock l( this ); QFile f( filename ); if ( !f.open( QFile::WriteOnly ) ) { @@ -110,7 +137,7 @@ GlobalStorage::save( const QString& filename ) } bool -GlobalStorage::load( const QString& filename ) +GlobalStorage::loadJson( const QString& filename ) { QFile f( filename ); if ( !f.open( QFile::ReadOnly ) ) @@ -130,10 +157,14 @@ GlobalStorage::load( const QString& filename ) } else { + WriteLock l( this ); + // Do **not** use method insert() here, because it would + // recursively lock the mutex, leading to deadlock. Also, + // that would emit changed() for each key. auto map = d.toVariant().toMap(); for ( auto i = map.constBegin(); i != map.constEnd(); ++i ) { - insert( i.key(), *i ); + m.insert( i.key(), *i ); } return true; } @@ -141,8 +172,9 @@ GlobalStorage::load( const QString& filename ) } bool -GlobalStorage::saveYaml( const QString& filename ) +GlobalStorage::saveYaml( const QString& filename ) const { + ReadLock l( this ); return CalamaresUtils::saveYaml( filename, m ); } @@ -150,12 +182,20 @@ bool GlobalStorage::loadYaml( const QString& filename ) { bool ok = false; - auto gs = CalamaresUtils::loadYaml( filename, &ok ); + auto map = CalamaresUtils::loadYaml( filename, &ok ); if ( ok ) { - m = gs; + WriteLock l( this ); + // Do **not** use method insert() here, because it would + // recursively lock the mutex, leading to deadlock. Also, + // that would emit changed() for each key. + for ( auto i = map.constBegin(); i != map.constEnd(); ++i ) + { + m.insert( i.key(), *i ); + } + return true; } - return ok; + return false; } diff --git a/src/libcalamares/GlobalStorage.h b/src/libcalamares/GlobalStorage.h index a2848f888..a12d78978 100644 --- a/src/libcalamares/GlobalStorage.h +++ b/src/libcalamares/GlobalStorage.h @@ -2,6 +2,7 @@ * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * 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 @@ -16,16 +17,12 @@ * 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 CALAMARES_GLOBALSTORAGE_H #define CALAMARES_GLOBALSTORAGE_H -#include "CalamaresConfig.h" - +#include #include #include #include @@ -33,67 +30,146 @@ namespace Calamares { -class DebugWindow; - +/** @brief Storage for data that passes between Calamares modules. + * + * The Global Storage is global to the Calamars JobQueue and + * everything that depends on that: all of its modules use the + * same instance of the JobQueue, and so of the Global Storage. + * + * GS is used to pass data between modules; there is only convention + * about what keys are used, and individual modules should document + * what they put in to GS or what they expect to find in it. + * + * GS behaves as a basic key-value store, with a QVariantMap behind + * it. Any QVariant can be put into the storage, and the signal + * changed() is emitted when any data is modified. + * + * In general, see QVariantMap (possibly after calling data()) for details. + * + * This class is thread-safe -- most accesses go through JobQueue, which + * handles threading itself, but because modules load in parallel and can + * have asynchronous tasks like GeoIP lookups, the storage itself also + * has locking. All methods are thread-safe, use data() to make a snapshot + * copy for use outside of the thread-safe API. + */ class GlobalStorage : public QObject { Q_OBJECT public: + /** @brief Create a GS object + * + * **Generally** there is only one GS object (hence, "global") which + * is owned by the JobQueue instance (which is a singleton). However, + * it is possible to create more GS objects. + */ explicit GlobalStorage( QObject* parent = nullptr ); - //NOTE: thread safety is guaranteed by JobQueue, which executes jobs one by one. - // If at any time jobs become concurrent, this class must be made thread-safe. + /** @brief Insert a key and value into the store + * + * The @p value is added to the store with key @p key. If @p key + * already exists in the store, its existing value is overwritten. + * The changed() signal is emitted regardless. + */ void insert( const QString& key, const QVariant& value ); + /** @brief Removes a key and its value + * + * The @p key is removed from the store. If the @p key does not + * exist, nothing happens. changed() is emitted regardless. + * + * @return the number of keys remaining + */ int remove( const QString& key ); - /// @brief dump keys and values to the debug log + /** @brief dump keys and values to the debug log + * + * All the keys and their values are written to the debug log. + * See save() for caveats: this can leak sensitive information. + */ void debugDump() const; /** @brief write as JSON to the given filename + * + * The file named @p filename is overwritten with a JSON representation + * of the entire global storage (this may be structured, for instance + * if maps or lists have been inserted). * * No tidying, sanitization, or censoring is done -- for instance, * the user module sets a slightly-obscured password in global storage, * and this JSON file will contain that password in-the-only-slightly- * obscured form. */ - bool save( const QString& filename ); + bool saveJson( const QString& filename ) const; /** @brief Adds the keys from the given JSON file * * No tidying, sanitization, or censoring is done. * The JSON file is read and each key is added as a value to - * the global storage. + * the global storage. The storage is not cleared first: existing + * keys will remain; keys that also occur in the JSON file are overwritten. */ - bool load( const QString& filename ); + bool loadJson( const QString& filename ); /** @brief write as YAML to the given filename * * See also save(), above. */ - bool saveYaml( const QString& filename ); + bool saveYaml( const QString& filename ) const; - /// @brief reads settings from the given filename + /** @brief reads settings from the given filename + * + * See also load(), above. + */ bool loadYaml( const QString& filename ); - /** @brief Get internal mapping as a constant object + /** @brief Make a complete copy of the data * - * Note that the VariantMap underneath may change, because - * it's not constant in itself. Connect to the changed() - * signal for notifications. + * Provides a snapshot of the data at a given time. */ - const QVariantMap& data() const { return m; } + QVariantMap data() const { return m; } public Q_SLOTS: + /** @brief Does the store contain the given key? + * + * This can distinguish an explicitly-inserted QVariant() from + * a no-value-exists QVariant. See value() for details. + */ bool contains( const QString& key ) const; + /** @brief The number of keys in the store + * + * This should be unsigned, but the underlying QMap uses signed as well. + * Equal to keys().length(), in theory. + */ int count() const; + /** @brief The keys in the store + * + * This makes a copy of all the keys currently in the store, which + * could be used for iterating over all the values in the store. + */ QStringList keys() const; + /** @brief Gets a value from the store + * + * If a value has been previously inserted, returns that value. + * If @p key does not exist in the store, returns a QVariant() + * (an invalid QVariant, which boolean-converts to false). Since + * QVariant() van also be inserted explicitly, use contains() + * to check for the presence of a key if you need that. + */ QVariant value( const QString& key ) const; signals: + /** @brief Emitted any time the store changes + * + * Also emitted sometimes when the store does not change, e.g. + * when removing a non-existent key or inserting a value that + * is already present. + */ void changed(); private: + class ReadLock; + class WriteLock; QVariantMap m; + mutable QMutex m_mutex; }; } // namespace Calamares diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp new file mode 100644 index 000000000..bf419d721 --- /dev/null +++ b/src/libcalamares/Tests.cpp @@ -0,0 +1,147 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2018-2020 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 + * 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 "GlobalStorage.h" + +#include "utils/Logger.h" + +#include +#include +#include + +class TestLibCalamares : public QObject +{ + Q_OBJECT +public: + TestLibCalamares() {} + virtual ~TestLibCalamares() {} + +private Q_SLOTS: + void testGSModify(); + void testGSLoadSave(); + void testGSLoadSave2(); +}; + +void +TestLibCalamares::testGSModify() +{ + Calamares::GlobalStorage gs; + QSignalSpy spy( &gs, &Calamares::GlobalStorage::changed ); + + const QString key( "derp" ); + + QCOMPARE( gs.count(), 0 ); + QVERIFY( !gs.contains( key ) ); + + const int value = 17; + gs.insert( key, value ); + QCOMPARE( gs.count(), 1 ); + QVERIFY( gs.contains( key ) ); + QCOMPARE( gs.value( key ).type(), QVariant::Int ); + QCOMPARE( gs.value( key ).toString(), QString( "17" ) ); // It isn't a string, but does convert + QCOMPARE( gs.value( key ).toInt(), value ); + + gs.remove( key ); + QCOMPARE( gs.count(), 0 ); + QVERIFY( !gs.contains( key ) ); + + QCOMPARE( spy.count(), 2 ); // one insert, one remove +} + +void +TestLibCalamares::testGSLoadSave() +{ + Calamares::GlobalStorage gs; + const QString jsonfilename( "gs.test.json" ); + const QString yamlfilename( "gs.test.yaml" ); + + gs.insert( "derp", 17 ); + gs.insert( "cow", "moo" ); + gs.insert( "dwarfs", QStringList { "dopey", "sneezy" } ); + + QCOMPARE( gs.count(), 3 ); + + QVERIFY( gs.saveJson( jsonfilename ) ); + Calamares::GlobalStorage gs2; + QCOMPARE( gs2.count(), 0 ); + QVERIFY( gs2.loadJson( jsonfilename ) ); + QCOMPARE( gs2.count(), 3 ); + QCOMPARE( gs2.data(), gs.data() ); + + QVERIFY( gs.saveYaml( yamlfilename ) ); + Calamares::GlobalStorage gs3; + QCOMPARE( gs3.count(), 0 ); + QVERIFY( gs3.loadYaml( jsonfilename ) ); + QCOMPARE( gs3.count(), 3 ); + QCOMPARE( gs3.data(), gs.data() ); + + // Failures in loading + QVERIFY( !gs3.loadYaml( jsonfilename ) ); + QVERIFY( !gs3.loadJson( yamlfilename ) ); + + Calamares::GlobalStorage gs4; + gs4.insert( "derp", 32 ); + gs4.insert( "dorp", "Varsseveld" ); + QCOMPARE( gs4.count(), 2 ); + QVERIFY( gs4.contains( "dorp" ) ); + QCOMPARE( gs4.value( "derp" ).toInt(), 32 ); + QVERIFY( gs4.loadJson( jsonfilename ) ); + // 3 keys from the file, but one overwrite + QCOMPARE( gs4.count(), 4 ); + QVERIFY( gs4.contains( "dorp" ) ); + QCOMPARE( gs4.value( "derp" ).toInt(), 17 ); // This one was overwritten +} + +void +TestLibCalamares::testGSLoadSave2() +{ + Logger::setupLogLevel( Logger::LOGDEBUG ); + + const QString filename( "../src/libcalamares/testdata/yaml-list.conf" ); + if ( !QFile::exists( filename ) ) + { + return; + } + + Calamares::GlobalStorage gs1; + const QString key( "dwarfs" ); + + QVERIFY( gs1.loadYaml( filename ) ); + QCOMPARE( gs1.count(), 3 ); // From examining the file + QVERIFY( gs1.contains( key ) ); + cDebug() << gs1.value( key ).type() << gs1.value( key ); + QCOMPARE( gs1.value( key ).type(), QVariant::List ); + + const QString yamlfilename( "gs.test.yaml" ); + QVERIFY( gs1.saveYaml( yamlfilename ) ); + + Calamares::GlobalStorage gs2; + QVERIFY( gs2.loadYaml( yamlfilename ) ); + QVERIFY( gs2.contains( key ) ); + QCOMPARE( gs2.value( key ).type(), QVariant::List ); +} + + +QTEST_GUILESS_MAIN( TestLibCalamares ) + +#include "utils/moc-warnings.h" + +#include "Tests.moc" diff --git a/src/libcalamares/testdata/yaml-list.conf b/src/libcalamares/testdata/yaml-list.conf new file mode 100644 index 000000000..d6992b1fb --- /dev/null +++ b/src/libcalamares/testdata/yaml-list.conf @@ -0,0 +1,8 @@ +# YAML dump +--- +"cow": "moo" +"derp": 17 +"dwarfs": + - "sleepy" + - "sneezy" + - "doc" diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index 300b9b531..5a01848bf 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -148,6 +148,7 @@ LibCalamaresTests::recursiveCompareMap( const QVariantMap& a, const QVariantMap& void LibCalamaresTests::testLoadSaveYamlExtended() { + Logger::setupLogLevel( Logger::LOGDEBUG ); bool loaded_ok; for ( const auto& confname : findConf( QDir( "../src" ) ) ) { diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index 8352e861d..7262630e0 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -138,7 +138,7 @@ PreserveFiles::exec() } if ( it.type == ItemType::Config ) { - if ( Calamares::JobQueue::instance()->globalStorage()->save( dest ) ) + if ( Calamares::JobQueue::instance()->globalStorage()->saveJson( dest ) ) { cWarning() << "Could not write config for" << dest; }