[OGR] Defer repacking while in explicit updateMode

qgis · Sep 21, 2017 · 59ed19f · 59ed19f
1 parent 54ea029
commit 59ed19f
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 5 deletions.
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -428,6 +428,7 @@ QgsOgrProvider::QgsOgrProvider( QString const &uri )
   , mDynamicWriteAccess( false )
   , mShapefileMayBeCorrupted( false )
   , mUpdateModeStackDepth( 0 )
+  , mDeferRepack( false )
   , mCapabilities( 0 )
 {
   QgsApplication::registerOgrDrivers();
@@ -1971,11 +1972,14 @@ bool QgsOgrProvider::doInitialActionsForEdition()
   if ( !mValid )
     return false;
 
-  if ( !mWriteAccess && mWriteAccessPossible && mDynamicWriteAccess )
+  // If mUpdateModeStackDepth > 0, it means that an updateMode is already active and that we have write access
+  if ( mUpdateModeStackDepth == 0 )
   {
     QgsDebugMsg( "Enter update mode implictly" );
     if ( !enterUpdateMode() )
       return false;
+    // For implicitly entered updateMode, don't defer repacking
+    mDeferRepack = false;
   }
 
   return true;
@@ -3416,10 +3420,13 @@ bool QgsOgrProvider::syncToDisc()
     pushError( tr( "OGR error syncing to disk: %1" ).arg( CPLGetLastErrorMsg() ) );
   }
 
-  if ( mShapefileMayBeCorrupted )
-    repack();
+  if ( !mDeferRepack )
+  {
+    if ( mShapefileMayBeCorrupted )
+      repack();
 
-  mShapefileMayBeCorrupted = false;
+    mShapefileMayBeCorrupted = false;
+  }
 
   QgsOgrConnPool::instance()->ref( dataSourceUri() );
   if ( shapeIndex )
@@ -3844,6 +3851,7 @@ bool QgsOgrProvider::enterUpdateMode()
     }
   }
   ++mUpdateModeStackDepth;
+  mDeferRepack = true;
   return true;
 }
 
@@ -3860,6 +3868,15 @@ bool QgsOgrProvider::leaveUpdateMode()
     mUpdateModeStackDepth = 0;
     return false;
   }
+  if ( mDeferRepack && mUpdateModeStackDepth == 0 )
+  {
+    // Only repack once update mode is inactive
+    if ( mShapefileMayBeCorrupted )
+      repack();
+
+    mShapefileMayBeCorrupted = false;
+    mDeferRepack = false;
+  }
   if ( !mDynamicWriteAccess )
   {
     return true;

diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h
@@ -252,6 +252,8 @@ class QgsOgrProvider : public QgsVectorDataProvider
 
     int mUpdateModeStackDepth;
 
+    bool mDeferRepack;
+
     void computeCapabilities();
 
     QgsVectorDataProvider::Capabilities mCapabilities;

diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py
@@ -459,7 +459,23 @@ def testDeleteShapes(self):
         # Test the content of the shapefile while it is still opened
         ds = osgeo.ogr.Open(datasource)
         # Test repacking has been done
-        self.assertTrue(ds.GetLayer(0).GetFeatureCount(), feature_count - 1)
+        self.assertTrue(ds.GetLayer(0).GetFeatureCount() == feature_count - 1)
+        ds = None
+
+        # Delete another feature while in update mode
+        self.assertTrue(2 == 2)
+        vl.dataProvider().enterUpdateMode()
+        vl.dataProvider().deleteFeatures([0])
+
+        # Test that repacking has not been done (since in update mode)
+        ds = osgeo.ogr.Open(datasource)
+        self.assertTrue(ds.GetLayer(0).GetFeatureCount() == feature_count - 1)
+        ds = None
+
+        # Test that repacking was performed when leaving updateMode
+        vl.dataProvider().leaveUpdateMode()
+        ds = osgeo.ogr.Open(datasource)
+        self.assertTrue(ds.GetLayer(0).GetFeatureCount() == feature_count - 2)
         ds = None
 
         vl = None