Skip to content

Commit

Permalink
[OGR provider] Check if REPACK has emitted errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rouault committed Oct 6, 2016
1 parent 5fb71b1 commit ee87dc3
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,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 ) )
{
Expand Down
57 changes: 57 additions & 0 deletions tests/src/python/test_provider_shapefile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()

0 comments on commit ee87dc3

Please sign in to comment.