Merge pull request #43854 from gacarrillor/addtopologicalpoints_results

Fix return value of addTopologicalPoints(QgsGeometry)
qgis · Jul 8, 2021 · f083876 · f083876
2 parents f3da999 + 92231d7
commit f083876
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 11 deletions.
diff --git a/python/core/auto_generated/vector/qgsvectorlayer.sip.in b/python/core/auto_generated/vector/qgsvectorlayer.sip.in
@@ -1630,8 +1630,14 @@ Adds topological points for every vertex of the geometry.
 
 :param geom: the geometry where each vertex is added to segments of other features
 
+:return: -1 in case of layer error (invalid or non editable)
+
 :return: 0 in case of success
 
+:return: 1 in case of geometry error (non spatial or null geometry)
+
+:return: 2 in case no vertices needed to be added
+
 .. note::
 
    geom is not going to be modified by the function
@@ -1653,8 +1659,14 @@ editing.
 
 :param p: position of the vertex
 
+:return: -1 in case of layer error (invalid or non editable)
+
 :return: 0 in case of success
 
+:return: 1 in case of geometry error (non spatial or null geometry)
+
+:return: 2 in case no vertices needed to be added
+
 .. note::
 
    Calls to :py:func:`~QgsVectorLayer.addTopologicalPoints` are only valid for layers in which edits have been enabled
@@ -1675,8 +1687,14 @@ editing.
 
 :param p: position of the vertex
 
+:return: -1 in case of layer error (invalid or non editable)
+
 :return: 0 in case of success
 
+:return: 1 in case of geometry error (non spatial or null geometry)
+
+:return: 2 in case no vertices needed to be added
+
 .. note::
 
    Calls to :py:func:`~QgsVectorLayer.addTopologicalPoints` are only valid for layers in which edits have been enabled
@@ -1696,8 +1714,14 @@ editing.
 
 :param ps: point sequence of the vertices
 
+:return: -1 in case of layer error (invalid or non editable)
+
 :return: 0 in case of success
 
+:return: 1 in case of geometry error (non spatial or null geometry)
+
+:return: 2 in case no vertices needed to be added
+
 .. note::
 
    Calls to :py:func:`~QgsVectorLayer.addTopologicalPoints` are only valid for layers in which edits have been enabled

