[OGR provider] Make changeGeometryValues() return false when an error…

… occurs
qgis · May 21, 2021 · b4fd455 · b4fd455
1 parent b5d875c
commit b4fd455
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 1 deletion.
diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp
@@ -2994,6 +2994,7 @@ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
     if ( !theOGRFeature )
     {
       pushError( tr( "OGR error changing geometry: feature %1 not found" ).arg( it.key() ) );
+      returnvalue = false;
       continue;
     }
 
@@ -3016,13 +3017,14 @@ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
       {
         pushError( tr( "OGR error creating geometry for feature %1: %2" ).arg( it.key() ).arg( CPLGetLastErrorMsg() ) );
         OGR_G_DestroyGeometry( newGeometry );
-        newGeometry = nullptr;
+        returnvalue = false;
         continue;
       }
 
       if ( !newGeometry )
       {
         pushError( tr( "OGR error in feature %1: geometry is null" ).arg( it.key() ) );
+        returnvalue = false;
         continue;
       }
 
@@ -3036,6 +3038,7 @@ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
       // Shouldn't happen normally. If it happens, ownership of the geometry
       // may be not really well defined, so better not destroy it, but just
       // the feature.
+      returnvalue = false;
       continue;
     }
 

diff --git a/tests/src/python/test_provider_ogr_gpkg.py b/tests/src/python/test_provider_ogr_gpkg.py
@@ -2197,6 +2197,31 @@ def testFixWrongMetadataReferenceColumnNameUpdate(self):
         gdal.Unlink(tmpfile)
         self.assertNotIn('column_nameIS', sql)
 
+    def testRejectedGeometryUpdate(self):
+        """Test that we correctly behave when a geometry update fails"""
+        tmpfile = os.path.join(self.basetestpath, 'testRejectedGeometryUpdate.gpkg')
+        ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('test', geom_type=ogr.wkbUnknown)
+        f = ogr.Feature(lyr.GetLayerDefn())
+        f.SetGeometry(ogr.CreateGeometryFromWkt('POLYGON ((0 0,0 1,1 1,1 0,0 0))'))
+        lyr.CreateFeature(f)
+        ds.ExecuteSQL("CREATE TRIGGER rejectGeometryUpdate BEFORE UPDATE OF geom ON test FOR EACH ROW BEGIN SELECT RAISE(ABORT, 'update forbidden'); END;")
+        f = None
+        ds = None
+
+        vl = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
+        self.assertTrue(vl.isValid())
+
+        self.assertTrue(vl.startEditing())
+        self.assertTrue(vl.changeGeometry(1, QgsGeometry.fromWkt('Point (0 0)')))
+        self.assertFalse(vl.commitChanges())
+
+        vl = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
+        self.assertTrue(vl.isValid())
+
+        g = [f.geometry() for f in vl.getFeatures()][0]
+        self.assertEqual(g.asWkt(), 'Polygon ((0 0, 0 1, 1 1, 1 0, 0 0))')
+
 
 if __name__ == '__main__':
     unittest.main()