[OGR provider] Check if REPACK has emitted errors

Refs #15393 and #15570 Real fix for the REPACK issues has been committed per GDAL ticket https://trac.osgeo.org/gdal/ticket/6672 (GDAL 2.1.2 or above) Add test to simulate the situations that caused problems.
qgis · Oct 6, 2016 · 5b6e4b8 · 5b6e4b8
1 parent daf8f7a
commit 5b6e4b8
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 0 deletions.
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -154,7 +154,12 @@ void QgsOgrProvider::repack()
   // run REPACK on shape files
   QByteArray sql = QByteArray( "REPACK " ) + layerName;   // don't quote the layer name as it works with spaces in the name and won't work if the name is quoted
   QgsDebugMsg( QString( "SQL: %1" ).arg( FROM8( sql ) ) );
+  CPLErrorReset();
   OGR_DS_ExecuteSQL( ogrDataSource, sql.constData(), nullptr, nullptr );
+  if ( CPLGetLastErrorType() != CE_None )
+  {
+    pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
+  }
 
   if ( mFilePath.endsWith( ".shp", Qt::CaseInsensitive ) || mFilePath.endsWith( ".dbf", Qt::CaseInsensitive ) )
   {

diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py
@@ -30,6 +30,19 @@
 TEST_DATA_DIR = unitTestDataPath()
 
 
+def GDAL_COMPUTE_VERSION(maj, min, rev):
+    return ((maj) * 1000000 + (min) * 10000 + (rev) * 100)
+
+
+class ErrorReceiver():
+
+    def __init__(self):
+        self.msg = None
+
+    def receiveError(self, msg):
+        self.msg = msg
+
+
 class TestPyQgsShapefileProvider(unittest.TestCase, ProviderTestCase):
 
     @classmethod
@@ -380,5 +393,49 @@ def testDeleteShapes(self):
 
         vl = None
 
+    def testRepackUnderFileLocks(self):
+        ''' Test fix for #15570 and #15393 '''
+
+        # 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')
+
+        vl = QgsVectorLayer('{}|layerid=0'.format(datasource), 'test', 'ogr')
+        feature_count = vl.featureCount()
+
+        # Keep a file descriptor opened on the .dbf, .shp and .shx
+        f_shp = open(os.path.join(tmpdir, 'shapefile.shp'), 'rb')
+        f_shx = open(os.path.join(tmpdir, 'shapefile.shx'), 'rb')
+        f_dbf = open(os.path.join(tmpdir, 'shapefile.dbf'), 'rb')
+
+        # Delete a feature
+        self.assertTrue(vl.startEditing())
+        self.assertTrue(vl.deleteFeature(1))
+
+        # Commit changes and check no error is emitted
+        cbk = ErrorReceiver()
+        vl.dataProvider().raiseError.connect(cbk.receiveError)
+        self.assertTrue(vl.commitChanges())
+        self.assertIsNone(cbk.msg)
+
+        vl = None
+
+        del f_shp
+        del f_shx
+        del f_dbf
+
+        # Test repacking has been done
+        ds = osgeo.ogr.Open(datasource)
+        self.assertTrue(ds.GetLayer(0).GetFeatureCount(), feature_count - 1)
+        ds = None
+
 if __name__ == '__main__':
     unittest.main()