[preservefiles] Split file-items into separate header

Put the Item class in a separate header; give it functionality
to create itself from Variants (e.g. from the configuration data)
and to run itself (do whatever the item is supposed to do).
This makes the polymorphic approach unnecessary: we just have
items that are sufficiently smart.

This moves do-a-thing to the Item, while the Job now has one
job: be a loop around creating Items and running items.

src/modules/preservefiles/CMakeLists.txt

@@ -9,6 +9,7 @@ calamares_add_plugin( preservefiles
     TYPE job
     EXPORT_MACRO PLUGINDLLEXPORT_PRO
     SOURCES
+        Item.cpp
         PreserveFiles.cpp
     # REQUIRES mount  # To set the rootMountPoint
     SHARED_LIB

src/modules/preservefiles/Item.cpp

@@ -0,0 +1,160 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *  SPDX-FileCopyrightText: 2018, 2021 Adriaan de Groot <groot@kde.org>
+ *  SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ */
+
+#include "Item.h"
+
+#include "GlobalStorage.h"
+#include "JobQueue.h"
+#include "utils/CalamaresUtilsSystem.h"
+#include "utils/Logger.h"
+#include "utils/Units.h"
+
+#include <QFile>
+
+using namespace CalamaresUtils::Units;
+
+static bool
+copy_file( const QString& source, const QString& dest )
+{
+    QFile sourcef( source );
+    if ( !sourcef.open( QFile::ReadOnly ) )
+    {
+        cWarning() << "Could not read" << source;
+        return false;
+    }
+
+    QFile destf( dest );
+    if ( !destf.open( QFile::WriteOnly ) )
+    {
+        sourcef.close();
+        cWarning() << "Could not open" << destf.fileName() << "for writing; could not copy" << source;
+        return false;
+    }
+
+    QByteArray b;
+    do
+    {
+        b = sourcef.read( 1_MiB );
+        destf.write( b );
+    } while ( b.count() > 0 );
+
+    sourcef.close();
+    destf.close();
+
+    return true;
+}
+
+Item
+Item::fromVariant( const QVariant& v, const CalamaresUtils::Permissions& defaultPermissions )
+{
+    if ( v.type() == QVariant::String )
+    {
+        QString filename = v.toString();
+        if ( !filename.isEmpty() )
+        {
+            return { filename, filename, defaultPermissions, ItemType::Path };
+        }
+        else
+        {
+            cWarning() << "Empty filename for preservefiles, item" << v;
+            return {};
+        }
+    }
+    else if ( v.type() == QVariant::Map )
+    {
+        const auto map = v.toMap();
+
+        CalamaresUtils::Permissions perm( defaultPermissions );
+        ItemType t = ItemType::None;
+
+        {
+            QString perm_string = map[ "perm" ].toString();
+            if ( !perm_string.isEmpty() )
+            {
+                perm = CalamaresUtils::Permissions( perm_string );
+            }
+        }
+
+        {
+            QString from = map[ "from" ].toString();
+            t = ( from == "log" ) ? ItemType::Log : ( from == "config" ) ? ItemType::Config : ItemType::None;
+
+            if ( t == ItemType::None && !map[ "src" ].toString().isEmpty() )
+            {
+                t = ItemType::Path;
+            }
+        }
+
+        QString dest = map[ "dest" ].toString();
+        if ( dest.isEmpty() )
+        {
+            cWarning() << "Empty dest for preservefiles, item" << v;
+            return {};
+        }
+
+        switch ( t )
+        {
+        case ItemType::Config:
+            return { QString(), dest, perm, t };
+        case ItemType::Log:
+            return { QString(), dest, perm, t };
+        case ItemType::Path:
+            return { map[ "src" ].toString(), dest, perm, t };
+        case ItemType::None:
+            cWarning() << "Invalid type for preservefiles, item" << v;
+            return {};
+        }
+    }
+    else
+    {
+        cWarning() << "Invalid type for preservefiles, item" << v;
+        return {};
+    }
+}
+
+
+bool
+Item::exec( const std::function< QString( QString ) >& replacements ) const
+{
+    QString expanded_dest = replacements( dest );
+    QString full_dest = CalamaresUtils::System::instance()->targetPath( expanded_dest );
+
+    bool success = false;
+    switch ( type )
+    {
+    case ItemType::None:
+        cWarning() << "Invalid item for preservefiles skipped.";
+        return false;
+    case ItemType::Config:
+        if ( !( success = Calamares::JobQueue::instance()->globalStorage()->saveJson( full_dest ) ) )
+        {
+            cWarning() << "Could not write a JSON dump of global storage to" << full_dest;
+        }
+        break;
+    case ItemType::Log:
+        if ( !( success = copy_file( Logger::logFile(), full_dest ) ) )
+        {
+            cWarning() << "Could not preserve log file to" << full_dest;
+        }
+        break;
+    case ItemType::Path:
+        if ( !( success = copy_file( source, full_dest ) ) )
+        {
+            cWarning() << "Could not preserve" << source << "to" << full_dest;
+        }
+        break;
+    }
+    if ( !success )
+    {
+        CalamaresUtils::System::instance()->removeTargetFile( expanded_dest );
+        return false;
+    }
+    else
+    {
+        return perm.apply( full_dest );
+    }
+}

src/modules/preservefiles/Item.h

@@ -0,0 +1,72 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *  SPDX-FileCopyrightText: 2018, 2021 Adriaan de Groot <groot@kde.org>
+ *  SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ */
+#ifndef PRESERVEFILES_ITEM_H
+#define PRESERVEFILES_ITEM_H
+
+#include "utils/Permissions.h"
+
+#include <QString>
+#include <QVariant>
+
+#include <memory>
+
+enum class ItemType
+{
+    None,
+    Path,
+    Log,
+    Config
+};
+
+/** @brief Represents one item to copy
+ *
+ * All item types need a destination (to place the data), this is
+ * intepreted within the target system. All items need a permission,
+ * which is applied to the data once written.
+ *
+ * The source may be a path, but not all types need a source.
+ */
+class Item
+{
+    QString source;
+    QString dest;
+    CalamaresUtils::Permissions perm;
+    ItemType type;
+
+public:
+    Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t )
+        : source( src )
+        , dest( d )
+        , perm( std::move( p ) )
+        , type( t )
+    {
+    }
+
+    Item()
+        : type( ItemType::None )
+    {
+    }
+
+    operator bool() const { return type != ItemType::None; }
+
+    bool exec( const std::function< QString( QString ) >& replacements ) const;
+
+
+    /** @brief Create an Item -- or one of its subclasses -- from @p v
+     *
+     * Depending on the structure and contents of @p v, a pointer
+     * to an Item is returned. If @p v cannot be interpreted meaningfully,
+     * then a nullptr is returned.
+     *
+     * When the entry contains a *perm* key, use that permission, otherwise
+     * apply @p defaultPermissions to the item.
+     */
+    static Item fromVariant( const QVariant& v, const CalamaresUtils::Permissions& defaultPermissions );
+};
+
+
+#endif

