Avoid signalling repaint / data signals when a setSubsetString doesn'…

…t actually change (#7217) Behavior across providers harmonized & test added.
qgis · Jun 11, 2018 · 0a325f1 · 0a325f1
1 parent 6e3ee46
commit 0a325f1
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 3 deletions.
diff --git a/src/core/providers/memory/qgsmemoryprovider.cpp b/src/core/providers/memory/qgsmemoryprovider.cpp
@@ -565,6 +565,9 @@ bool QgsMemoryProvider::setSubsetString( const QString &theSQL, bool updateFeatu
       return false;
   }
 
+  if ( theSQL == mSubsetString )
+    return true;
+
   mSubsetString = theSQL;
   clearMinMaxCache();
   mExtent.setMinimal();

diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -890,6 +890,9 @@ bool QgsVectorLayer::setSubsetString( const QString &subset )
     return false;
   }
 
+  if ( subset == mDataProvider->subsetString() )
+    return true;
+
   bool res = mDataProvider->setSubsetString( subset );
 
   // get the updated data source string from the provider

diff --git a/src/providers/oracle/qgsoracleprovider.cpp b/src/providers/oracle/qgsoracleprovider.cpp
@@ -2095,6 +2095,9 @@ bool QgsOracleProvider::setSubsetString( const QString &theSQL, bool updateFeatu
   if ( !mConnection )
     return false;
 
+  if ( theSQL.trimmed() == mSqlWhereClause )
+    return true;
+
   QString prevWhere = mSqlWhereClause;
 
   mSqlWhereClause = theSQL.trimmed();

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -3098,6 +3098,9 @@ QgsVectorDataProvider::Capabilities QgsPostgresProvider::capabilities() const
 
 bool QgsPostgresProvider::setSubsetString( const QString &theSQL, bool updateFeatureCount )
 {
+  if ( theSQL.trimmed() == mSqlWhereClause )
+    return true;
+
   QString prevWhere = mSqlWhereClause;
 
   mSqlWhereClause = theSQL.trimmed();

diff --git a/src/providers/spatialite/qgsspatialiteprovider.cpp b/src/providers/spatialite/qgsspatialiteprovider.cpp
@@ -604,9 +604,10 @@ QgsSpatiaLiteProvider::QgsSpatiaLiteProvider( QString const &uri, const Provider
                   << QgsVectorDataProvider::NativeType( tr( "Array of decimal numbers (double)" ), SPATIALITE_ARRAY_PREFIX.toUpper() + "REAL" + SPATIALITE_ARRAY_SUFFIX.toUpper(), QVariant::List, 0, 0, 0, 0, QVariant::Double )
                   << QgsVectorDataProvider::NativeType( tr( "Array of whole numbers (integer)" ), SPATIALITE_ARRAY_PREFIX.toUpper() + "INTEGER" + SPATIALITE_ARRAY_SUFFIX.toUpper(), QVariant::List, 0, 0, 0, 0, QVariant::LongLong )
                 );
+
   // Update extent and feature count
   if ( ! mSubsetString.isEmpty() )
-    setSubsetString( mSubsetString, true );
+    getTableSummary();
 
   mValid = true;
 }
@@ -3450,6 +3451,9 @@ QString QgsSpatiaLiteProvider::subsetString() const
 
 bool QgsSpatiaLiteProvider::setSubsetString( const QString &theSQL, bool updateFeatureCount )
 {
+  if ( theSQL == mSubsetString )
+    return true;
+
   QString prevSubsetString = mSubsetString;
   mSubsetString = theSQL;
 
@@ -3461,6 +3465,7 @@ bool QgsSpatiaLiteProvider::setSubsetString( const QString &theSQL, bool updateF
   // update feature count and extents
   if ( updateFeatureCount && getTableSummary() )
   {
+    emit dataChanged();
     return true;
   }
 
@@ -3473,8 +3478,6 @@ bool QgsSpatiaLiteProvider::setSubsetString( const QString &theSQL, bool updateF
 
   getTableSummary();
 
-  emit dataChanged();
-
   return false;
 }
 

diff --git a/src/providers/virtual/qgsvirtuallayerprovider.cpp b/src/providers/virtual/qgsvirtuallayerprovider.cpp
@@ -503,10 +503,16 @@ QString QgsVirtualLayerProvider::subsetString() const
 
 bool QgsVirtualLayerProvider::setSubsetString( const QString &subset, bool updateFeatureCount )
 {
+  if ( subset == mSubset )
+    return true;
+
   mSubset = subset;
   clearMinMaxCache();
   if ( updateFeatureCount )
     updateStatistics();
+
+  emit dataChanged();
+
   return true;
 }
 

diff --git a/src/providers/wfs/qgswfsprovider.cpp b/src/providers/wfs/qgswfsprovider.cpp
@@ -679,6 +679,9 @@ bool QgsWFSProvider::setSubsetString( const QString &theSQL, bool updateFeatureC
 {
   QgsDebugMsg( QString( "theSql = '%1'" ).arg( theSQL ) );
 
+  if ( theSQL == mSubsetString )
+    return true;
+
   // Invalid and cancel current download before altering fields, etc...
   // (crashes might happen if not done at the beginning)
   mShared->invalidateCache();
@@ -710,13 +713,17 @@ bool QgsWFSProvider::setSubsetString( const QString &theSQL, bool updateFeatureC
     mShared->mURI.setSql( QString() );
     mShared->mURI.setFilter( theSQL );
   }
+
   setDataSourceUri( mShared->mURI.uri() );
   QString errorMsg;
   if ( !mShared->computeFilter( errorMsg ) )
     QgsMessageLog::logMessage( errorMsg, tr( "WFS" ) );
   reloadData();
   if ( updateFeatureCount )
     featureCount();
+
+  emit dataChanged();
+
   return true;
 }
 

diff --git a/tests/src/python/provider_python.py b/tests/src/python/provider_python.py
@@ -373,10 +373,13 @@ def subsetString(self):
         return self._subset_string
 
     def setSubsetString(self, subsetString):
+        if subsetString == self._subset_string:
+            return True
         self._subset_string = subsetString
         self.updateExtents()
         self.clearMinMaxCache()
         self.dataChanged.emit()
+        return True
 
     def supportsSubsetString(self):
         return True

diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py
@@ -30,6 +30,7 @@
     QgsTestUtils,
     NULL
 )
+from qgis.PyQt.QtTest import QSignalSpy
 
 from utilities import compareWkt
 from featuresourcetestbase import FeatureSourceTestCase
@@ -157,9 +158,16 @@ def testSubsetString(self):
             print('Provider does not support subset strings')
             return
 
+        changed_spy = QSignalSpy(self.source.dataChanged)
         subset = self.getSubsetString()
         self.source.setSubsetString(subset)
         self.assertEqual(self.source.subsetString(), subset)
+        self.assertEqual(len(changed_spy), 1)
+
+        # No signal should be emitted if the subset string is not modified
+        self.source.setSubsetString(subset)
+        self.assertEqual(len(changed_spy), 1)
+
         result = set([f['pk'] for f in self.source.getFeatures()])
         all_valid = (all(f.isValid() for f in self.source.getFeatures()))
         self.source.setSubsetString(None)