Get rid of the connect for capabilities update

qgis · Jan 6, 2018 · 248ad5f · 248ad5f
1 parent c1677c5
commit 248ad5f
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 101 deletions.
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -528,90 +528,7 @@ QgsAbstractFeatureSource *QgsOgrProvider::featureSource() const
 
 bool QgsOgrProvider::setSubsetString( const QString &theSQL, bool updateFeatureCount )
 {
-  QgsCPLErrorHandler handler;
-
-  if ( !mOgrOrigLayer )
-    return false;
-
-  if ( theSQL == mSubsetString && mFeaturesCounted != QgsVectorDataProvider::Uncounted )
-    return true;
-
-  if ( !theSQL.isEmpty() )
-  {
-    bool origFidAdded = false;
-    QMutex *mutex = nullptr;
-    OGRLayerH layer = mOgrOrigLayer->getHandleAndMutex( mutex );
-    GDALDatasetH ds = mOgrOrigLayer->getDatasetHandleAndMutex( mutex );
-    OGRLayerH subsetLayerH;
-    {
-      QMutexLocker locker( mutex );
-      subsetLayerH = QgsOgrProviderUtils::setSubsetString( layer, ds, textEncoding(), theSQL, origFidAdded );
-    }
-    if ( !subsetLayerH )
-    {
-      pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
-      return false;
-    }
-    mOgrSqlLayer = QgsOgrProviderUtils::getSqlLayer( mOgrOrigLayer.get(), subsetLayerH, theSQL );
-    Q_ASSERT( mOgrSqlLayer.get() );
-    mOgrLayer = mOgrSqlLayer.get();
-  }
-  else
-  {
-    mOgrSqlLayer.reset();
-    mOgrLayer = mOgrOrigLayer.get();
-  }
-  mSubsetString = theSQL;
-
-  QString uri = mFilePath;
-  if ( !mLayerName.isNull() )
-  {
-    uri += QStringLiteral( "|layername=%1" ).arg( mLayerName );
-  }
-  else if ( mLayerIndex >= 0 )
-  {
-    uri += QStringLiteral( "|layerid=%1" ).arg( mLayerIndex );
-  }
-
-  if ( !mSubsetString.isEmpty() )
-  {
-    uri += QStringLiteral( "|subset=%1" ).arg( mSubsetString );
-  }
-
-  if ( mOgrGeometryTypeFilter != wkbUnknown )
-  {
-    uri += QStringLiteral( "|geometrytype=%1" ).arg( ogrWkbGeometryTypeName( mOgrGeometryTypeFilter ) );
-  }
-
-  if ( uri != dataSourceUri() )
-  {
-    QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
-    setDataSourceUri( uri );
-    QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
-  }
-
-  mOgrLayer->ResetReading();
-
-  // getting the total number of features in the layer
-  // TODO: This can be expensive, do we really need it!
-  if ( updateFeatureCount )
-  {
-    recalculateFeatureCount();
-  }
-
-  // check the validity of the layer
-  QgsDebugMsgLevel( "checking validity", 4 );
-  loadFields();
-  QgsDebugMsgLevel( "Done checking validity", 4 );
-
-  invalidateCachedExtent( false );
-
-  // Changing the filter may change capabilities
-  emit capabilitiesNeedUpdate();
-
-  emit dataChanged();
-
-  return true;
+  return _setSubsetString( theSQL, updateFeatureCount, true );
 }
 
 QString QgsOgrProvider::subsetString() const
@@ -1763,6 +1680,96 @@ bool QgsOgrProvider::commitTransaction()
   return true;
 }
 
