QgsVectorLayer is a QgsFeatureSink

Also cleanup API for addFeature(s) in QgsVectorLayer, by removing the unused extra argument from addFeature() and be removing the makeSelected argument from addFeatures() (code should be adapted to manually select added features after adding if desired - this was only used in a single place in the QGIS code and I suspect this was unintentional in any case)
qgis · Apr 26, 2017 · c4578c3 · c4578c3
1 parent 8d59833
commit c4578c3
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 40 deletions.
diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -2113,6 +2113,8 @@ displayExpression instead. For the map tip use mapTipTemplate() instead.
 - snapPoint() has been removed - use QgsPointLocator class instead.
 - snapWithContext() has been removed - use QgsPointLocator class instead.
 - insertSegmentVerticesForSnap() has been removed - use addTopologicalPoints() directly.
+- addFeature() no longer accepts an alsoUpdateExtent boolean - this extra argument has been ignored for some time
+- addFeatures() no longer accepts a makeSelected boolean, and will not automatically select newly added features. If desired, features must be manually selected by calling selectByIds() after addFeatures()
 
 
 QgsVectorLayerEditBuffer        {#qgis_api_break_3_0_QgsVectorLayerEditBuffer}

diff --git a/python/core/qgsvectorlayer.sip b/python/core/qgsvectorlayer.sip
@@ -19,7 +19,7 @@ typedef QList<QgsPointV2> QgsPointSequence;
 
 
 
-class QgsVectorLayer : QgsMapLayer, QgsExpressionContextGenerator
+class QgsVectorLayer : QgsMapLayer, QgsExpressionContextGenerator, QgsFeatureSink
 {
 %Docstring
  Represents a vector layer which manages a vector based data sets.
@@ -901,12 +901,16 @@ Return the provider type for this layer
  :rtype: QgsFeatureIterator
 %End
 
-    bool addFeature( QgsFeature &feature, bool alsoUpdateExtent = true );
+    virtual bool addFeature( QgsFeature &feature /In,Out/ );
+
 %Docstring
- Adds a feature
-\param feature feature to add
-\param alsoUpdateExtent If True, will also go to the effort of e.g. updating the extents.
-:return:                    True in case of success and False in case of error
+ Adds a single ``feature`` to the layer.
+ Calling this method causes the layer to recalculate it's extents, which can be
+ expensive. If multiple features are to be added to the layer then it is more
+ efficient to call addFeatures(), as addFeatures() will only trigger a single
+ layer extent recalculation.
+ \see addFeatures()
+ :return: true in case of success and false in case of failure
  :rtype: bool
 %End
 
@@ -1256,11 +1260,8 @@ Delete an attribute field (but does not commit it)
  :rtype: bool
 %End
 
-    bool addFeatures( QgsFeatureList features, bool makeSelected = true );
-%Docstring
-Insert a copy of the given features into the layer  (but does not commit it)
- :rtype: bool
-%End
+    virtual bool addFeatures( QgsFeatureList &features /In,Out/ );
+
 
     bool deleteFeature( QgsFeatureId fid );
 %Docstring

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -7632,7 +7632,7 @@ void QgisApp::mergeSelectedFeatures()
     vl->deleteFeature( *feature_it );
   }
 
-  vl->addFeature( newFeature, false );
+  vl->addFeature( newFeature );
 
   vl->endEditCommand();
 
@@ -7934,7 +7934,7 @@ void QgisApp::editPaste( QgsMapLayer *destinationLayer )
     // now create new feature using pasted feature as a template. This automatically handles default
     // values and field constraints
     QgsFeature newFeature = QgsVectorLayerUtils::createFeature( pasteVectorLayer, geom, dstAttr, &context );
-    pasteVectorLayer->addFeature( newFeature, false );
+    pasteVectorLayer->addFeature( newFeature );
     newIds << newFeature.id();
 
     ++featureIt;
@@ -8131,7 +8131,7 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()
       feature.setGeometry( g );
     }
   }
-  if ( ! layer->addFeatures( features, false ) || !layer->commitChanges() )
+  if ( ! layer->addFeatures( features ) || !layer->commitChanges() )
   {
     QgsDebugMsg( "Cannot add features or commit changes" );
     delete layer;

diff --git a/src/core/qgsofflineediting.cpp b/src/core/qgsofflineediting.cpp
@@ -661,7 +661,7 @@ QgsVectorLayer *QgsOfflineEditing::copyVectorLayer( QgsVectorLayer *layer, sqlit
         }
         f.setAttributes( newAttrs );
 
-        newLayer->addFeature( f, false );
+        newLayer->addFeature( f );
 
         emit progressUpdated( featureCount++ );
       }
@@ -786,7 +786,7 @@ void QgsOfflineEditing::applyFeaturesAdded( QgsVectorLayer *offlineLayer, QgsVec
 
     // respect constraints and provider default values
     QgsFeature f = QgsVectorLayerUtils::createFeature( remoteLayer, it->geometry(), newAttrs.toMap(), &context );
-    remoteLayer->addFeature( f, false );
+    remoteLayer->addFeature( f );
 
     emit progressUpdated( i++ );
   }

diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -955,9 +955,8 @@ QgsFeatureIterator QgsVectorLayer::getFeatures( const QgsFeatureRequest &request
   return QgsFeatureIterator( new QgsVectorLayerFeatureIterator( new QgsVectorLayerFeatureSource( this ), true, request ) );
 }
 
-bool QgsVectorLayer::addFeature( QgsFeature &feature, bool alsoUpdateExtent )
+bool QgsVectorLayer::addFeature( QgsFeature &feature )
 {
-  Q_UNUSED( alsoUpdateExtent ); // TODO[MD]
   if ( !mValid || !mEditBuffer || !mDataProvider )
     return false;
 
@@ -2559,23 +2558,12 @@ QgsFeatureIterator QgsVectorLayer::selectedFeaturesIterator( QgsFeatureRequest r
   return getFeatures( request );
 }
 
-bool QgsVectorLayer::addFeatures( QgsFeatureList features, bool makeSelected )
+bool QgsVectorLayer::addFeatures( QgsFeatureList &features )
 {
   if ( !mEditBuffer || !mDataProvider )
     return false;
 
   bool res = mEditBuffer->addFeatures( features );
-
-  if ( makeSelected )
-  {
-    QgsFeatureIds ids;
-
-    for ( QgsFeatureList::iterator iter = features.begin(); iter != features.end(); ++iter )
-      ids << iter->id();
-
-    selectByIds( ids );
-  }
-
   updateExtents();
 
   return res;

diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h
@@ -348,7 +348,7 @@ typedef QList<QgsPointV2> QgsPointSequence;
  * TODO QGIS3: Remove virtual from non-inherited methods (like isModified)
  * \see QgsVectorLayerUtils()
  */
-class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionContextGenerator
+class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionContextGenerator, public QgsFeatureSink
 {
     Q_OBJECT
 
@@ -872,12 +872,16 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
       return getFeatures( QgsFeatureRequest( rectangle ) );
     }
 
-    /** Adds a feature
-        \param feature feature to add
-        \param alsoUpdateExtent If True, will also go to the effort of e.g. updating the extents.
-        \returns                    True in case of success and False in case of error
+    /**
+     * Adds a single \a feature to the layer.
+     * Calling this method causes the layer to recalculate it's extents, which can be
+     * expensive. If multiple features are to be added to the layer then it is more
+     * efficient to call addFeatures(), as addFeatures() will only trigger a single
+     * layer extent recalculation.
+     * \see addFeatures()
+     * \returns true in case of success and false in case of failure
      */
-    bool addFeature( QgsFeature &feature, bool alsoUpdateExtent = true );
+    bool addFeature( QgsFeature &feature SIP_INOUT ) override;
 
     /** Updates an existing feature. This method needs to query the datasource
         on every call. Consider using changeAttributeValue() or
@@ -1201,8 +1205,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
      */
     bool deleteAttributes( QList<int> attrs );
 
-    //! Insert a copy of the given features into the layer  (but does not commit it)
-    bool addFeatures( QgsFeatureList features, bool makeSelected = true );
+    bool addFeatures( QgsFeatureList &features SIP_INOUT ) override;
 
     //! Delete a feature from the layer (but does not commit it)
     bool deleteFeature( QgsFeatureId fid );

diff --git a/src/core/qgsvectorlayertools.cpp b/src/core/qgsvectorlayertools.cpp
@@ -60,7 +60,7 @@ bool QgsVectorLayerTools::copyMoveFeatures( QgsVectorLayer *layer, QgsFeatureReq
       const QgsFeatureId fid = f.id();
 #endif
       // paste feature
-      if ( !layer->addFeature( f, false ) )
+      if ( !layer->addFeature( f ) )
       {
         couldNotWriteCount++;
         QgsDebugMsg( QString( "Could not add new feature. Original copied feature id: %1" ).arg( fid ) );

diff --git a/src/gui/qgsrelationeditorwidget.cpp b/src/gui/qgsrelationeditorwidget.cpp
@@ -371,6 +371,11 @@ void QgsRelationEditorWidget::linkFeature()
       }
 
       mRelation.referencingLayer()->addFeatures( newFeatures );
+      QgsFeatureIds ids;
+      Q_FOREACH ( const QgsFeature &f, newFeatures )
+        ids << f.id();
+      mRelation.referencingLayer()->selectByIds( ids );
+
 
       updateUi();
     }

diff --git a/tests/src/core/testqgstracer.cpp b/tests/src/core/testqgstracer.cpp
@@ -68,7 +68,7 @@ static QgsVectorLayer *make_layer( const QStringList &wkts )
   Q_FOREACH ( const QString &wkt, wkts )
   {
     QgsFeature f( make_feature( wkt ) );
-    vl->addFeature( f, false );
+    vl->addFeature( f );
   }
   vl->commitChanges();