[project] Avoid needlessly dirtying when written value does not change

qgis · Nov 14, 2018 · 8b9e86d · 8b9e86d
1 parent 6b2d44a
commit 8b9e86d
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 15 deletions.
diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp
@@ -197,11 +197,13 @@ QgsProjectProperty *findKey_( const QString &scope,
 \param key key name
 \param rootProperty is the property from which to start adding
 \param value the value associated with the key
+\param propertiesModified the parameter will be set to true if the written entry modifies pre-existing properties
 */
 QgsProjectProperty *addKey_( const QString &scope,
                              const QString &key,
                              QgsProjectPropertyKey *rootProperty,
-                             const QVariant &value )
+                             const QVariant &value,
+                             bool &propertiesModified )
 {
   QStringList keySequence = makeKeyTokens_( scope, key );
 
@@ -210,6 +212,7 @@ QgsProjectProperty *addKey_( const QString &scope,
   QgsProjectProperty *nextProperty; // link to next property down hierarchy
   QgsProjectPropertyKey *newPropertyKey = nullptr;
 
+  propertiesModified = false;
   while ( ! keySequence.isEmpty() )
   {
     // if the current head of the sequence list matches the property name,
@@ -223,14 +226,24 @@ QgsProjectProperty *addKey_( const QString &scope,
       // name to store the value
       if ( 1 == keySequence.count() )
       {
-        currentProperty->setValue( keySequence.front(), value );
+        QgsProjectProperty *property = currentProperty->find( keySequence.front() );
+        if ( !property || property->value() != value )
+        {
+          currentProperty->setValue( keySequence.front(), value );
+          propertiesModified = true;
+        }
+
         return currentProperty;
       }
       // we're at the top element if popping the keySequence element
       // will leave it empty; in that case, just add the key
       else if ( keySequence.isEmpty() )
       {
-        currentProperty->setValue( value );
+        if ( currentProperty->value() != value )
+        {
+          currentProperty->setValue( value );
+          propertiesModified = true;
+        }
 
         return currentProperty;
       }
@@ -263,7 +276,6 @@ QgsProjectProperty *addKey_( const QString &scope,
   }
 
   return nullptr;
-
 }
 
 
@@ -325,7 +337,6 @@ void removeKey_( const QString &scope,
       return;
     }
   }
-
 }
 
 QgsProject::QgsProject( QObject *parent )
@@ -1899,37 +1910,57 @@ bool QgsProject::writeProjectFile( const QString &filename )
 
 bool QgsProject::writeEntry( const QString &scope, QString const &key, bool value )
 {
-  setDirty( true );
+  bool propertiesModified;
+  bool success = addKey_( scope, key, &mProperties, value, propertiesModified );
+
+  if ( propertiesModified )
+    setDirty( true );
 
-  return addKey_( scope, key, &mProperties, value );
+  return success;
 }
 
 bool QgsProject::writeEntry( const QString &scope, const QString &key, double value )
 {
-  setDirty( true );
+  bool propertiesModified;
+  bool success = addKey_( scope, key, &mProperties, value, propertiesModified );
+
+  if ( propertiesModified )
+    setDirty( true );
 
-  return addKey_( scope, key, &mProperties, value );
+  return success;
 }
 
 bool QgsProject::writeEntry( const QString &scope, QString const &key, int value )
 {
-  setDirty( true );
+  bool propertiesModified;
+  bool success = addKey_( scope, key, &mProperties, value, propertiesModified );
+
+  if ( propertiesModified )
+    setDirty( true );
 
-  return addKey_( scope, key, &mProperties, value );
+  return success;
 }
 
 bool QgsProject::writeEntry( const QString &scope, const QString &key, const QString &value )
 {
-  setDirty( true );
+  bool propertiesModified;
+  bool success = addKey_( scope, key, &mProperties, value, propertiesModified );
+
+  if ( propertiesModified )
+    setDirty( true );
 
-  return addKey_( scope, key, &mProperties, value );
+  return success;
 }
 
 bool QgsProject::writeEntry( const QString &scope, const QString &key, const QStringList &value )
 {
-  setDirty( true );
+  bool propertiesModified;
+  bool success = addKey_( scope, key, &mProperties, value, propertiesModified );
+
+  if ( propertiesModified )
+    setDirty( true );
 
-  return addKey_( scope, key, &mProperties, value );
+  return success;
 }
 
 QStringList QgsProject::readListEntry( const QString &scope,

diff --git a/tests/src/python/test_qgsproject.py b/tests/src/python/test_qgsproject.py
@@ -1121,6 +1121,25 @@ def testWriteEntry(self):
         self.assertTrue(ok)
         self.assertEqual(q, query)
 
+    def testWriteEntryDirtying(self):
+
+        project = QgsProject()
+
+        # writing a new entry should dirty the project
+        project.setDirty(False)
+        self.assertTrue(project.writeEntry('myscope', 'myentry', True))
+        self.assertTrue(project.isDirty())
+
+        # over-writing a pre-existing entry with the same value should _not_ dirty the project
+        project.setDirty(False)
+        self.assertTrue(project.writeEntry('myscope', 'myentry', True))
+        self.assertFalse(project.isDirty())
+
+        # over-writing a pre-existing entry with a different value should dirty the project
+        project.setDirty(False)
+        self.assertTrue(project.writeEntry('myscope', 'myentry', False))
+        self.assertTrue(project.isDirty())
+
 
 if __name__ == '__main__':
     unittest.main()