[Geometry checker] Handle fid changes

src/plugins/geometry_checker/CMakeLists.txt

@@ -39,7 +39,6 @@ SET (geometrychecker_HDRS
     checks/qgsgeometrycheck.h
     qgsgeometrycheckerplugin.h
     qgsgeometrycheckfactory.h
-    utils/qgsfeaturepool.h
     utils/qgsgeometrycheckerutils.h
 )
 
@@ -71,6 +70,7 @@ SET (geometrychecker_MOC_HDRS
     ui/qgsgeometrycheckerresulttab.h
     ui/qgsgeometrycheckfixdialog.h
     ui/qgsgeometrycheckerfixsummarydialog.h
+    utils/qgsfeaturepool.h
 )
 
 SET (geometrychecker_UIS

src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp

@@ -143,6 +143,17 @@ bool QgsGeometryCheckError::handleChanges( const QgsGeometryCheck::Changes &chan
   return true;
 }
 
+bool QgsGeometryCheckError::handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap )
+{
+  if ( layerId == mLayerId )
+  {
+    QgsFeatureId oldId = mFeatureId;
+    mFeatureId = oldNewFidMap.value( mFeatureId, mFeatureId );
+    return oldId != mFeatureId;
+  }
+  return false;
+}
+
 QMap<QString, QgsFeatureIds> QgsGeometryCheck::allLayerFeatureIds() const
 {
   QMap<QString, QgsFeatureIds> featureIds;

src/plugins/geometry_checker/checks/qgsgeometrycheck.h

@@ -159,6 +159,7 @@ class QgsGeometryCheckError
     }
 
     virtual bool handleChanges( const QgsGeometryCheck::Changes &changes );
+    virtual bool handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap );
 
   protected:
     // Users of this constructor must ensure geometry and errorLocation are in map coordinates

src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.cpp

@@ -17,6 +17,19 @@
 #include "qgsgeometrycontainedcheck.h"
 #include "../utils/qgsfeaturepool.h"
 
+
+bool QgsGeometryContainedCheckError::handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap )
+{
+  bool changed = QgsGeometryCheckError::handleFidChanges( layerId, oldNewFidMap );
+  if ( mContainingFeature.first == layerId )
+  {
+    QgsFeatureId oldId = mContainingFeature.second;
+    mContainingFeature.second = oldNewFidMap.value( mContainingFeature.second, mContainingFeature.second );
+    changed |= ( oldId != mContainingFeature.second );
+  }
+  return changed;
+}
+
 void QgsGeometryContainedCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &messages, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;

src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.h

@@ -39,6 +39,7 @@ class QgsGeometryContainedCheckError : public QgsGeometryCheckError
     }
 
     virtual QString description() const override { return QApplication::translate( "QgsGeometryContainedCheckError", "Within feature" ); }
+    bool handleFidChanges( const QString& layerId, const QMap<QgsFeatureId, QgsFeatureId>& oldNewFidMap) override;
 
   private:
     QPair<QString, QgsFeatureId> mContainingFeature;

src/plugins/geometry_checker/checks/qgsgeometryduplicatecheck.cpp

@@ -19,6 +19,24 @@
 #include "qgsgeometry.h"
 #include "../utils/qgsfeaturepool.h"
 
+
+bool QgsGeometryDuplicateCheckError::handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap )
+{
+  bool changed = QgsGeometryCheckError::handleFidChanges( layerId, oldNewFidMap );
+  if ( mDuplicates.contains( layerId ) )
+  {
+    QList<QgsFeatureId> &fids = mDuplicates[layerId];
+    for ( int i = 0, n = fids.size(); i < n; ++i )
+    {
+      QgsFeatureId oldId = fids[i];
+      fids[i] = oldNewFidMap.value( fids[i], fids[i] );
+      changed |= ( oldId != fids[i] );
+    }
+  }
+  return changed;
+}
+
+
 void QgsGeometryDuplicateCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &messages, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;

src/plugins/geometry_checker/checks/qgsgeometryduplicatecheck.h

@@ -38,6 +38,7 @@ class QgsGeometryDuplicateCheckError : public QgsGeometryCheckError
              // static_cast: since other->checker() == checker is only true if the types are actually the same
              static_cast<QgsGeometryDuplicateCheckError *>( other )->duplicates() == duplicates();
     }
+    bool handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap ) override;
 
   private:
     QMap<QString, QList<QgsFeatureId>> mDuplicates;

src/plugins/geometry_checker/checks/qgsgeometrygapcheck.cpp

@@ -18,6 +18,29 @@
 #include "qgsgeometrycollection.h"
 #include "../utils/qgsfeaturepool.h"
 
