Checking of invalid values when calling edit buffer operations

qgis · Dec 19, 2012 · 648bcc0 · 648bcc0
1 parent 31f6ce0
commit 648bcc0
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 4 deletions.
diff --git a/src/core/qgsvectorlayereditbuffer.cpp b/src/core/qgsvectorlayereditbuffer.cpp
@@ -86,6 +86,12 @@ bool QgsVectorLayerEditBuffer::addFeature( QgsFeature& f )
     return false;
   }
 
+  int layerFieldCount = L->dataProvider()->fields().count() + mAddedAttributes.count() - mDeletedAttributeIds.count();
+  if (layerFieldCount != f.attributes().count())
+    return false;
+
+  // TODO: check correct geometry type
+
   L->undoStack()->push( new QgsVectorLayerUndoCommandAddFeature( this, f ) );
   return true;
 }
@@ -94,9 +100,7 @@ bool QgsVectorLayerEditBuffer::addFeature( QgsFeature& f )
 bool QgsVectorLayerEditBuffer::addFeatures( QgsFeatureList& features )
 {
   if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::AddFeatures ) )
-  {
     return false;
-  }
 
   for ( QgsFeatureList::iterator iter = features.begin(); iter != features.end(); ++iter )
   {
@@ -111,6 +115,9 @@ bool QgsVectorLayerEditBuffer::addFeatures( QgsFeatureList& features )
 
 bool QgsVectorLayerEditBuffer::deleteFeature( QgsFeatureId fid )
 {
+  if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::DeleteFeatures ) )
+    return false;
+
   if ( FID_IS_NEW( fid ) )
   {
     if (!mAddedFeatures.contains(fid))
@@ -129,25 +136,52 @@ bool QgsVectorLayerEditBuffer::deleteFeature( QgsFeatureId fid )
 
 bool QgsVectorLayerEditBuffer::changeGeometry( QgsFeatureId fid, QgsGeometry* geom )
 {
+  if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::ChangeGeometries ) )
+    return false;
+
   if ( !L->hasGeometryType() )
   {
     return false;
   }
 
+  if ( FID_IS_NEW( fid ) )
+  {
+    if (!mAddedFeatures.contains(fid))
+      return false;
+  }
+
+  // TODO: check compatible geometry
+
   L->undoStack()->push( new QgsVectorLayerUndoCommandChangeGeometry( this, fid, geom ) );
   return true;
 }
 
 
 bool QgsVectorLayerEditBuffer::changeAttributeValue( QgsFeatureId fid, int field, QVariant value )
 {
+  if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::ChangeAttributeValues ) )
+    return false;
+
+  if ( FID_IS_NEW( fid ) )
+  {
+    if (!mAddedFeatures.contains(fid))
+      return false;
+  }
+
+  if ( field < 0 || field >= L->pendingFields().count() ||
+       L->pendingFields().fieldOrigin(field) == QgsFields::OriginJoin )
+    return false;
+
   L->undoStack()->push( new QgsVectorLayerUndoCommandChangeAttribute( this, fid, field, value ) );
   return true;
 }
 
 
 bool QgsVectorLayerEditBuffer::addAttribute( const QgsField &field )
 {
+  if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::AddAttributes ) )
+    return false;
+
   if ( field.name().isEmpty() )
     return false;
 
@@ -168,6 +202,9 @@ bool QgsVectorLayerEditBuffer::addAttribute( const QgsField &field )
 
 bool QgsVectorLayerEditBuffer::deleteAttribute( int index )
 {
+  if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::DeleteAttributes ) )
+    return false;
+
   if ( index < 0 || index >= L->pendingFields().count() )
     return false;
 

diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py
@@ -187,6 +187,9 @@ def checkBefore():
         assert layer.deleteFeature(fid)
 
         checkAfter()
+
+        # make sure calling it twice does not work
+        assert not layer.deleteFeature(fid)
 
         # now try undo/redo
         layer.undoStack().undo()
@@ -950,12 +953,42 @@ def test_join(self):
         assert f2[3].toInt()[0] == 321
 
 
+    def test_InvalidOperations(self):
+        layer = createLayerWithOnePoint()
+
+        layer.startEditing()
+
+        # ADD FEATURE
+
+        newF1 = QgsFeature()
+        assert not layer.addFeature(newF1)  # need attributes like the layer has
+
+        # DELETE FEATURE
+
+        assert not layer.deleteFeature(-333)
+        # we do not check for existence of the feature id if it's not newly added feature
+        #assert not layer.deleteFeature(333)
+
+        # CHANGE GEOMETRY
+
+        assert not layer.changeGeometry(-333, QgsGeometry.fromPoint(QgsPoint(1,1)))
+
+        # CHANGE VALUE
+
+        assert not layer.changeAttributeValue(-333, 0, QVariant(1))
+        assert not layer.changeAttributeValue(1, -1, QVariant(1))
+
+        # ADD ATTRIBUTE
+
+        assert not layer.addAttribute( QgsField() )
+
+        # DELETE ATTRIBUTE
+
+        assert not layer.deleteAttribute(-1)
 
 
 # TODO:
-# - invalid API usage (wrong FID, field index, ...)
 # - fetch rect: feat with changed geometry: 1. in rect, 2. out of rect
-# - rollback: each case with two variants: 1. commit, 2. rollback
 # - more join tests
 # - import