Skip to content
Permalink
Browse files

[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
  • Loading branch information
rouault committed Oct 7, 2016
1 parent 61a0a1e commit aa5ec22c1d28e12a06cd6e0f1050d1189e1e90bc
Showing with 56 additions and 0 deletions.
  1. +12 −0 src/providers/ogr/qgsogrprovider.cpp
  2. +44 −0 tests/src/python/test_provider_shapefile.py
@@ -3367,6 +3367,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 = QgsOgrProviderUtils::OGROpenWrapper( TO8F( mFilePath ), false, &ogrDriver );

mWriteAccess = false;
@@ -437,5 +437,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()

0 comments on commit aa5ec22

Please sign in to comment.
You can’t perform that action at this time.