add assert on metaEnum and convert existing settings to strings

qgis · May 14, 2018 · a668362 · a668362
1 parent d26f1e1
commit a668362
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 8 deletions.
diff --git a/src/core/qgssettings.h b/src/core/qgssettings.h
@@ -232,9 +232,10 @@ class CORE_EXPORT QgsSettings : public QObject
      */
     template <class T>
     T enumValue( const QString &key, const T &defaultValue,
-                 const Section section = NoSection ) const
+                 const Section section = NoSection )
     {
       QMetaEnum metaEnum = QMetaEnum::fromType<T>();
+      Q_ASSERT( metaEnum.isValid() );
       if ( !metaEnum.isValid() )
       {
         QgsDebugMsg( "Invalid metaenum. Enum probably misses Q_ENUM or Q_FLAG declaration." );
@@ -245,16 +246,29 @@ class CORE_EXPORT QgsSettings : public QObject
 
       if ( metaEnum.isValid() )
       {
+        // read as string
         QByteArray ba = value( key, metaEnum.valueToKey( defaultValue ) ).toString().toUtf8();
         const char *vs = ba.data();
         v = static_cast<T>( metaEnum.keyToValue( vs, &ok ) );
       }
       if ( !ok )
       {
+        // if failed, try to read as int (old behavior)
+        // this code shall be removed later (probably after QGIS 3.4 LTR for 3.6)
+        // then the method could be marked as const
         v = static_cast<T>( value( key, static_cast<int>( defaultValue ), section ).toInt( &ok ) );
-        if ( metaEnum.isValid() && ( !ok || !metaEnum.valueToKey( static_cast<int>( v ) ) ) )
+        if ( metaEnum.isValid() )
         {
-          v = defaultValue;
+          if ( !ok || !metaEnum.valueToKey( static_cast<int>( v ) ) )
+          {
+            v = defaultValue;
+          }
+          else
+          {
+            // found setting as an integer
+            // convert the setting to the new form (string)
+            setEnumValue( key, v, section );
+          }
         }
       }
 
@@ -273,6 +287,7 @@ class CORE_EXPORT QgsSettings : public QObject
                        const Section section = NoSection )
     {
       QMetaEnum metaEnum = QMetaEnum::fromType<T>();
+      Q_ASSERT( metaEnum.isValid() );
       if ( metaEnum.isValid() )
       {
         setValue( key, metaEnum.valueToKey( value ), section );
@@ -294,9 +309,10 @@ class CORE_EXPORT QgsSettings : public QObject
      */
     template <class T>
     T flagValue( const QString &key, const T &defaultValue,
-                 const Section section = NoSection ) const
+                 const Section section = NoSection )
     {
       QMetaEnum metaEnum = QMetaEnum::fromType<T>();
+      Q_ASSERT( metaEnum.isValid() );
       if ( !metaEnum.isValid() )
       {
         QgsDebugMsg( "Invalid metaenum. Enum probably misses Q_ENUM or Q_FLAG declaration." );
@@ -305,19 +321,31 @@ class CORE_EXPORT QgsSettings : public QObject
       T v;
       bool ok = false;
 
-
       if ( metaEnum.isValid() )
       {
+        // read as string
         QByteArray ba = value( key, metaEnum.valueToKeys( defaultValue ) ).toString().toUtf8();
         const char *vs = ba.data();
         v = static_cast<T>( metaEnum.keysToValue( vs, &ok ) );
       }
       if ( !ok )
       {
+        // if failed, try to read as int (old behavior)
+        // this code shall be removed later (probably after QGIS 3.4 LTR for 3.6)
+        // then the method could be marked as const
         v = T( value( key, static_cast<int>( defaultValue ), section ).toInt( &ok ) );
-        if ( metaEnum.isValid() && ( !ok || !metaEnum.valueToKeys( static_cast<int>( v ) ).size() ) )
+        if ( metaEnum.isValid() )
         {
-          v = defaultValue;
+          if ( !ok || !metaEnum.valueToKeys( static_cast<int>( v ) ).size() )
+          {
+            v = defaultValue;
+          }
+          else
+          {
+            // found setting as an integer
+            // convert the setting to the new form (string)
+            setFlagValue( key, v, section );
+          }
         }
       }
 
@@ -336,6 +364,7 @@ class CORE_EXPORT QgsSettings : public QObject
                        const Section section = NoSection )
     {
       QMetaEnum metaEnum = QMetaEnum::fromType<T>();
+      Q_ASSERT( metaEnum.isValid() );
       if ( metaEnum.isValid() )
       {
         setValue( key, metaEnum.valueToKeys( value ), section );

diff --git a/tests/src/core/testqgssettings.cpp b/tests/src/core/testqgssettings.cpp
@@ -31,6 +31,7 @@ class TestQgsSettings : public QObject
 
   private slots:
     void enumValue();
+    void flagValue();
 };
 
 
@@ -59,12 +60,17 @@ void TestQgsSettings::enumValue()
   QgsUnitTypes::LayoutUnit v3 = settings.enumValue( QStringLiteral( "qgis/testing/my_value_for_units" ), QgsUnitTypes::LayoutMeters );
   QCOMPARE( v3, QgsUnitTypes::LayoutCentimeters );
   settings.setEnumValue( QStringLiteral( "qgis/testing/my_value_for_units" ), QgsUnitTypes::LayoutCentimeters );
+  // auto conversion of old settings (int to str)
+  QCOMPARE( settings.value( "qgis/testing/my_value_for_units" ), QStringLiteral( "LayoutCentimeters" ) );
   QgsUnitTypes::LayoutUnit v3s = settings.enumValue( QStringLiteral( "qgis/testing/my_value_for_units" ), QgsUnitTypes::LayoutMeters );
   QCOMPARE( v3s, QgsUnitTypes::LayoutCentimeters );
   QString v3ss = settings.value( QStringLiteral( "qgis/testing/my_value_for_units" ), QStringLiteral( "myDummyValue" ) ).toString();
   QCOMPARE( v3ss, QStringLiteral( "LayoutCentimeters" ) );
+}
 
-  // test for flags
+void TestQgsSettings::flagValue()
+{
+  QgsSettings settings;
   QgsMapLayerProxyModel::Filters pointAndLine = QgsMapLayerProxyModel::Filters( QgsMapLayerProxyModel::PointLayer | QgsMapLayerProxyModel::LineLayer );
   QgsMapLayerProxyModel::Filters pointAndPolygon = QgsMapLayerProxyModel::Filters( QgsMapLayerProxyModel::PointLayer | QgsMapLayerProxyModel::PolygonLayer );
   settings.setValue( QStringLiteral( "qgis/testing/my_value_for_a_flag" ), 1e8 ); // invalid
@@ -74,6 +80,8 @@ void TestQgsSettings::enumValue()
   settings.setValue( QStringLiteral( "qgis/testing/my_value_for_a_flag" ), static_cast<int>( pointAndPolygon ) );
   QgsMapLayerProxyModel::Filters v5 = settings.flagValue( QStringLiteral( "qgis/testing/my_value_for_a_flag" ), pointAndLine, QgsSettings::NoSection );
   QCOMPARE( v5, pointAndPolygon );
+  // auto conversion of old settings (int to str)
+  QCOMPARE( settings.value( "qgis/testing/my_value_for_a_flag" ), QStringLiteral( "PointLayer|PolygonLayer" ) );
 
   settings.setFlagValue( QStringLiteral( "qgis/testing/my_value_for_a_flag_as_string" ), pointAndPolygon, QgsSettings::NoSection );
   QgsMapLayerProxyModel::Filters v5s = settings.flagValue( QStringLiteral( "qgis/testing/my_value_for_a_flag_as_string" ), pointAndLine, QgsSettings::NoSection );