src/modules/preservefiles/PreserveFiles.cpp

@@ -7,70 +7,20 @@
 
 #include "PreserveFiles.h"
 
+#include "Item.h"
+
 #include "CalamaresVersion.h"
 #include "GlobalStorage.h"
 #include "JobQueue.h"
 #include "utils/CalamaresUtilsSystem.h"
 #include "utils/CommandList.h"
 #include "utils/Logger.h"
-#include "utils/Permissions.h"
 #include "utils/Units.h"
 
 #include <QFile>
 
-enum class ItemType
-{
-    None,
-    Path,
-    Log,
-    Config
-};
-
-struct Item
-{
-    QString source;
-    QString dest;
-    CalamaresUtils::Permissions perm;
-    ItemType type;
-
-    Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t )
-        : source( src )
-        , dest( d )
-        , perm( std::move( p ) )
-        , type( t )
-    {
-    }
-};
-
 using namespace CalamaresUtils::Units;
 
-QString
-targetPrefix()
-{
-    if ( CalamaresUtils::System::instance()->doChroot() )
-    {
-        Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
-        if ( gs && gs->contains( "rootMountPoint" ) )
-        {
-            QString r = gs->value( "rootMountPoint" ).toString();
-            if ( !r.isEmpty() )
-            {
-                return r;
-            }
-            else
-            {
-                cDebug() << "RootMountPoint is empty";
-            }
-        }
-        else
-        {
-            cDebug() << "No rootMountPoint defined, preserving files to '/'";
-        }
-    }
-
-    return QLatin1String( "/" );
-}
-
 QString
 atReplacements( QString s )
 {
@@ -103,37 +53,6 @@ PreserveFiles::prettyName() const
     return tr( "Saving files for later ..." );
 }
 
-static bool
-copy_file( const QString& source, const QString& dest )
-{
-    QFile sourcef( source );
-    if ( !sourcef.open( QFile::ReadOnly ) )
-    {
-        cWarning() << "Could not read" << source;
-        return false;
-    }
-
-    QFile destf( dest );
-    if ( !destf.open( QFile::WriteOnly ) )
-    {
-        sourcef.close();
-        cWarning() << "Could not open" << destf.fileName() << "for writing; could not copy" << source;
-        return false;
-    }
-
-    QByteArray b;
-    do
-    {
-        b = sourcef.read( 1_MiB );
-        destf.write( b );
-    } while ( b.count() > 0 );
-
-    sourcef.close();
-    destf.close();
-
-    return true;
-}
-
 Calamares::JobResult
 PreserveFiles::exec()
 {
@@ -142,52 +61,20 @@ PreserveFiles::exec()
         return Calamares::JobResult::error( tr( "No files configured to save for later." ) );
     }
 
