From 872d86eb04e8ea2a2fe77a8c198d3636a698b6f8 Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Fri, 7 Oct 2016 12:14:52 +0200
Subject: [PATCH] [OGR provider] Force REPACK at the first edit action.

In the case where we deal with a shapefile, it is possible that it has
pre-existing holes in the DBF (see #15407), so if using a GDAL version
recent enough (>=2.1.2) to have reliable packing, do a packing at the
first edit action.

Fixes #15407

Conflicts:
	src/providers/ogr/qgsogrprovider.cpp
---
 src/providers/ogr/qgsogrprovider.cpp        | 12 ++++++
 tests/src/python/test_provider_shapefile.py | 44 +++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
index 756b8257a790..5eaee571a3d0 100644
--- a/src/providers/ogr/qgsogrprovider.cpp
+++ b/src/providers/ogr/qgsogrprovider.cpp
@@ -3109,6 +3109,18 @@ void QgsOgrProvider::open( OpenMode mode )
     ogrLayer = ogrOrigLayer = nullptr;
     mValid = false;
 
+#if defined(GDAL_COMPUTE_VERSION)
+    // In the case where we deal with a shapefile, it is possible that it has
+    // pre-existing holes in the DBF (see #15407), so if using a GDAL version
+    // recent enough to have reliable packing, do a packing at the first edit
+    // action.
+    if ( ogrDriverName == "ESRI Shapefile"  &&
+         atoi( GDALVersionInfo( "VERSION_NUM" ) ) >= GDAL_COMPUTE_VERSION( 2, 1, 2 ) )
+    {
+      mShapefileMayBeCorrupted = true;
+    }
+#endif
+
     ogrDataSource = OGROpen( TO8F( mFilePath ), false, &ogrDriver );
 
     mWriteAccess = false;
diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py
index d419f3907dfd..4ebbcb6ac9e8 100644
--- a/tests/src/python/test_provider_shapefile.py
+++ b/tests/src/python/test_provider_shapefile.py
@@ -319,5 +319,49 @@ def testRepackUnderFileLocks(self):
         self.assertTrue(ds.GetLayer(0).GetFeatureCount(), feature_count - 1)
         ds = None
 
+    def testRepackAtFirstSave(self):
+        ''' Test fix for #15407 '''
+
+        # This requires a GDAL fix done per https://trac.osgeo.org/gdal/ticket/6672
+        # but on non-Windows version the test would succeed
+        if int(osgeo.gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(2, 1, 2):
+            return
+
+        tmpdir = tempfile.mkdtemp()
+        self.dirs_to_cleanup.append(tmpdir)
+        srcpath = os.path.join(TEST_DATA_DIR, 'provider')
+        for file in glob.glob(os.path.join(srcpath, 'shapefile.*')):
+            shutil.copy(os.path.join(srcpath, file), tmpdir)
+        datasource = os.path.join(tmpdir, 'shapefile.shp')
+
+        ds = osgeo.ogr.Open(datasource)
+        lyr = ds.GetLayer(0)
+        original_feature_count = lyr.GetFeatureCount()
+        lyr.DeleteFeature(2)
+        ds = None
+
+        vl = QgsVectorLayer('{}|layerid=0'.format(datasource), 'test', 'ogr')
+
+        self.assertTrue(vl.featureCount(), original_feature_count)
+
+        # Edit a feature (attribute change only)
+        self.assertTrue(vl.startEditing())
+        self.assertTrue(vl.dataProvider().changeAttributeValues({0: {0: 100}}))
+
+        # Commit changes and check no error is emitted
+        cbk = ErrorReceiver()
+        vl.dataProvider().raiseError.connect(cbk.receiveError)
+        self.assertTrue(vl.commitChanges())
+        self.assertIsNone(cbk.msg)
+
+        self.assertTrue(vl.featureCount(), original_feature_count - 1)
+
+        vl = None
+
+        # Test repacking has been done
+        ds = osgeo.ogr.Open(datasource)
+        self.assertTrue(ds.GetLayer(0).GetFeatureCount(), original_feature_count - 1)
+        ds = None
+
 if __name__ == '__main__':
     unittest.main()