[Geometry checker] Fix features not correctly removed from QgsFeaturePool spatial index

manisandro · manisandro · commit efc00c38a1a3 · 2017-10-23T17:25:08.000+02:00
diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp b/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp
@@ -105,6 +105,9 @@ void QgsFeaturePool::addFeature( QgsFeature &feature )
 
 void QgsFeaturePool::updateFeature( QgsFeature &feature )
 {
+  QgsFeature origFeature;
+  get( feature.id(), origFeature );
+
   QgsGeometryMap geometryMap;
   geometryMap.insert( feature.id(), QgsGeometry( feature.geometry().geometry()->clone() ) );
   QgsChangedAttributesMap changedAttributesMap;
@@ -120,19 +123,23 @@ void QgsFeaturePool::updateFeature( QgsFeature &feature )
   mLayer->dataProvider()->changeAttributeValues( changedAttributesMap );
   mLayerMutex.unlock();
   mIndexMutex.lock();
-  mIndex.deleteFeature( feature );
+  mIndex.deleteFeature( origFeature );
   mIndex.insertFeature( feature );
   mIndexMutex.unlock();
 }
 
-void QgsFeaturePool::deleteFeature( QgsFeature &feature )
+void QgsFeaturePool::deleteFeature( const QgsFeatureId &fid )
 {
-  mIndexMutex.lock();
-  mIndex.deleteFeature( feature );
-  mIndexMutex.unlock();
+  QgsFeature origFeature;
+  if ( get( fid, origFeature ) )
+  {
+    mIndexMutex.lock();
+    mIndex.deleteFeature( origFeature );
+    mIndexMutex.unlock();
+  }
   mLayerMutex.lock();
-  mFeatureCache.remove( feature.id() );
-  mLayer->dataProvider()->deleteFeatures( QgsFeatureIds() << feature.id() );
+  mFeatureCache.remove( origFeature.id() );
+  mLayer->dataProvider()->deleteFeatures( QgsFeatureIds() << fid );
   mLayerMutex.unlock();
 }
 
diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.h b/src/analysis/vector/geometry_checker/qgsfeaturepool.h
@@ -36,7 +36,7 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QObject
     bool get( QgsFeatureId id, QgsFeature &feature );
     void addFeature( QgsFeature &feature );
     void updateFeature( QgsFeature &feature );
