From 6fb4c77fdec91b92e599a76a511924065c818795 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 17 Sep 2019 23:07:10 +0200 Subject: [PATCH] GeoJSON driver: use VSIOverwriteFile() to fix update of file on Windows (fixes https://github.com/qgis/QGIS/issues/28580) --- autotest/ogr/ogr_geojson.py | 23 ++++--- .../geojson/ogrgeojsondatasource.cpp | 67 +++++++++++++++---- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/autotest/ogr/ogr_geojson.py b/autotest/ogr/ogr_geojson.py index 4baa06759e68..f4ddde3b2685 100755 --- a/autotest/ogr/ogr_geojson.py +++ b/autotest/ogr/ogr_geojson.py @@ -2293,17 +2293,18 @@ def test_ogr_geojson_47(): assert 'something' in data - # Test appending to feature collection with "bbox" - gdal.FileFromMemBuffer('/vsimem/ogr_geojson_47.json', """{ "type": "FeatureCollection", "bbox": [0,0,0,0], "features": [ { "type": "Feature", "geometry": { "type": "Point", "coordinates": [0,0]} } ]}""") - ds = ogr.Open('/vsimem/ogr_geojson_47.json', update=1) - lyr = ds.GetLayer(0) - f = ogr.Feature(lyr.GetLayerDefn()) - lyr.CreateFeature(f) - ds = None - ds = ogr.Open('/vsimem/ogr_geojson_47.json') - lyr = ds.GetLayer(0) - assert lyr.GetFeatureCount() == 2 - ds = None + with gdaltest.config_option('OGR_GEOJSON_REWRITE_IN_PLACE', 'YES'): + # Test appending to feature collection with "bbox" + gdal.FileFromMemBuffer('/vsimem/ogr_geojson_47.json', """{ "type": "FeatureCollection", "bbox": [0,0,0,0], "features": [ { "type": "Feature", "geometry": { "type": "Point", "coordinates": [0,0]} } ]}""") + ds = ogr.Open('/vsimem/ogr_geojson_47.json', update=1) + lyr = ds.GetLayer(0) + f = ogr.Feature(lyr.GetLayerDefn()) + lyr.CreateFeature(f) + ds = None + ds = ogr.Open('/vsimem/ogr_geojson_47.json') + lyr = ds.GetLayer(0) + assert lyr.GetFeatureCount() == 2 + ds = None fp = gdal.VSIFOpenL('/vsimem/ogr_geojson_47.json', 'rb') if fp is not None: diff --git a/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsondatasource.cpp b/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsondatasource.cpp index e16eb82badb2..9772c30fd065 100644 --- a/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsondatasource.cpp +++ b/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsondatasource.cpp @@ -1075,22 +1075,65 @@ void OGRGeoJSONDataSource::FlushCache() } if( bOK ) { - CPLString osBackup(pszName_); - osBackup += ".bak"; - if( VSIRename(pszName_, osBackup) < 0 ) + const bool bOverwrite = + CPLTestBool(CPLGetConfigOption("OGR_GEOJSON_REWRITE_IN_PLACE", +#ifdef WIN32 + "YES" +#else + "NO" +#endif + )); + if( bOverwrite ) { - CPLError(CE_Failure, CPLE_AppDefined, - "Cannot create backup copy"); - } - else if( VSIRename(osNewFilename, pszName_) < 0 ) - { - CPLError(CE_Failure, CPLE_AppDefined, - "Cannot rename %s to %s", - osNewFilename.c_str(), pszName_); + VSILFILE* fpTarget = nullptr; + for( int attempt = 0; attempt < 10; attempt++ ) + { + fpTarget = VSIFOpenL(pszName_, "rb+"); + if( fpTarget ) + break; + CPLSleep(0.1); + } + if( !fpTarget ) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Cannot rewrite %s", pszName_); + } + else + { + const bool bCopyOK = CPL_TO_BOOL( + VSIOverwriteFile(fpTarget, osNewFilename)); + VSIFCloseL(fpTarget); + if( bCopyOK ) + { + VSIUnlink(osNewFilename); + } + else + { + CPLError(CE_Failure, CPLE_AppDefined, + "Cannot rewrite %s with content of %s", + pszName_, osNewFilename.c_str()); + } + } } else { - VSIUnlink(osBackup); + CPLString osBackup(pszName_); + osBackup += ".bak"; + if( VSIRename(pszName_, osBackup) < 0 ) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Cannot create backup copy"); + } + else if( VSIRename(osNewFilename, pszName_) < 0 ) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Cannot rename %s to %s", + osNewFilename.c_str(), pszName_); + } + else + { + VSIUnlink(osBackup); + } } } }