From d4fdc6e17b74629d8b6c39d3809cd54d6497ef9c Mon Sep 17 00:00:00 2001
From: Lars Toenning <dev@ltoenning.de>
Date: Tue, 5 Apr 2022 17:49:04 +0200
Subject: [PATCH] Prevent saving invalid values

If a value has not changed and should be saved to a file, only the key (and the invalid value) was passed to the save function. The save function does not check this case or read the value from the cache.
This happens when changing simulator paths as the settings are updated and saved separately.
---
 src/blackmisc/valuecache.cpp      | 14 +++++++-------
 src/blackmisc/valuecache.h        |  2 +-
 src/blackmisc/valuecacheprivate.h |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/blackmisc/valuecache.cpp b/src/blackmisc/valuecache.cpp
index b1ead3263..61acfbb91 100644
--- a/src/blackmisc/valuecache.cpp
+++ b/src/blackmisc/valuecache.cpp
@@ -363,6 +363,7 @@ namespace BlackMisc
         QMap<QString, CVariantMap> namespaces;
         for (auto it = values.cbegin(); it != values.cend(); ++it)
         {
+            Q_ASSERT(it.value().isValid());
             namespaces[it.key().section('/', 0, m_fileSplitDepth - 1)].insert(it.key(), it.value());
         }
         if (! QDir::root().mkpath(dir))
@@ -693,28 +694,27 @@ namespace BlackMisc
         return element.m_value.read();
     }
 
-    CStatusMessage CValuePage::setValue(Element &element, CVariant value, qint64 timestamp, bool save, bool ignoreValue)
+    CStatusMessage CValuePage::setValue(Element &element, CVariant value, qint64 timestamp, bool save)
     {
         Q_ASSERT_X(! element.m_key.isEmpty(), Q_FUNC_INFO, "Empty key suggests an attempt to use value before objectName available for %%OwnerName%%");
         Q_ASSERT(QThread::currentThread() == thread());
 
         if (timestamp == 0) { timestamp = QDateTime::currentMSecsSinceEpoch(); }
+        if (! value.isValid()) { value = element.m_value.read(); }
+
         bool changed = element.m_timestamp != timestamp || element.m_value.read() != value;
-        if (! changed && ! save && ! ignoreValue)
+        if (! changed && ! save)
         {
             return CStatusMessage(this).info(u"Value '%1' not set, same timestamp and value") << element.m_nameWithKey;
         }
 
-        if (ignoreValue) { value = element.m_value.read(); }
-        else { ignoreValue = ! changed; }
-
         auto status = validate(element, value, CStatusMessage::SeverityError);
         if (status.isSuccess())
         {
-            if (ignoreValue)
+            if (! changed)
             {
                 element.m_saved = save;
-                emit valuesWantToCache({ { { element.m_key, {} } }, 0, save, false });
+                emit valuesWantToCache({ { { element.m_key, value } }, 0, save, false });
             }
             else if (m_batchMode > 0)
             {
diff --git a/src/blackmisc/valuecache.h b/src/blackmisc/valuecache.h
index e31426f2d..0b3899e63 100644
--- a/src/blackmisc/valuecache.h
+++ b/src/blackmisc/valuecache.h
@@ -404,7 +404,7 @@ namespace BlackMisc
         CStatusMessage setAndSave(const T &value, qint64 timestamp = 0) { return m_page->setValue(*m_element, CVariant::from(value), timestamp, true); }
 
         //! Save using the currently set value. Must be called from the thread in which the owner lives.
-        CStatusMessage save() { return m_page->setValue(*m_element, {}, 0, true, true); }
+        CStatusMessage save() { return m_page->setValue(*m_element, {}, 0, true); }
 
         //! Write a property of the value. Must be called from the thread in which the owner lives.
         CStatusMessage setProperty(CPropertyIndexRef index, const CVariant &value, qint64 timestamp = 0) { auto v = get(); v.setPropertyByIndex(index, value); return set(v, timestamp); }
diff --git a/src/blackmisc/valuecacheprivate.h b/src/blackmisc/valuecacheprivate.h
index a43f9684b..8c2d2d4fc 100644
--- a/src/blackmisc/valuecacheprivate.h
+++ b/src/blackmisc/valuecacheprivate.h
@@ -103,7 +103,7 @@ namespace BlackMisc
             CVariant getValueCopy(const Element &element) const;
 
             //! Write the value corresponding to the element's key and begin synchronizing it to any other pages.
-            CStatusMessage setValue(Element &element, CVariant value, qint64 timestamp, bool save = false, bool ignoreValue = false);
+            CStatusMessage setValue(Element &element, CVariant value, qint64 timestamp, bool save = false);
 
             //! Get the key string corresponding to the element.
             const QString &getKey(const Element &element) const;