ReadEntry ok value must be set to False when property doesn't exist

Fixes #37851 : readEntry ok value must be set to False when property doesn't exist
qgis · Oct 20, 2020 · a259f50 · a259f50
1 parent 58280a0
commit a259f50
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 13 deletions.
diff --git a/python/core/auto_generated/qgsproject.sip.in b/python/core/auto_generated/qgsproject.sip.in
@@ -475,18 +475,49 @@ a hierarchy of keys and corresponding values
    The key string must be valid xml tag names in order to be saved to the file.
 %End
 
-    QStringList readListEntry( const QString &scope, const QString &key, const QStringList &def = QStringList(), bool *ok = 0 ) const;
+    QStringList readListEntry( const QString &scope, const QString &key, const QStringList &def = QStringList(), bool *ok /Out/ = 0 ) const;
 %Docstring
 Key value accessors
 
 keys would be the familiar QgsSettings-like '/' delimited entries,
 implying a hierarchy of keys and corresponding values
 %End
 
-    QString readEntry( const QString &scope, const QString &key, const QString &def = QString(), bool *ok = 0 ) const;
-    int readNumEntry( const QString &scope, const QString &key, int def = 0, bool *ok = 0 ) const;
-    double readDoubleEntry( const QString &scope, const QString &key, double def = 0, bool *ok = 0 ) const;
-    bool readBoolEntry( const QString &scope, const QString &key, bool def = false, bool *ok = 0 ) const;
+    QString readEntry( const QString &scope, const QString &key, const QString &def = QString(), bool *ok /Out/ = 0 ) const;
+%Docstring
+
+:param def: returned value if key doesn't exist
+
+:return: - entry value as string from ``scope`` given its ``key``
+         - ok: set to ``True`` if key exists and has been successfully retrieved
+%End
+
+    int readNumEntry( const QString &scope, const QString &key, int def = 0, bool *ok /Out/ = 0 ) const;
+%Docstring
+
+:param def: returned value if key doesn't exist
+
+:return: - entry value as integer from ``scope`` given its ``key``
+         - ok: set to ``True`` if key exists and has been successfully retrieved
+%End
+
+    double readDoubleEntry( const QString &scope, const QString &key, double def = 0, bool *ok /Out/ = 0 ) const;
+%Docstring
+
+:param def: returned value if key doesn't exist
+
+:return: - entry value as double from ``scope`` given its ``key``
+         - ok: set to ``True`` if key exists and has been successfully retrieved
+%End
+
+    bool readBoolEntry( const QString &scope, const QString &key, bool def = false, bool *ok /Out/ = 0 ) const;
+%Docstring
+
+:param def: returned value if key doesn't exist
+
+:return: - entry value as boolean from ``scope`` given its ``key``
+         - ok: set to ``True`` if key exists and has been successfully retrieved
+%End
 
     bool removeEntry( const QString &scope, const QString &key );
 %Docstring

diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp
@@ -2515,6 +2515,9 @@ QStringList QgsProject::readListEntry( const QString &scope,
       return value.toStringList();
     }
   }
+  else if ( ok )
+    *ok = false;
+
 
   return def;
 }
@@ -2540,6 +2543,8 @@ QString QgsProject::readEntry( const QString &scope,
     if ( valid )
       return value.toString();
   }
+  else if ( ok )
+    *ok = false;
 
   return def;
 }
@@ -2587,6 +2592,8 @@ double QgsProject::readDoubleEntry( const QString &scope, const QString &key,
     if ( valid )
       return value.toDouble();
   }
+  else if ( ok )
+    *ok = false;
 
   return def;
 }
@@ -2607,6 +2614,8 @@ bool QgsProject::readBoolEntry( const QString &scope, const QString &key, bool d
     if ( valid )
       return value.toBool();
   }
+  else if ( ok )
+    *ok = false;
 
   return def;
 }

diff --git a/src/core/qgsproject.h b/src/core/qgsproject.h
@@ -498,12 +498,35 @@ class CORE_EXPORT QgsProject : public QObject, public QgsExpressionContextGenera
      * keys would be the familiar QgsSettings-like '/' delimited entries,
      * implying a hierarchy of keys and corresponding values
      */
