enforce lower-case qgssettings keys (#4218)

nirvn · NathanW2 · commit f11eb3dca8a9 · 2017-03-04T15:50:20.000+10:00
* enforce lower-case qgssettings keys

* add QgsSettings::remove() unit test
diff --git a/src/core/qgssettings.cpp b/src/core/qgssettings.cpp
@@ -84,10 +84,10 @@ QgsSettings::~QgsSettings()
 
 void QgsSettings::beginGroup( const QString &prefix )
 {
-  mUserSettings->beginGroup( prefix );
+  mUserSettings->beginGroup( prefix.toLower() );
   if ( mGlobalSettings )
   {
-    mGlobalSettings->beginGroup( prefix );
+    mGlobalSettings->beginGroup( prefix.toLower() );
   }
 }
 
@@ -152,7 +152,7 @@ QStringList QgsSettings::childGroups() const
 
 QVariant QgsSettings::value( const QString &key, const QVariant &defaultValue, const QgsSettings::Section section ) const
 {
-  QString pKey = prefixedKey( key, section );
+  QString pKey = prefixedKey( key.toLower(), section );
   if ( !mUserSettings->value( pKey ).isNull() )
   {
     return mUserSettings->value( pKey );
@@ -166,7 +166,7 @@ QVariant QgsSettings::value( const QString &key, const QVariant &defaultValue, c
 
 bool QgsSettings::contains( const QString &key, const QgsSettings::Section section ) const
 {
-  QString pKey = prefixedKey( key, section );
+  QString pKey = prefixedKey( key.toLower(), section );
   return mUserSettings->contains( pKey ) ||
          ( mGlobalSettings && mGlobalSettings->contains( pKey ) );
 }
@@ -183,7 +183,7 @@ void QgsSettings::sync()
 
 void QgsSettings::remove( const QString &key )
 {
-  mUserSettings->remove( key );
+  mUserSettings->remove( key.toLower() );
 }
 
 QString QgsSettings::prefixedKey( const QString &key, const Section section ) const
@@ -208,18 +208,18 @@ QString QgsSettings::prefixedKey( const QString &key, const Section section ) co
       break;
     case Section::NoSection:
     default:
-      return sanitizeKey( key );
+      return sanitizeKey( key.toLower() );
   }
-  return prefix  + "/" + sanitizeKey( key );
+  return prefix  + "/" + sanitizeKey( key.toLower() );
 }
 
 
 int QgsSettings::beginReadArray( const QString &prefix )
 {
-  int size = mUserSettings->beginReadArray( prefix );
+  int size = mUserSettings->beginReadArray( prefix.toLower() );
   if ( 0 == size && mGlobalSettings )
   {
-    size = mGlobalSettings->beginReadArray( prefix );
+    size = mGlobalSettings->beginReadArray( prefix.toLower() );
     mUsingGlobalArray = ( size > 0 );
   }
   return size;
@@ -250,7 +250,7 @@ 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 );
+  mUserSettings->setValue( prefixedKey( key.toLower(), section ), value );
 }
 
 // To lower case and clean the path
diff --git a/tests/src/python/test_qgssettings.py b/tests/src/python/test_qgssettings.py
@@ -130,8 +130,7 @@ def test_groups(self):
         self.settings.endGroup()
         self.settings.beginGroup('testqgissettings/names')
         self.settings.setValue('name4', 'qgisrocks-4')
-        keys = list(self.settings.childKeys())
-        keys.sort()
+        keys = sorted(self.settings.childKeys())
         self.assertEqual(keys, ['name1', 'name2', 'name3', 'name4'])
         self.settings.endGroup()
         self.assertEqual('qgisrocks-1', self.settings.value('testqgissettings/names/name1'))
@@ -218,6 +217,12 @@ def test_contains(self):
         self.settings.setValue('testqgissettings/name3', 'qgisrocks3')
         self.assertTrue(self.settings.contains('testqgissettings/name3'))
 
+    def test_remove(self):
+        self.settings.setValue('testQgisSettings/temp', True)
+        self.assertEqual(self.settings.value('testqgissettings/temp'), True)
+        self.settings.remove('testQgisSettings/temp')
+        self.assertEqual(self.settings.value('testqgissettings/temp'), None)
+
 
 if __name__ == '__main__':
     unittest.main()

Original file line number	Diff line number	Diff line change
`@@ -84,10 +84,10 @@ QgsSettings::~QgsSettings()`
`84`	`84`
`85`	`85`	`void QgsSettings::beginGroup( const QString &prefix )`
`86`	`86`	`{`
`87`		`- mUserSettings->beginGroup( prefix );`
	`87`	`+ mUserSettings->beginGroup( prefix.toLower() );`
`88`	`88`	`if ( mGlobalSettings )`
`89`	`89`	`{`
`90`		`- mGlobalSettings->beginGroup( prefix );`
	`90`	`+ mGlobalSettings->beginGroup( prefix.toLower() );`
`91`	`91`	`}`
`92`	`92`	`}`
`93`	`93`
`@@ -152,7 +152,7 @@ QStringList QgsSettings::childGroups() const`
`152`	`152`
`153`	`153`	`QVariant QgsSettings::value( const QString &key, const QVariant &defaultValue, const QgsSettings::Section section ) const`
`154`	`154`	`{`
`155`		`- QString pKey = prefixedKey( key, section );`
	`155`	`+ QString pKey = prefixedKey( key.toLower(), section );`
`156`	`156`	`if ( !mUserSettings->value( pKey ).isNull() )`
`157`	`157`	`{`
`158`	`158`	`return mUserSettings->value( pKey );`
`@@ -166,7 +166,7 @@ QVariant QgsSettings::value( const QString &key, const QVariant &defaultValue, c`
`166`	`166`
`167`	`167`	`bool QgsSettings::contains( const QString &key, const QgsSettings::Section section ) const`
`168`	`168`	`{`
`169`		`- QString pKey = prefixedKey( key, section );`
	`169`	`+ QString pKey = prefixedKey( key.toLower(), section );`
`170`	`170`	`return mUserSettings->contains( pKey ) \|\|`
`171`	`171`	`( mGlobalSettings && mGlobalSettings->contains( pKey ) );`
`172`	`172`	`}`
`@@ -183,7 +183,7 @@ void QgsSettings::sync()`
`183`	`183`
`184`	`184`	`void QgsSettings::remove( const QString &key )`
`185`	`185`	`{`
`186`		`- mUserSettings->remove( key );`
	`186`	`+ mUserSettings->remove( key.toLower() );`
`187`	`187`	`}`
`188`	`188`
`189`	`189`	`QString QgsSettings::prefixedKey( const QString &key, const Section section ) const`
`@@ -208,18 +208,18 @@ QString QgsSettings::prefixedKey( const QString &key, const Section section ) co`
`208`	`208`	`break;`
`209`	`209`	`case Section::NoSection:`
`210`	`210`	`default:`
`211`		`- return sanitizeKey( key );`
	`211`	`+ return sanitizeKey( key.toLower() );`
`212`	`212`	`}`
`213`		`- return prefix + "/" + sanitizeKey( key );`
	`213`	`+ return prefix + "/" + sanitizeKey( key.toLower() );`
`214`	`214`	`}`
`215`	`215`
`216`	`216`
`217`	`217`	`int QgsSettings::beginReadArray( const QString &prefix )`
`218`	`218`	`{`
`219`		`- int size = mUserSettings->beginReadArray( prefix );`
	`219`	`+ int size = mUserSettings->beginReadArray( prefix.toLower() );`
`220`	`220`	`if ( 0 == size && mGlobalSettings )`
`221`	`221`	`{`
`222`		`- size = mGlobalSettings->beginReadArray( prefix );`
	`222`	`+ size = mGlobalSettings->beginReadArray( prefix.toLower() );`
`223`	`223`	`mUsingGlobalArray = ( size > 0 );`
`224`	`224`	`}`
`225`	`225`	`return size;`
`@@ -250,7 +250,7 @@ void QgsSettings::setArrayIndex( int i )`
`250`	`250`	`void QgsSettings::setValue( const QString &key, const QVariant &value, const QgsSettings::Section section )`
`251`	`251`	`{`
`252`	`252`	`// TODO: add valueChanged signal`
`253`		`- mUserSettings->setValue( prefixedKey( key, section ), value );`
	`253`	`+ mUserSettings->setValue( prefixedKey( key.toLower(), section ), value );`
`254`	`254`	`}`
`255`	`255`
`256`	`256`	`// To lower case and clean the path`