Merge pull request #11208 from Chocobo1/storage

Improve SettingsStorage behavior

src/base/settingsstorage.cpp

@@ -51,16 +51,16 @@ namespace
         {
         }
 
-        QVariantHash read();
-        bool write(const QVariantHash &data);
+        QVariantHash read() const;
+        bool write(const QVariantHash &data) const;
 
     private:
         // we return actual file names used by QSettings because
         // there is no other way to get that name except
         // actually create a QSettings object.
         // if serialization operation was not successful we return empty string
-        QString deserialize(const QString &name, QVariantHash &data);
-        QString serialize(const QString &name, const QVariantHash &data);
+        QString deserialize(const QString &name, QVariantHash &data) const;
+        QString serialize(const QString &name, const QVariantHash &data) const;
 
         const QString m_name;
     };
@@ -186,33 +186,35 @@ SettingsStorage *SettingsStorage::instance()
 
 bool SettingsStorage::save()
 {
-    if (!m_dirty) return false; // Obtaining the lock is expensive, let's check early
-    QWriteLocker locker(&m_lock);
-    if (!m_dirty) return false; // something might have changed while we were getting the lock
-
-    TransactionalSettings settings(QLatin1String("qBittorrent"));
-    if (settings.write(m_data)) {
-        m_dirty = false;
-        return true;
+    if (!m_dirty) return true; // Obtaining the lock is expensive, let's check early
+    const QWriteLocker locker(&m_lock);  // to guard for `m_dirty`
+    if (!m_dirty) return true; // something might have changed while we were getting the lock
+
+    const TransactionalSettings settings(QLatin1String("qBittorrent"));
+    if (!settings.write(m_data)) {
+        m_timer.start();
+        return false;
     }
 
-    m_timer.start();
-    return false;
+    m_dirty = false;
+    return true;
 }
 
 QVariant SettingsStorage::loadValue(const QString &key, const QVariant &defaultValue) const
 {
-    QReadLocker locker(&m_lock);
+    const QReadLocker locker(&m_lock);
     return m_data.value(mapKey(key), defaultValue);
 }
 
 void SettingsStorage::storeValue(const QString &key, const QVariant &value)
 {
     const QString realKey = mapKey(key);
-    QWriteLocker locker(&m_lock);
-    if (m_data.value(realKey) != value) {
+    const QWriteLocker locker(&m_lock);
+
+    QVariant &currentValue = m_data[realKey];
+    if (currentValue != value) {
         m_dirty = true;
-        m_data.insert(realKey, value);
+        currentValue = value;
         m_timer.start();
     }
 }
@@ -220,15 +222,14 @@ void SettingsStorage::storeValue(const QString &key, const QVariant &value)
 void SettingsStorage::removeValue(const QString &key)
 {
     const QString realKey = mapKey(key);
-    QWriteLocker locker(&m_lock);
-    if (m_data.contains(realKey)) {
+    const QWriteLocker locker(&m_lock);
+    if (m_data.remove(realKey) > 0) {
         m_dirty = true;
-        m_data.remove(realKey);
         m_timer.start();
     }
 }
 
-QVariantHash TransactionalSettings::read()
+QVariantHash TransactionalSettings::read() const
 {
     QVariantHash res;
 
@@ -256,7 +257,7 @@ QVariantHash TransactionalSettings::read()
     return res;
 }
 
-bool TransactionalSettings::write(const QVariantHash &data)
+bool TransactionalSettings::write(const QVariantHash &data) const
 {
     // QSettings deletes the file before writing it out. This can result in problems
     // if the disk is full or a power outage occurs. Those events might occur
@@ -277,7 +278,7 @@ bool TransactionalSettings::write(const QVariantHash &data)
     return QFile::rename(newPath, finalPath);
 }
 
-QString TransactionalSettings::deserialize(const QString &name, QVariantHash &data)
+QString TransactionalSettings::deserialize(const QString &name, QVariantHash &data) const
 {
     SettingsPtr settings = Profile::instance().applicationSettings(name);
 
@@ -293,7 +294,7 @@ QString TransactionalSettings::deserialize(const QString &name, QVariantHash &da
     return settings->fileName();
 }
 
-QString TransactionalSettings::serialize(const QString &name, const QVariantHash &data)
+QString TransactionalSettings::serialize(const QString &name, const QVariantHash &data) const
 {
     SettingsPtr settings = Profile::instance().applicationSettings(name);
     for (auto i = data.begin(); i != data.end(); ++i)

src/base/settingvalue.h

@@ -68,6 +68,9 @@ public:
 
     CachedSettingValue<T> &operator=(const T &newValue)
     {
+        if (m_value == newValue)
+            return *this;
+
         m_value = newValue;
         storeValue(m_value);
         return *this;