diff --git a/src/core/vector/qgsvectorlayer.h b/src/core/vector/qgsvectorlayer.h
@@ -1580,7 +1580,10 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
     /**
      * Adds topological points for every vertex of the geometry.
      * \param geom the geometry where each vertex is added to segments of other features
+     * \returns -1 in case of layer error (invalid or non editable)
      * \returns 0 in case of success
+     * \returns 1 in case of geometry error (non spatial or null geometry)
+     * \returns 2 in case no vertices needed to be added
      * \note geom is not going to be modified by the function
      * \note Calls to addTopologicalPoints() are only valid for layers in which edits have been enabled
      * by a call to startEditing(). Changes made to features using this method are not committed
@@ -1595,7 +1598,10 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
      * no additional vertex is inserted. This method is useful for topological
      * editing.
      * \param p position of the vertex
+     * \returns -1 in case of layer error (invalid or non editable)
      * \returns 0 in case of success
+     * \returns 1 in case of geometry error (non spatial or null geometry)
+     * \returns 2 in case no vertices needed to be added
      * \note Calls to addTopologicalPoints() are only valid for layers in which edits have been enabled
      * by a call to startEditing(). Changes made to features using this method are not committed
      * to the underlying data provider until a commitChanges() call is made. Any uncommitted
@@ -1610,7 +1616,10 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
      * no additional vertex is inserted. This method is useful for topological
      * editing.
      * \param p position of the vertex
+     * \returns -1 in case of layer error (invalid or non editable)
      * \returns 0 in case of success
+     * \returns 1 in case of geometry error (non spatial or null geometry)
+     * \returns 2 in case no vertices needed to be added
      * \note Calls to addTopologicalPoints() are only valid for layers in which edits have been enabled
      * by a call to startEditing(). Changes made to features using this method are not committed
      * to the underlying data provider until a commitChanges() call is made. Any uncommitted
@@ -1625,7 +1634,10 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
      * no additional vertex is inserted. This method is useful for topological
      * editing.
      * \param ps point sequence of the vertices
+     * \returns -1 in case of layer error (invalid or non editable)
      * \returns 0 in case of success
+     * \returns 1 in case of geometry error (non spatial or null geometry)
+     * \returns 2 in case no vertices needed to be added
      * \note Calls to addTopologicalPoints() are only valid for layers in which edits have been enabled
      * by a call to startEditing(). Changes made to features using this method are not committed
      * to the underlying data provider until a commitChanges() call is made. Any uncommitted

diff --git a/src/core/vector/qgsvectorlayereditutils.cpp b/src/core/vector/qgsvectorlayereditutils.cpp
@@ -543,19 +543,19 @@ int QgsVectorLayerEditUtils::addTopologicalPoints( const QgsGeometry &geom )
     return 1;
   }
 
-  int returnVal = 0;
+  bool pointsAdded = false;
 
   QgsAbstractGeometry::vertex_iterator it = geom.vertices_begin();
   while ( it != geom.vertices_end() )
   {
-    if ( addTopologicalPoints( *it ) != 0 )
+    if ( addTopologicalPoints( *it ) == 0 )
     {
-      returnVal = 2;
+      pointsAdded = true;
     }
     ++it;
   }
 
-  return returnVal;
+  return pointsAdded ? 0 : 2;
 }
 
 int QgsVectorLayerEditUtils::addTopologicalPoints( const QgsPoint &p )

diff --git a/tests/src/core/testqgsvectorlayer.cpp b/tests/src/core/testqgsvectorlayer.cpp
@@ -334,7 +334,7 @@ void TestQgsVectorLayer::testAddTopologicalPoints()
   QVERIFY( layerLine->isValid() );
 
   QgsPolylineXY line1;
-  line1 << QgsPointXY( 2, 1 ) << QgsPointXY( 1, 1 ) << QgsPointXY( 1, 3 );
+  line1 << QgsPointXY( 2, 1 ) << QgsPointXY( 1, 1 ) << QgsPointXY( 1, 5 );
   QgsFeature lineF1;
   lineF1.setGeometry( QgsGeometry::fromPolylineXY( line1 ) );
 
@@ -346,24 +346,71 @@ void TestQgsVectorLayer::testAddTopologicalPoints()
   QCOMPARE( layerLine->undoStack()->index(), 1 );
 
   // outside of the linestring - nothing should happen
-  layerLine->addTopologicalPoints( QgsPoint( 2, 2 ) );
+  int result = layerLine->addTopologicalPoints( QgsPoint( 2, 2 ) );
+  QCOMPARE( result, 2 );
 
   QCOMPARE( layerLine->undoStack()->index(), 1 );
-  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ).asWkt() );
+  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 5)" ).asWkt() );
 
   // add point at an existing vertex
-  layerLine->addTopologicalPoints( QgsPoint( 1, 1 ) );
+  result = layerLine->addTopologicalPoints( QgsPoint( 1, 1 ) );
+  QCOMPARE( result, 2 );
 
   QCOMPARE( layerLine->undoStack()->index(), 1 );
-  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ).asWkt() );
+  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 5)" ).asWkt() );
 
   // add point on segment of linestring
-  layerLine->addTopologicalPoints( QgsPoint( 1, 2 ) );
+  result = layerLine->addTopologicalPoints( QgsPoint( 1, 2 ) );
+  QCOMPARE( result, 0 );
 
   QCOMPARE( layerLine->undoStack()->index(), 2 );
-  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 2, 1 3)" ).asWkt() );
+  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 2, 1 5)" ).asWkt() );
+
+  // add points from disjoint geometry - nothing should happen
+  result = layerLine->addTopologicalPoints( QgsGeometry::fromWkt( "LINESTRING(2 0, 2 1, 2 2)" ) );
+  QCOMPARE( result, 2 );
+
+  QCOMPARE( layerLine->undoStack()->index(), 2 );
+  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 2, 1 5)" ).asWkt() );
+
+  // add 2 out of 3 points from intersecting geometry
+  result = layerLine->addTopologicalPoints( QgsGeometry::fromWkt( "LINESTRING(2 0, 2 1, 2 3, 1 3, 0 3, 0 4, 1 4)" ) );
+  QCOMPARE( result, 0 );
+
+  QCOMPARE( layerLine->undoStack()->index(), 2 );
+  QCOMPARE( layerLine->getFeature( fidLineF1 ).geometry().asWkt(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 2, 1 3, 1 4, 1 5)" ).asWkt() );
 
   delete layerLine;
+
+  // Test error results -1: layer error, 1: geometry error
+  QgsVectorLayer *nonSpatialLayer = new QgsVectorLayer( QStringLiteral( "None" ), QStringLiteral( "non spatial layer" ), QStringLiteral( "memory" ) );
+  QVERIFY( nonSpatialLayer->isValid() );
+
+  result = nonSpatialLayer->addTopologicalPoints( QgsPoint( 2, 2 ) );
+  QCOMPARE( result, -1 );  // Non editable
+
+  nonSpatialLayer->startEditing();
+  result = nonSpatialLayer->addTopologicalPoints( QgsPoint( 2, 2 ) );
+  QCOMPARE( result, 1 );  // Non spatial
+
+  delete nonSpatialLayer;
+
+  QgsVectorLayer *layerPoint = new QgsVectorLayer( QStringLiteral( "Point?crs=EPSG:27700" ), QStringLiteral( "layer point" ), QStringLiteral( "memory" ) );
+  QVERIFY( layerPoint->isValid() );
+
+  layerPoint->startEditing();
+  result = layerPoint->addTopologicalPoints( QgsGeometry() );
+  QCOMPARE( result, 1 );  // Null geometry
+
+  delete layerPoint;
+
+  QgsVectorLayer *layerInvalid = new QgsVectorLayer( QStringLiteral(), QStringLiteral( "layer invalid" ), QStringLiteral( "none" ) );
+  QVERIFY( !layerInvalid->isValid() );
+
+  result = layerInvalid->addTopologicalPoints( QgsPoint( 2, 2 ) );
+  QCOMPARE( result, -1 );  // Invalid layer
+
+  delete layerInvalid;
 }
 
 void TestQgsVectorLayer::testCopyPasteFieldConfiguration_data()