Do not store default values in user's QgsSettings

The new behavior is to store a value in user's QSettings
(that overrides the global settings) only if the the value
has changed from the default reported by QgsSettings.

If a value was changed and it is changed back to the default
the override must be removed from the user settings.

The rationale is that global settings should be the ultimate
source of default values, unless the user override the
default with a different value.

Fixes #21049

src/core/qgssettings.cpp

@@ -281,7 +281,17 @@ void QgsSettings::setArrayIndex( int i )
 void QgsSettings::setValue( const QString &key, const QVariant &value, const QgsSettings::Section section )
 {
   // TODO: add valueChanged signal
-  mUserSettings->setValue( prefixedKey( key, section ), value );
+  // Do not store if it hasn't changed from default value
+  QVariant v = QgsSettings::value( prefixedKey( key, section ) );
+  QVariant currentValue { QgsSettings::value( prefixedKey( key, section ) ) };
+  if ( ( currentValue.isValid() || value.isValid() ) && ( currentValue != value ) )
+  {
+    mUserSettings->setValue( prefixedKey( key, section ), value );
+  }
+  else if ( mGlobalSettings->value( prefixedKey( key, section ) ) == currentValue )
+  {
+    mUserSettings->remove( prefixedKey( key, section ) );
+  }
 }
 
 // To lower case and clean the path

tests/src/python/test_qgssettings.py

@@ -14,7 +14,8 @@ import os
 import tempfile
 from qgis.core import QgsSettings, QgsTolerance, QgsMapLayerProxyModel
 from qgis.testing import start_app, unittest
-from qgis.PyQt.QtCore import QSettings
+from qgis.PyQt.QtCore import QSettings, QVariant
+from pathlib import Path
 
 __author__ = 'Alessandro Pasotti'
 __date__ = '02/02/2017'
@@ -33,9 +34,12 @@ class TestQgsSettings(unittest.TestCase):
     def setUp(self):
         self.cnt += 1
         h, path = tempfile.mkstemp('.ini')
+        Path(path).touch()
         assert QgsSettings.setGlobalSettingsPath(path)
         self.settings = QgsSettings('testqgissettings', 'testqgissettings%s' % self.cnt)
         self.globalsettings = QSettings(self.settings.globalSettingsPath(), QSettings.IniFormat)
+        self.globalsettings.sync()
+        assert os.path.exists(self.globalsettings.fileName())
 
     def tearDown(self):
         settings_file = self.settings.fileName()
@@ -329,6 +333,7 @@ class TestQgsSettings(unittest.TestCase):
         self.settings.setValue('key1', 'provider1', section=QgsSettings.Providers)
         self.settings.setValue('key2', 'provider2', section=QgsSettings.Providers)
 
+        # This is an overwrite of previous setting and it is intentional
         self.settings.setValue('key1', 'auth1', section=QgsSettings.Auth)
         self.settings.setValue('key2', 'auth2', section=QgsSettings.Auth)
 
@@ -422,6 +427,46 @@ class TestQgsSettings(unittest.TestCase):
         self.assertEqual(self.settings.flagValue('flag', pointAndLine), pointAndLine)
         self.assertEqual(type(self.settings.flagValue('enum', pointAndLine)), QgsMapLayerProxyModel.Filters)
 
+    def test_overwriteDefaultValues(self):
+        """Test that unchanged values are not stored"""
+        self.globalsettings.setValue('a_value_with_default', 'a value')
+        self.globalsettings.setValue('an_invalid_value', QVariant())
+
+        self.assertEqual(self.settings.value('a_value_with_default'), 'a value')
+        self.assertEqual(self.settings.value('an_invalid_value'), QVariant())
+
+        # Now, set them with the same current value
+        self.settings.setValue('a_value_with_default', 'a value')
+        self.settings.setValue('an_invalid_value', QVariant())
+
+        # Check
+        pure_settings = QSettings(self.settings.fileName(), QSettings.IniFormat)
+        self.assertFalse('a_value_with_default' in pure_settings.allKeys())
+        self.assertFalse('an_invalid_value' in pure_settings.allKeys())
+
+        # Set a changed value
+        self.settings.setValue('a_value_with_default', 'a new value')
+        self.settings.setValue('an_invalid_value', 'valid value')
+
+        # Check
+        self.assertTrue('a_value_with_default' in pure_settings.allKeys())
+        self.assertTrue('an_invalid_value' in pure_settings.allKeys())
+
+        self.assertEqual(self.settings.value('a_value_with_default'), 'a new value')
+        self.assertEqual(self.settings.value('an_invalid_value'), 'valid value')
+
+        # Re-set to original values
+        self.settings.setValue('a_value_with_default', 'a value')
+        self.settings.setValue('an_invalid_value', QVariant())
+
+        self.assertEqual(self.settings.value('a_value_with_default'), 'a value')
+        self.assertEqual(self.settings.value('an_invalid_value'), QVariant())
+
+        # Check if they are gone
+        pure_settings = QSettings(self.settings.fileName(), QSettings.IniFormat)
+        self.assertFalse('a_value_with_default' not in pure_settings.allKeys())
+        self.assertFalse('an_invalid_value' not in pure_settings.allKeys())
+
 
 if __name__ == '__main__':
     unittest.main()