From 2e90a8d8298e6c902e10dc930945dbcdb8517c09 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 12 Mar 2021 18:42:37 +0100
Subject: [PATCH] [libcalamares] Report preset mis-configurations

- warn about fields applied twice (program error)
- warn about fields not used (configuration error)
- add operator<< for "clean" looking preset application
---
 src/libcalamares/modulesystem/Config.cpp | 28 +++++++++++++++++++++++-
 src/libcalamares/modulesystem/Config.h   | 17 +++++++++++++-
 src/libcalamares/modulesystem/Preset.cpp | 15 +++++++++++--
 src/libcalamares/modulesystem/Preset.h   |  6 +++++
 src/modules/users/Config.cpp             |  3 ++-
 5 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/src/libcalamares/modulesystem/Config.cpp b/src/libcalamares/modulesystem/Config.cpp
index af1536129..71210a9cb 100644
--- a/src/libcalamares/modulesystem/Config.cpp
+++ b/src/libcalamares/modulesystem/Config.cpp
@@ -64,6 +64,28 @@ Config::ApplyPresets::ApplyPresets( Calamares::ModuleSystem::Config& c, const QV
     }
 }
 
+Config::ApplyPresets::~ApplyPresets()
+{
+    m_c.m_unlocked = false;
+
+    // Check that there's no **settings** (from the configuration map)
+    // that have not been consumed by apply() -- if they are there,
+    // that means the configuration map specifies things that the
+    // Config object does not expect.
+    bool haveWarned = false;
+    for ( const auto& k : m_map.keys() )
+    {
+        if ( !m_c.d->m_presets->find( k ).isValid() )
+        {
+            if ( !haveWarned )
+            {
+                cWarning() << "Preset configuration contains unused keys";
+                haveWarned = true;
+            }
+            cDebug() << Logger::SubEntry << "Unused key" << k;
+        }
+    }
+}
 
 Config::ApplyPresets&
 Config::ApplyPresets::apply( const char* fieldName )
@@ -76,7 +98,11 @@ Config::ApplyPresets::apply( const char* fieldName )
     else
     {
         const QString key( fieldName );
-        if ( !key.isEmpty() && m_map.contains( key ) )
+        if ( !key.isEmpty() && m_c.d->m_presets->find( key ).isValid() )
+        {
+            cWarning() << "Applying duplicate property" << fieldName;
+        }
+        else if ( !key.isEmpty() && m_map.contains( key ) )
         {
             QVariantMap m = CalamaresUtils::getSubMap( m_map, key, m_bogus );
             QVariant value = m[ "value" ];
diff --git a/src/libcalamares/modulesystem/Config.h b/src/libcalamares/modulesystem/Config.h
index c38b0c11c..c31433e1c 100644
--- a/src/libcalamares/modulesystem/Config.h
+++ b/src/libcalamares/modulesystem/Config.h
@@ -70,10 +70,25 @@ protected:
     class ApplyPresets
     {
     public:
+        /** @brief Create a preset-applier for this config
+         *
+         * The @p configurationMap should be the one passed in to
+         * setConfigurationMap() . Presets are extracted from the
+         * standard key *presets* and can be applied to the configuration
+         * with apply() or operator<<.
+         */
         ApplyPresets( Config& c, const QVariantMap& configurationMap );
-        ~ApplyPresets() { m_c.m_unlocked = false; }
+        ~ApplyPresets();
 
+        /** @brief Add a preset for the given @p fieldName
+         *
+         * This checks for preset-entries in the configuration map that was
+         * passed in to the constructor.
+         */
         ApplyPresets& apply( const char* fieldName );
+        /** @brief Alternate way of writing apply()
+         */
+        ApplyPresets& operator<<( const char* fieldName ) { return apply( fieldName ); }
 
     private:
         Config& m_c;
diff --git a/src/libcalamares/modulesystem/Preset.cpp b/src/libcalamares/modulesystem/Preset.cpp
index 3a54c9e68..a2e3f3264 100644
--- a/src/libcalamares/modulesystem/Preset.cpp
+++ b/src/libcalamares/modulesystem/Preset.cpp
@@ -38,8 +38,6 @@ namespace Calamares
 namespace ModuleSystem
 {
 Presets::Presets( const QVariantMap& configurationMap )
-
-
 {
     reserve( configurationMap.count() );
     loadPresets( *this, configurationMap, []( const QString& ) { return true; } );
@@ -66,6 +64,19 @@ Presets::isEditable( const QString& fieldName ) const
     return true;
 }
 
+PresetField
+Presets::find( const QString& fieldName ) const
+{
+    for ( const auto& p : *this )
+    {
+        if ( p.fieldName == fieldName )
+        {
+            return p;
+        }
+    }
+
+    return PresetField();
+}
 
 }  // namespace ModuleSystem
 }  // namespace Calamares
diff --git a/src/libcalamares/modulesystem/Preset.h b/src/libcalamares/modulesystem/Preset.h
index f7b5023e5..b768a31c9 100644
--- a/src/libcalamares/modulesystem/Preset.h
+++ b/src/libcalamares/modulesystem/Preset.h
@@ -78,6 +78,12 @@ public:
      * returns @c true.
      */
     bool isEditable( const QString& fieldName ) const;
+
+    /** @brief Finds the settings for a field @p fieldName
+     *
+     * If there is no such field, returns an invalid PresetField.
+     */
+    PresetField find( const QString& fieldName ) const;
 };
 }  // namespace ModuleSystem
 }  // namespace Calamares
diff --git a/src/modules/users/Config.cpp b/src/modules/users/Config.cpp
index 047b8d516..4e02cdcdf 100644
--- a/src/modules/users/Config.cpp
+++ b/src/modules/users/Config.cpp
@@ -842,7 +842,8 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     updateGSAutoLogin( doAutoLogin(), loginName() );
     checkReady();
 
-    ApplyPresets( *this, configurationMap ).apply( "fullName" ).apply( "loginName" );
+    ApplyPresets( *this, configurationMap ) << "fullName"
+                                            << "loginName";
 }
 
 void