+bool QgsOgrProvider::_setSubsetString( const QString &theSQL, bool updateFeatureCount, bool updateCapabilities )
+{
+  QgsCPLErrorHandler handler;
+
+  if ( !mOgrOrigLayer )
+    return false;
+
+  if ( theSQL == mSubsetString && mFeaturesCounted != QgsVectorDataProvider::Uncounted )
+    return true;
+
+  if ( !theSQL.isEmpty() )
+  {
+    bool origFidAdded = false;
+    QMutex *mutex = nullptr;
+    OGRLayerH layer = mOgrOrigLayer->getHandleAndMutex( mutex );
+    GDALDatasetH ds = mOgrOrigLayer->getDatasetHandleAndMutex( mutex );
+    OGRLayerH subsetLayerH;
+    {
+      QMutexLocker locker( mutex );
+      subsetLayerH = QgsOgrProviderUtils::setSubsetString( layer, ds, textEncoding(), theSQL, origFidAdded );
+    }
+    if ( !subsetLayerH )
+    {
+      pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
+      return false;
+    }
+    mOgrSqlLayer = QgsOgrProviderUtils::getSqlLayer( mOgrOrigLayer.get(), subsetLayerH, theSQL );
+    Q_ASSERT( mOgrSqlLayer.get() );
+    mOgrLayer = mOgrSqlLayer.get();
+  }
+  else
+  {
+    mOgrSqlLayer.reset();
+    mOgrLayer = mOgrOrigLayer.get();
+  }
+  mSubsetString = theSQL;
+
+  QString uri = mFilePath;
+  if ( !mLayerName.isNull() )
+  {
+    uri += QStringLiteral( "|layername=%1" ).arg( mLayerName );
+  }
+  else if ( mLayerIndex >= 0 )
+  {
+    uri += QStringLiteral( "|layerid=%1" ).arg( mLayerIndex );
+  }
+
+  if ( !mSubsetString.isEmpty() )
+  {
+    uri += QStringLiteral( "|subset=%1" ).arg( mSubsetString );
+  }
+
+  if ( mOgrGeometryTypeFilter != wkbUnknown )
+  {
+    uri += QStringLiteral( "|geometrytype=%1" ).arg( ogrWkbGeometryTypeName( mOgrGeometryTypeFilter ) );
+  }
+
+  if ( uri != dataSourceUri() )
+  {
+    QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
+    setDataSourceUri( uri );
+    QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
+  }
+
+  mOgrLayer->ResetReading();
+
+  // getting the total number of features in the layer
+  // TODO: This can be expensive, do we really need it!
+  if ( updateFeatureCount )
+  {
+    recalculateFeatureCount();
+  }
+
+  // check the validity of the layer
+  QgsDebugMsgLevel( "checking validity", 4 );
+  loadFields();
+  QgsDebugMsgLevel( "Done checking validity", 4 );
+
+  invalidateCachedExtent( false );
+
+  // Changing the filter may change capabilities
+  if ( updateCapabilities )
+    computeCapabilities();
+
+  emit dataChanged();
+
+  return true;
+
+}
+
 
 bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_map )
 {
@@ -3981,7 +3988,8 @@ void QgsOgrProvider::open( OpenMode mode )
     mSubsetString.clear();
     // Block signals to avoid endless recusion reloadData -> emit dataChanged -> reloadData
     blockSignals( true );
-    mValid = setSubsetString( origSubsetString );
+    // Do not update capabilities: it will be done later
+    mValid = _setSubsetString( origSubsetString, true, false );
     blockSignals( false );
     if ( mValid )
     {
@@ -4044,22 +4052,13 @@ void QgsOgrProvider::open( OpenMode mode )
       {
         int featuresCountedBackup = mFeaturesCounted;
         mFeaturesCounted = -1;
-        mValid = setSubsetString( mSubsetString, false );
+        // Do not update capabilities here
+        mValid = _setSubsetString( mSubsetString, false, false );
         mFeaturesCounted = featuresCountedBackup;
       }
     }
   }
 
-  // Updates capabilities when the subset string changes, it needs to be
-  // asynchronous because if called directly if will recurse
-  if ( mode == OpenModeInitial )
-  {
-    connect( this, &QgsOgrProvider::capabilitiesNeedUpdate, this, [ =  ]
-    {
-      computeCapabilities();
-    } );
-  }
-
   // For debug/testing purposes
   if ( !mValid )
     setProperty( "_debug_open_mode", "invalid" );

diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h
@@ -210,6 +210,9 @@ class QgsOgrProvider : public QgsVectorDataProvider
     //! Commits a transaction
     bool commitTransaction();
 
+    //! Does the real job of settings the subset string and adds an argument to disable update capabilities
+    bool _setSubsetString( const QString &theSQL, bool updateFeatureCount = true, bool updateCapabilities = true );
+
     void addSubLayerDetailsToSubLayerList( int i, QgsOgrLayer *layer ) const;
 
     QgsFields mAttributeFields;
@@ -309,11 +312,6 @@ class QgsOgrProvider : public QgsVectorDataProvider
     void setupProxy();
 #endif
 
-  signals:
-
-    //! Emitted when capabilities need to be re-computed
-    void capabilitiesNeedUpdate( );
-
 };
 
 /**