Add a base class implemention for QgsVectorDataProvider::changeFeatures

nyalldawson · nyalldawson · commit b256075e30b3 · 2016-12-27T13:55:42.000+10:00
Previously this method would only succeed for providers which
explicitly implement it. Now, providers which do not implement
changeFeatures but do support both ChangeAttributeValues
and ChangeGeometries capabilities will use a non-optimised
version of changeFeatures which calls changeAttributeValues
and changeGeometries in turn.

This makes QgsVectorDataProvider::changeFeatures easier to use
in scripts - instead of writing manual fallbacks for providers
which do not implement it you can instead safely call this
method regardless of the provider and it will succeed wherever
both the attributes/geometries can be changed.

Also add a provider unit test covering this.
diff --git a/src/core/qgsvectordataprovider.cpp b/src/core/qgsvectordataprovider.cpp
@@ -128,9 +128,13 @@ bool QgsVectorDataProvider::changeGeometryValues( const QgsGeometryMap &geometry
 bool QgsVectorDataProvider::changeFeatures( const QgsChangedAttributesMap &attr_map,
     const QgsGeometryMap &geometry_map )
 {
-  Q_UNUSED( attr_map );
-  Q_UNUSED( geometry_map );
-  return false;
+  if ( !( capabilities() & ChangeAttributeValues ) || !( capabilities() & ChangeGeometries ) )
+    return false;
+
+  bool result = true;
+  result = result && changeAttributeValues( attr_map );
+  result = result && changeGeometryValues( geometry_map );
+  return result;
 }
 
 bool QgsVectorDataProvider::createSpatialIndex()
diff --git a/src/core/qgsvectordataprovider.h b/src/core/qgsvectordataprovider.h
@@ -274,14 +274,18 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider
     virtual bool renameAttributes( const QgsFieldNameMap& renamedAttributes );
 
     /**
-     * Changes attribute values of existing features.
+     * Changes attribute values of existing features. This should
+     * succeed if the provider reports the ChangeAttributeValues capability.
      * @param attr_map a map containing changed attributes
      * @return true in case of success and false in case of failure
      */
     virtual bool changeAttributeValues( const QgsChangedAttributesMap &attr_map );
 
     /**
-     * Changes attribute values and geometries of existing features.
+     * Changes attribute values and geometries of existing features. This should
+     * succeed if the provider reports both the ChangeAttributeValues and
+     * ChangeGeometries capabilities. Providers which report the ChangeFeatures
+     * capability implement an optimised version of this method.
      * @param attr_map a map containing changed attributes
      * @param geometry_map   A QgsGeometryMap whose index contains the feature IDs
      *                       that will have their geometries changed.
diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py
@@ -802,3 +802,51 @@ def testChangeGeometries(self):
             # expect fail
             self.assertFalse(l.dataProvider().changeGeometryValues(changes),
                              'Provider reported no ChangeGeometries capability, but returned true to changeGeometryValues')
+
+    def testChangeFeatures(self):
+        if not getattr(self, 'getEditableLayer', None):
+            return
+
+        l = self.getEditableLayer()
+        self.assertTrue(l.isValid())
+
+        features = [f for f in l.dataProvider().getFeatures()]
+
+        # find 2 features to change attributes for
+        features = [f for f in l.dataProvider().getFeatures()]
+        # need to keep order here
+        to_change = [f for f in features if f.attributes()[0] == 1]
+        to_change.extend([f for f in features if f.attributes()[0] == 2])
+        # changes by feature id, for changeAttributeValues call
+        attribute_changes = {to_change[0].id(): {1: 501, 3: 'new string'}, to_change[1].id(): {1: 502, 4: 'NEW'}}
+        # changes by pk, for testing after retrieving changed features
+        new_attr_map = {1: {1: 501, 3: 'new string'}, 2: {1: 502, 4: 'NEW'}}
+
+        # find 2 features to change geometries for
+        to_change = [f for f in features if f.attributes()[0] == 1]
+        to_change.extend([f for f in features if f.attributes()[0] == 3])
+        # changes by feature id, for changeGeometryValues call
+        geometry_changes = {to_change[0].id(): QgsGeometry.fromWkt('Point (10 20)'), to_change[1].id(): QgsGeometry()}
+        # changes by pk, for testing after retrieving changed features
+        new_geom_map = {1: QgsGeometry.fromWkt('Point ( 10 20 )'), 3: QgsGeometry()}
+
+        if l.dataProvider().capabilities() & QgsVectorDataProvider.ChangeGeometries and l.dataProvider().capabilities() & QgsVectorDataProvider.ChangeAttributeValues:
+            # expect success
+            result = l.dataProvider().changeFeatures(attribute_changes, geometry_changes)
+            self.assertTrue(result,
+                            'Provider reported ChangeGeometries and ChangeAttributeValues capability, but returned False to changeFeatures')
+
+            # check result
+            self.testGetFeatures(l.dataProvider(), changed_attributes=new_attr_map, changed_geometries=new_geom_map)
+
+            # change empty list, should return true for consistency
+            self.assertTrue(l.dataProvider().changeFeatures({}, {}))
+
+        elif not l.dataProvider().capabilities() & QgsVectorDataProvider.ChangeGeometries:
+            # expect fail
+            self.assertFalse(l.dataProvider().changeFeatures(attribute_changes, geometry_changes),
+                             'Provider reported no ChangeGeometries capability, but returned true to changeFeatures')
+        elif not l.dataProvider().capabilities() & QgsVectorDataProvider.ChangeAttributeValues:
+            # expect fail
+            self.assertFalse(l.dataProvider().changeFeatures(attribute_changes, geometry_changes),
+                             'Provider reported no ChangeAttributeValues capability, but returned true to changeFeatures')
