From 59ed19fff08443da475424218f23499501f8e7d8 Mon Sep 17 00:00:00 2001
From: Sandro Mani <manisandro@gmail.com>
Date: Wed, 20 Sep 2017 18:08:34 +0200
Subject: [PATCH] [OGR] Defer repacking while in explicit updateMode

---
 src/providers/ogr/qgsogrprovider.cpp        | 25 +++++++++++++++++----
 src/providers/ogr/qgsogrprovider.h          |  2 ++
 tests/src/python/test_provider_shapefile.py | 18 ++++++++++++++-
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
index af8bf1ecc97..748bc42dacf 100644
--- 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
index a8a3b9bbba3..f360d980ccb 100644
--- 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
index 20713c6229e..1f75c08ab78 100644
--- a/tests/src/python/test_provider_shapefile.py
+++ b/tests/src/python/test_provider_shapefile.py
@@ -459,7 +459,23 @@ class TestPyQgsShapefileProvider(unittest.TestCase, ProviderTestCase):
         # 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