-    void deleteFeature( QgsFeature &feature );
+    void deleteFeature( const QgsFeatureId &fid );
     QgsFeatureIds getIntersects( const QgsRectangle &rect ) const;
     QgsVectorLayer *getLayer() const { return mLayer; }
     const QgsFeatureIds &getFeatureIds() const { return mFeatureIds; }
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrycheck.cpp
@@ -185,7 +185,7 @@ void QgsGeometryCheck::deleteFeatureGeometryPart( const QString &layerId, QgsFea
     static_cast<QgsGeometryCollection *>( geom )->removeGeometry( partIdx );
     if ( static_cast<QgsGeometryCollection *>( geom )->numGeometries() == 0 )
     {
-      featurePool->deleteFeature( feature );
+      featurePool->deleteFeature( feature.id() );
       changes[layerId][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );
     }
     else
@@ -197,7 +197,7 @@ void QgsGeometryCheck::deleteFeatureGeometryPart( const QString &layerId, QgsFea
   }
   else
   {
-    featurePool->deleteFeature( feature );
+    featurePool->deleteFeature( feature.id() );
     changes[layerId][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );
   }
 }
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp
@@ -94,7 +94,7 @@ void QgsGeometryContainedCheck::fixError( QgsGeometryCheckError *error, int meth
   else if ( method == Delete )
   {
     changes[error->layerId()][featureA.id()].append( Change( ChangeFeature, ChangeRemoved ) );
-    featurePoolA->deleteFeature( featureA );
+    featurePoolA->deleteFeature( featureA.id() );
     error->setFixed( method );
   }
   else
diff --git a/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp
@@ -117,7 +117,7 @@ void QgsGeometryDuplicateCheck::fixError( QgsGeometryCheckError *error, int meth
         QgsAbstractGeometry *diffGeom = geomEngineA->symDifference( layerFeatureB.geometry() );
         if ( diffGeom && diffGeom->isEmpty() )
         {
-          featurePoolB->deleteFeature( featureB );
+          featurePoolB->deleteFeature( featureB.id() );
           changes[layerIdB][idB].append( Change( ChangeFeature, ChangeRemoved ) );
         }
 
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrymultipartcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrymultipartcheck.cpp
@@ -64,7 +64,7 @@ void QgsGeometryMultipartCheck::fixError( QgsGeometryCheckError *error, int meth
   }
   else if ( method == RemoveObject )
   {
-    featurePool->deleteFeature( feature );
+    featurePool->deleteFeature( feature.id() );
     error->setFixed( method );
     changes[error->layerId()][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );
   }
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrytypecheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrytypecheck.cpp
@@ -132,14 +132,14 @@ void QgsGeometryTypeCheck::fixError( QgsGeometryCheckError *error, int method, c
     // Delete feature
     else
     {
-      featurePool->deleteFeature( feature );
+      featurePool->deleteFeature( feature.id() );
       changes[error->layerId()][error->featureId()].append( Change( ChangeFeature, ChangeRemoved ) );
     }
     error->setFixed( method );
   }
   else if ( method == Delete )
   {
-    featurePool->deleteFeature( feature );
+    featurePool->deleteFeature( feature.id() );
     error->setFixed( method );
     changes[error->layerId()][error->featureId()].append( Change( ChangeFeature, ChangeRemoved ) );
   }

Original file line number	Diff line number	Diff line change
`@@ -185,7 +185,7 @@ void QgsGeometryCheck::deleteFeatureGeometryPart( const QString &layerId, QgsFea`
`185`	`185`	`static_cast<QgsGeometryCollection *>( geom )->removeGeometry( partIdx );`
`186`	`186`	`if ( static_cast<QgsGeometryCollection *>( geom )->numGeometries() == 0 )`
`187`	`187`	`{`
`188`		`- featurePool->deleteFeature( feature );`
	`188`	`+ featurePool->deleteFeature( feature.id() );`
`189`	`189`	`changes[layerId][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );`
`190`	`190`	`}`
`191`	`191`	`else`
`@@ -197,7 +197,7 @@ void QgsGeometryCheck::deleteFeatureGeometryPart( const QString &layerId, QgsFea`
`197`	`197`	`}`
`198`	`198`	`else`
`199`	`199`	`{`
`200`		`- featurePool->deleteFeature( feature );`
	`200`	`+ featurePool->deleteFeature( feature.id() );`
`201`	`201`	`changes[layerId][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );`
`202`	`202`	`}`
`203`	`203`	`}`
Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,7 @@ void QgsGeometryContainedCheck::fixError( QgsGeometryCheckError *error, int meth`
`94`	`94`	`else if ( method == Delete )`
`95`	`95`	`{`
`96`	`96`	`changes[error->layerId()][featureA.id()].append( Change( ChangeFeature, ChangeRemoved ) );`
`97`		`- featurePoolA->deleteFeature( featureA );`
	`97`	`+ featurePoolA->deleteFeature( featureA.id() );`
`98`	`98`	`error->setFixed( method );`
`99`	`99`	`}`
`100`	`100`	`else`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,7 @@ void QgsGeometryDuplicateCheck::fixError( QgsGeometryCheckError *error, int meth`
`117`	`117`	`QgsAbstractGeometry *diffGeom = geomEngineA->symDifference( layerFeatureB.geometry() );`
`118`	`118`	`if ( diffGeom && diffGeom->isEmpty() )`
`119`	`119`	`{`
`120`		`- featurePoolB->deleteFeature( featureB );`
	`120`	`+ featurePoolB->deleteFeature( featureB.id() );`
`121`	`121`	`changes[layerIdB][idB].append( Change( ChangeFeature, ChangeRemoved ) );`
`122`	`122`	`}`
`123`	`123`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ void QgsGeometryMultipartCheck::fixError( QgsGeometryCheckError *error, int meth`
`64`	`64`	`}`
`65`	`65`	`else if ( method == RemoveObject )`
`66`	`66`	`{`
`67`		`- featurePool->deleteFeature( feature );`
	`67`	`+ featurePool->deleteFeature( feature.id() );`
`68`	`68`	`error->setFixed( method );`
`69`	`69`	`changes[error->layerId()][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );`
`70`	`70`	`}`
Original file line number	Diff line number	Diff line change
`@@ -132,14 +132,14 @@ void QgsGeometryTypeCheck::fixError( QgsGeometryCheckError *error, int method, c`
`132`	`132`	`// Delete feature`
`133`	`133`	`else`
`134`	`134`	`{`
`135`		`- featurePool->deleteFeature( feature );`
	`135`	`+ featurePool->deleteFeature( feature.id() );`
`136`	`136`	`changes[error->layerId()][error->featureId()].append( Change( ChangeFeature, ChangeRemoved ) );`
`137`	`137`	`}`
`138`	`138`	`error->setFixed( method );`
`139`	`139`	`}`
`140`	`140`	`else if ( method == Delete )`
`141`	`141`	`{`
`142`		`- featurePool->deleteFeature( feature );`
	`142`	`+ featurePool->deleteFeature( feature.id() );`
`143`	`143`	`error->setFixed( method );`
`144`	`144`	`changes[error->layerId()][error->featureId()].append( Change( ChangeFeature, ChangeRemoved ) );`
`145`	`145`	`}`