GeoJSON driver: use VSIOverwriteFile() to fix update of file on Windo…

…ws (fixes qgis/QGIS#28580)
OSGeo · Sep 18, 2019 · 6fb4c77 · 6fb4c77
1 parent 3a0511d
commit 6fb4c77
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 23 deletions.
diff --git 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
@@ -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);
+                        }
                     }
                 }
             }