+bool QgsGeometryGapCheckError::handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap )
+{
+  bool changed = false;
+  if ( mNeighbors.contains( layerId ) )
+  {
+    QgsFeatureIds &fids = mNeighbors[layerId];
+    QgsFeatureIds newIds;
+    for ( auto it = fids.begin(), itEnd = fids.end(); it != itEnd; ++it )
+    {
+      const QgsFeatureId &fid = *it;
+      QgsFeatureId newId = oldNewFidMap.value( fid, fid );
+      if ( fid != newId )
+      {
+        it = fids.erase( it );
+        newIds.insert( newId );
+        changed = true;
+      }
+    }
+    fids.unite( newIds );
+  }
+  return changed;
+}
+
 
 void QgsGeometryGapCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &messages, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {

src/plugins/geometry_checker/checks/qgsgeometrygapcheck.h

@@ -66,6 +66,8 @@ class QgsGeometryGapCheckError : public QgsGeometryCheckError
       return mGapAreaBBox;
     }
 
+    bool handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap ) override;
+
   private:
     QMap<QString, QgsFeatureIds> mNeighbors;
     QgsRectangle mGapAreaBBox;

src/plugins/geometry_checker/checks/qgsgeometryoverlapcheck.cpp

@@ -17,6 +17,19 @@
 #include "qgsgeometryoverlapcheck.h"
 #include "../utils/qgsfeaturepool.h"
 
+
+bool QgsGeometryOverlapCheckError::handleFidChanges( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFidMap )
+{
+  bool changed = QgsGeometryCheckError::handleFidChanges( layerId, oldNewFidMap );
+  if ( mOverlappedFeature.first == layerId )
+  {
+    QgsFeatureId oldId = mOverlappedFeature.second;
+    mOverlappedFeature.second = oldNewFidMap.value( mOverlappedFeature.second, mOverlappedFeature.second );
+    changed |= ( oldId != mOverlappedFeature.second );
+  }
+  return changed;
+}
+
 void QgsGeometryOverlapCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &messages, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   double overlapThreshold = mThresholdMapUnits;

src/plugins/geometry_checker/checks/qgsgeometryoverlapcheck.h

@@ -51,6 +51,7 @@ class QgsGeometryOverlapCheckError : public QgsGeometryCheckError
     }
 
     virtual QString description() const override { return QApplication::translate( "QgsGeometryTypeCheckError", "Overlap with %1:%2" ).arg( mOverlappedFeature.first ).arg( mOverlappedFeature.second ); }
+    bool handleFidChanges( const QString& layerId, const QMap<QgsFeatureId, QgsFeatureId>& oldNewFidMap) override;
 
   private:
     QPair<QString, QgsFeatureId> mOverlappedFeature;

src/plugins/geometry_checker/qgsgeometrychecker.cpp

@@ -29,6 +29,10 @@ QgsGeometryChecker::QgsGeometryChecker( const QList<QgsGeometryCheck *> &checks,
   : mChecks( checks )
   , mContext( context )
 {
+  for ( const QString &layerId : mContext->featurePools.keys() )
+  {
+    connect( mContext->featurePools[layerId], SIGNAL( featureIdsChanged( QString, QMap<QgsFeatureId, QgsFeatureId> ) ), this, SLOT( updateFeatureIds( QString, QMap<QgsFeatureId, QgsFeatureId> ) ) );
+  }
 }
 
 QgsGeometryChecker::~QgsGeometryChecker()
@@ -283,3 +287,14 @@ void QgsGeometryChecker::runCheck( const QgsGeometryCheck *check )
     emit errorAdded( error );
   }
 }
+
+void QgsGeometryChecker::updateFeatureIds( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFid )
+{
+  for ( QgsGeometryCheckError *error : mCheckErrors )
+  {
+    if ( error->handleFidChanges( layerId, oldNewFid ) )
+    {
+      emit errorUpdated( error, false );
+    }
+  }
+}

src/plugins/geometry_checker/qgsgeometrychecker.h

@@ -71,6 +71,7 @@ class QgsGeometryChecker : public QObject
 
   private slots:
     void emitProgressValue();
+    void updateFeatureIds( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFid );
 };
 
 #endif // QGS_GEOMETRY_CHECKER_H

src/plugins/geometry_checker/ui/qgsgeometrycheckerresulttab.cpp

@@ -173,7 +173,9 @@ void QgsGeometryCheckerResultTab::updateError( QgsGeometryCheckError *error, boo
   }
   // Disable sorting to prevent crashes: if i.e. sorting by col 0, as soon as the item(row, 0) is set,
   // the row is potentially moved due to sorting, and subsequent item(row, col) reference wrong item
-  ui.tableWidgetErrors->setSortingEnabled( false );
+  bool sortingWasEnabled = ui.tableWidgetErrors->isSortingEnabled();
+  if ( sortingWasEnabled )
+    ui.tableWidgetErrors->setSortingEnabled( false );
 
   int row = mErrorMap.value( error ).row();
   int prec = 7 - std::floor( std::max( 0., std::log10( std::max( error->location().x(), error->location().y() ) ) ) );