-    QString prefix = targetPrefix();
+    int count = 0;
-    if ( !prefix.endsWith( '/' ) )
-    {
-        prefix.append( '/' );
-    }
-
-    size_t count = 0;
     for ( const auto& it : qAsConst( m_items ) )
     {
-        QString source = it->source;
+        if ( !it )
-        QString bare_dest = atReplacements( it->dest );
-        QString dest = prefix + bare_dest;
-
-        if ( it->type == ItemType::Log )
         {
-            source = Logger::logFile();
+            // Invalid entries are nullptr, ignore them but count as a success
+            // because they shouldn't block the installation. There are
+            // warnings in the log showing what the configuration problem is.
+            ++count;
+            continue;
         }
-        if ( it->type == ItemType::Config )
+        if ( it.exec( atReplacements ) )
         {
-            if ( !Calamares::JobQueue::instance()->globalStorage()->saveJson( dest ) )
+            ++count;
-            {
-                cWarning() << "Could not write a JSON dump of global storage to" << dest;
-            }
-            else
-            {
-                ++count;
-            }
-        }
-        else if ( source.isEmpty() )
-        {
-            cWarning() << "Skipping unnamed source file for" << dest;
-        }
-        else
-        {
-            if ( copy_file( source, dest ) )
-            {
-                if ( it->perm.isValid() )
-                {
-                    if ( !it->perm.apply( CalamaresUtils::System::instance()->targetPath( bare_dest ) ) )
-                    {
-                        cWarning() << "Could not set attributes of" << bare_dest;
-                    }
-                }
-
-                ++count;
-            }
         }
     }
 
@@ -217,54 +104,11 @@ PreserveFiles::setConfigurationMap( const QVariantMap& configurationMap )
     {
         defaultPermissions = QStringLiteral( "root:root:0400" );
     }
+    CalamaresUtils::Permissions perm( defaultPermissions );
 
-    QVariantList l = files.toList();
+    for ( const auto& li : files.toList() )
-    unsigned int c = 0;
-    for ( const auto& li : l )
     {
-        if ( li.type() == QVariant::String )
+        m_items.push_back( Item::fromVariant( li, perm ) );
-        {
-            QString filename = li.toString();
-            if ( !filename.isEmpty() )
-                m_items.push_back( std::make_unique< Item >(
-                    filename, filename, CalamaresUtils::Permissions( defaultPermissions ), ItemType::Path ) );
-            else
-            {
-                cDebug() << "Empty filename for preservefiles, item" << c;
-            }
-        }
-        else if ( li.type() == QVariant::Map )
-        {
-            const auto map = li.toMap();
-            QString dest = map[ "dest" ].toString();
-            QString from = map[ "from" ].toString();
-            ItemType t = ( from == "log" ) ? ItemType::Log : ( from == "config" ) ? ItemType::Config : ItemType::None;
-            QString perm = map[ "perm" ].toString();
-            if ( perm.isEmpty() )
-            {
-                perm = defaultPermissions;
-            }
-
-            if ( dest.isEmpty() )
-            {
-                cDebug() << "Empty dest for preservefiles, item" << c;
-            }
-            else if ( t == ItemType::None )
-            {
-                cDebug() << "Invalid type for preservefiles, item" << c;
-            }
-            else
-            {
-                m_items.push_back(
-                    std::make_unique< Item >( QString(), dest, CalamaresUtils::Permissions( perm ), t ) );
-            }
-        }
-        else
-        {
-            cDebug() << "Invalid type for preservefiles, item" << c;
-        }
-
-        ++c;
     }
 }
 

src/modules/preservefiles/PreserveFiles.h

@@ -10,19 +10,15 @@
 
 #include "CppJob.h"
 #include "DllMacro.h"
-#include "utils/Permissions.h"
 #include "utils/PluginFactory.h"
 
-#include <memory>
+class Item;
-#include <vector>
-
-struct Item;
 
 class PLUGINDLLEXPORT PreserveFiles : public Calamares::CppJob
 {
     Q_OBJECT
 
-    using ItemList = std::vector< std::unique_ptr< Item > >;
+    using ItemList = QList< Item >;
 
 public:
     explicit PreserveFiles( QObject* parent = nullptr );