-    QStringList readListEntry( const QString &scope, const QString &key, const QStringList &def = QStringList(), bool *ok = nullptr ) const;
+    QStringList readListEntry( const QString &scope, const QString &key, const QStringList &def = QStringList(), bool *ok SIP_OUT = nullptr ) const;
 
-    QString readEntry( const QString &scope, const QString &key, const QString &def = QString(), bool *ok = nullptr ) const;
-    int readNumEntry( const QString &scope, const QString &key, int def = 0, bool *ok = nullptr ) const;
-    double readDoubleEntry( const QString &scope, const QString &key, double def = 0, bool *ok = nullptr ) const;
-    bool readBoolEntry( const QString &scope, const QString &key, bool def = false, bool *ok = nullptr ) const;
+    /**
+     * \param def returned value if key doesn't exist
+     * \param ok set to TRUE if key exists and has been successfully retrieved
+     * \returns entry value as string from \a scope given its \a key
+     */
+    QString readEntry( const QString &scope, const QString &key, const QString &def = QString(), bool *ok SIP_OUT = nullptr ) const;
+
+    /**
+     * \param def returned value if key doesn't exist
+     * \param ok set to TRUE if key exists and has been successfully retrieved
+     * \returns entry value as integer from \a scope given its \a key
+     */
+    int readNumEntry( const QString &scope, const QString &key, int def = 0, bool *ok SIP_OUT = nullptr ) const;
+
+    /**
+     * \param def returned value if key doesn't exist
+     * \param ok set to TRUE if key exists and has been successfully retrieved
+     * \returns entry value as double from \a scope given its \a key
+     */
+    double readDoubleEntry( const QString &scope, const QString &key, double def = 0, bool *ok SIP_OUT = nullptr ) const;
+
+    /**
+     * \param def returned value if key doesn't exist
+     * \param ok set to TRUE if key exists and has been successfully retrieved
+     * \returns entry value as boolean from \a scope given its \a key
+     */
+    bool readBoolEntry( const QString &scope, const QString &key, bool def = false, bool *ok SIP_OUT = nullptr ) const;
 
     //! Remove the given key
     bool removeEntry( const QString &scope, const QString &key );

diff --git a/tests/src/python/test_qgsproject.py b/tests/src/python/test_qgsproject.py
@@ -190,10 +190,22 @@ def testReadEntry(self):
         prj = QgsProject.instance()
         prj.read(os.path.join(TEST_DATA_DIR, 'labeling/test-labeling.qgs'))
 
-        # valid key, valid int value
-        self.assertEqual(prj.readNumEntry("SpatialRefSys", "/ProjectionsEnabled", -1)[0], 0)
+        # add a test entry list
+        prj.writeEntry("TestScope", "/TestListProperty", ["Entry1", "Entry2"])
+
+        # valid key, valid value
+        self.assertEqual(prj.readNumEntry("SpatialRefSys", "/ProjectionsEnabled", -1), (0, True))
+        self.assertEqual(prj.readEntry("SpatialRefSys", "/ProjectCrs"), ("EPSG:32613", True))
+        self.assertEqual(prj.readBoolEntry("PAL", "/ShowingCandidates"), (False, True))
+        self.assertEqual(prj.readNumEntry("PAL", "/CandidatesPolygon"), (8., True))
+        self.assertEqual(prj.readListEntry("TestScope", "/TestListProperty"), (["Entry1", "Entry2"], True))
+
         # invalid key
-        self.assertEqual(prj.readNumEntry("SpatialRefSys", "/InvalidKey", -1)[0], -1)
+        self.assertEqual(prj.readNumEntry("SpatialRefSys", "/InvalidKey", -1), (-1, False))
+        self.assertEqual(prj.readEntry("SpatialRefSys", "/InvalidKey", "wrong"), ("wrong", False))
+        self.assertEqual(prj.readBoolEntry("PAL", "/InvalidKey", True), (True, False))
+        self.assertEqual(prj.readDoubleEntry("PAL", "/InvalidKey", 42.), (42., False))
+        self.assertEqual(prj.readListEntry("TestScope", "/InvalidKey", ["Default1", "Default2"]), (["Default1", "Default2"], False))
 
     def testEmbeddedGroup(self):
         testdata_path = unitTestDataPath('embedded_groups') + '/'