@@ -211,7 +213,8 @@ void QgsGeometryCheckerResultTab::updateError( QgsGeometryCheckError *error, boo
   }
   ui.labelErrorCount->setText( tr( "Total errors: %1, fixed errors: %2" ).arg( mErrorCount ).arg( mFixedCount ) );
 
-  ui.tableWidgetErrors->setSortingEnabled( true );
+  if ( sortingWasEnabled )
+    ui.tableWidgetErrors->setSortingEnabled( true );
 }
 
 void QgsGeometryCheckerResultTab::exportErrors()
@@ -475,6 +478,9 @@ void QgsGeometryCheckerResultTab::fixErrors( bool prompt )
     return;
   }
 
+  // Disable sorting while fixing errors
+  ui.tableWidgetErrors->setSortingEnabled( false );
+
   //! Reset statistics, clear rubberbands *
   mStatistics = QgsGeometryCheckerFixSummaryDialog::Statistics();
   qDeleteAll( mCurrentRubberBands );
@@ -526,6 +532,7 @@ void QgsGeometryCheckerResultTab::fixErrors( bool prompt )
     parentWidget()->parentWidget()->parentWidget()->setEnabled( true );
   }
   mCloseable = true;
+  ui.tableWidgetErrors->setSortingEnabled( true );
 }
 
 void QgsGeometryCheckerResultTab::setRowStatus( int row, const QColor &color, const QString &message, bool selectable )

src/plugins/geometry_checker/utils/qgsfeaturepool.cpp

@@ -57,6 +57,7 @@ QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer, double layerToMapUnits, c
       mFeatureIds.remove( feature.id() );
     }
   }
+  connect( mLayer->dataProvider(), SIGNAL( featureIdsChanged( QMap<QgsFeatureId, QgsFeatureId> ) ), this, SLOT( updateFeatureIds( QMap<QgsFeatureId, QgsFeatureId> ) ) );
 }
 
 bool QgsFeaturePool::get( QgsFeatureId id, QgsFeature &feature )
@@ -141,3 +142,31 @@ QgsFeatureIds QgsFeaturePool::getIntersects( const QgsRectangle &rect ) const
   QMutexLocker lock( &mIndexMutex );
   return QgsFeatureIds::fromList( mIndex.intersects( rect ) );
 }
+
+void QgsFeaturePool::updateFeatureIds( const QMap<QgsFeatureId, QgsFeatureId> &oldNewFid )
+{
+  QList<QgsFeature *> changedFeatures;
+  QgsFeatureIds newIds;
+  for ( const QgsFeatureId &oldId : oldNewFid.keys() )
+  {
+    auto it = mFeatureIds.find( oldId );
+    if ( it != mFeatureIds.end() )
+    {
+      mFeatureIds.erase( it );
+    }
+    newIds.insert( oldNewFid[oldId] );
+    QgsFeature *feature = mFeatureCache.take( oldId );
+    if ( feature )
+    {
+      feature->setId( oldNewFid[oldId] );
+      changedFeatures.append( feature );
+    }
+  }
+  for ( QgsFeature *feature : changedFeatures )
+  {
+    Q_ASSERT( !mFeatureCache.contains( feature->id() ) );
+    mFeatureCache.insert( feature->id(), feature );
+  }
+  mFeatureIds.unite( newIds );
+  emit featureIdsChanged( mLayer->id(), oldNewFid );
+}

src/plugins/geometry_checker/utils/qgsfeaturepool.h

@@ -27,8 +27,9 @@
 
 class QgsVectorLayer;
 
-class QgsFeaturePool
+class QgsFeaturePool : public QObject
 {
+    Q_OBJECT
   public:
     QgsFeaturePool( QgsVectorLayer *layer, double layerToMapUnits, const QgsCoordinateTransform &layerToMapTransform, bool selectedOnly = false );
     bool get( QgsFeatureId id, QgsFeature &feature );
@@ -43,6 +44,8 @@ class QgsFeaturePool
     bool getSelectedOnly() const { return mSelectedOnly; }
     void clearLayer() { mLayer = nullptr; }
 
+  signals:
+    void featureIdsChanged( const QString &layerId, const QMap<QgsFeatureId, QgsFeatureId> &oldNewFid );
   private:
     struct MapEntry
     {
@@ -67,6 +70,9 @@ class QgsFeaturePool
     bool mSelectedOnly;
 
     bool getTouchingWithSharedEdge( QgsFeature &feature, QgsFeatureId &touchingId, const double & ( *comparator )( const double &, const double & ), double init );
+
+  private slots:
+    void updateFeatureIds( const QMap<QgsFeatureId, QgsFeatureId> &oldNewFid );
 };
 
 #endif // QGS_FEATUREPOOL_H