[GRASS] new feature attributes fixes

qgis · Sep 30, 2015 · c531b2d · nyalldawson · Oct 19, 2015 · blazek
1 parent d6b8b3c
commit c531b2d
Show file tree

Hide file tree

Showing 5 changed files with 219 additions and 16 deletions.
diff --git a/src/providers/grass/qgsgrassprovider.cpp b/src/providers/grass/qgsgrassprovider.cpp
@@ -1132,28 +1132,111 @@ void QgsGrassProvider::onFeatureAdded( QgsFeatureId fid )
     if ( FID_IS_NEW( fid ) )
     {
       // add new category
+      // TODO: add category also to boundary if user defined at least one attribute?
       if ( type != GV_BOUNDARY )
       {
         // TODO: redo of deleted new features - save new cats somewhere,
         // resetting fid probably is not possible because it is stored in undo commands and used in buffer maps
 
-        QgsDebugMsg( QString( "get new cat for mCidxFieldIndex = %1" ).arg( mCidxFieldIndex ) );
-        if ( mCidxFieldIndex == -1 )
+        // It may be that user manualy entered cat value
+        const QgsFeature &feature = mEditBuffer->addedFeatures()[fid];
+        int catIndex = feature.fields()->indexFromName( mLayer->keyColumnName() );
+        if ( catIndex != -1 )
         {
-          // No features with this field yet in map
-          newCat = 1;
+          QVariant userCatVariant = feature.attributes().value( catIndex );
+          if ( !userCatVariant.isNull() )
+          {
+            newCat = userCatVariant.toInt();
+            QgsDebugMsg( QString( "user defined newCat = %1" ).arg( newCat ) );
+          }
         }
-        else
+        if ( newCat == 0 )
         {
-          newCat = cidxGetMaxCat( mCidxFieldIndex ) + 1;
+          QgsDebugMsg( QString( "get new cat for mCidxFieldIndex = %1" ).arg( mCidxFieldIndex ) );
+          if ( mCidxFieldIndex == -1 )
+          {
+            // No features with this field yet in map
+            newCat = 1;
+          }
+          else
+          {
+            newCat = cidxGetMaxCat( mCidxFieldIndex ) + 1;
+          }
         }
         QgsDebugMsg( QString( "newCat = %1" ).arg( newCat ) );
         Vect_cat_set( cats, mLayerField, newCat );
         QString error;
-        mLayer->insertAttributes( newCat, error );
-        if ( !error.isEmpty() )
+
+        // if the cat is user defined, the record may alredy exist
+        if ( mLayer->attributes().contains( newCat ) )
         {
-          QgsGrass::warning( error );
+          QgsDebugMsg( "record exists" );
+          // TODO: open warning dialog?
+          // For now we are expecting that user knows what he is doing.
+          // We update existing record by non null values and set feature null values to existing values
+          mLayer->updateAttributes( newCat, feature, error ); // also updates feature by existing non null attributes
+
+          // There may be other new features with the same cat which we have to update
+          QgsFeatureMap& addedFeatures = const_cast<QgsFeatureMap&>( mEditBuffer->addedFeatures() );
+          Q_FOREACH ( QgsFeatureId addedFid, addedFeatures.keys() )
+          {
+            if ( addedFid == fid )
+            {
+              continue;
+            }
+            int addedCat = mLayer->map()->newCats().value( addedFid ); // it should always exist
+            QgsDebugMsg( QString( "addedFid = %1 addedCat = %2" ).arg( addedFid ).arg( addedCat ) );
+            if ( addedCat == newCat )
+            {
+              QgsFeature addedFeature = addedFeatures[addedFid];
+              // TODO: better to update form mLayer->attributes() ?
+              for ( int i = 0; i < feature.fields()->size(); i++ )
+              {
+                if ( feature.fields()->field( i ).name() == QgsGrassVectorMap::topoSymbolFieldName() )
+                {
+                  continue;
+                }
+                if ( feature.attributes()[i].isNull() )
+                {
+                  continue;
+                }
+                addedFeature.setAttribute( i, feature.attributes()[i] );
+              }
+              addedFeatures[addedFid] = addedFeature;
+            }
+          }
+
+          // Update all changed attributes
+          // TODO: table does not get refreshed immediately
+          QgsChangedAttributesMap &changedAttributes = const_cast<QgsChangedAttributesMap &>( mEditBuffer->changedAttributeValues() );
+          Q_FOREACH ( QgsFeatureId changedFid, changedAttributes.keys() )
+          {
+            int changedCat = QgsGrassFeatureIterator::catFromFid( changedFid );
+            int realChangedCat = changedCat;
+            if ( mLayer->map()->newCats().contains( changedFid ) )
+            {
+              realChangedCat = mLayer->map()->newCats().value( changedFid );
+            }
+            QgsDebugMsg( QString( "changedFid = %1 changedCat = %2 realChangedCat = %3" )
+                         .arg( changedFid ).arg( changedCat ).arg( realChangedCat ) );
+            if ( realChangedCat == newCat )
+            {
+              QgsAttributeMap attributeMap = changedAttributes[changedFid];
+              Q_FOREACH ( int index, attributeMap.keys() )
+              {
+                attributeMap[index] = feature.attributes().value( index );
+              }
+              changedAttributes[changedFid] = attributeMap;
+            }
+          }
+        }
+        else
+        {
+          mLayer->insertAttributes( newCat, feature, error );
+          if ( !error.isEmpty() )
+          {
+            QgsGrass::warning( error );
+          }
         }
       }
     }
@@ -1228,6 +1311,8 @@ void QgsGrassProvider::onFeatureAdded( QgsFeatureId fid )
           {
             addedFeatures[fid].setAttribute( 0, newCat );
           }
+          mLayer->map()->newCats()[fid] = newCat;
+          QgsDebugMsg( QString( "newCats[%1] = %2" ).arg( fid ).arg( newCat ) );
         }
       }
     }
@@ -1365,11 +1450,17 @@ void QgsGrassProvider::onAttributeValueChanged( QgsFeatureId fid, int idx, const
   int oldLid = QgsGrassFeatureIterator::lidFromFid( fid );
   int cat = QgsGrassFeatureIterator::catFromFid( fid );
   int realLine = oldLid;
+  int realCat = cat;
   if ( mLayer->map()->newLids().contains( oldLid ) ) // if it was changed already
   {
     realLine = mLayer->map()->newLids().value( oldLid );
   }
-  QgsDebugMsg( QString( "fid = %1 oldLid = %2 realLine = %3 cat = %4" ).arg( fid ).arg( oldLid ).arg( realLine ).arg( cat ) );
+  if ( mLayer->map()->newCats().contains( fid ) )
+  {
+    realCat = mLayer->map()->newCats().value( fid );
+  }
+  QgsDebugMsg( QString( "fid = %1 oldLid = %2 realLine = %3 cat = %4 realCat = %5" )
+               .arg( fid ).arg( oldLid ).arg( realLine ).arg( cat ).arg( realCat ) );
 
   // index is for current fields
   if ( idx < 0 || idx > mEditLayer->fields().size() )
@@ -1379,10 +1470,10 @@ void QgsGrassProvider::onAttributeValueChanged( QgsFeatureId fid, int idx, const
   }
   QgsField field = mEditLayer->fields()[idx];
 
-  if ( cat > 0 )
+  if ( realCat > 0 )
   {
     QString error;
-    mLayer->changeAttributeValue( cat, field, value, error );
+    mLayer->changeAttributeValue( realCat, field, value, error );
     if ( !error.isEmpty() )
     {
       QgsGrass::warning( error );

diff --git a/src/providers/grass/qgsgrassvectormap.cpp b/src/providers/grass/qgsgrassvectormap.cpp
@@ -318,6 +318,7 @@ bool QgsGrassVectorMap::closeEdit( bool newMap )
   mOldLids.clear();
   mNewLids.clear();
   mOldGeometries.clear();
+  mNewCats.clear();
 
   // Mapset must be set before Vect_close()
   QgsGrass::setMapset( mGrassObject.gisdbase(), mGrassObject.location(), mGrassObject.mapset() );

diff --git a/src/providers/grass/qgsgrassvectormap.h b/src/providers/grass/qgsgrassvectormap.h
@@ -78,6 +78,7 @@ class GRASS_LIB_EXPORT QgsGrassVectorMap : public QObject
     QHash<int, int> & oldLids() { return mOldLids; }
     QHash<int, int> & newLids() { return mNewLids; }
     QHash<int, QgsAbstractGeometryV2*> & oldGeometries() { return mOldGeometries; }
+    QHash<int, int> & newCats() { return mNewCats; }
 
     /** Get geometry of line.
      * @return geometry (point,line or polygon(GV_FACE)) or 0 */
@@ -183,6 +184,9 @@ class GRASS_LIB_EXPORT QgsGrassVectorMap : public QObject
     QHash<int, int> mNewLids;
     // Hash of original lines' geometries of lines which were changed, keys are GRASS lid
     QHash<int, QgsAbstractGeometryV2*> mOldGeometries;
+    // New categories attached to new features or old features without category
+    // fid -> cat, the fid may be old fid without category or new (negative) feature id
+    QHash<int, int> mNewCats;
 
     // Mutex used to avoid concurrent read/write, used only in editing mode
     QMutex mReadWriteMutex;

diff --git a/src/providers/grass/qgsgrassvectormaplayer.cpp b/src/providers/grass/qgsgrassvectormaplayer.cpp
@@ -772,7 +772,8 @@ void QgsGrassVectorMapLayer::insertCats( QString &error )
     {
       int cat;
       Vect_cidx_get_cat_by_index( map()->map(), cidxIndex, i, &cat, 0, 0 );
-      insertAttributes( cat, error );
+      QgsFeature feature;
+      insertAttributes( cat, feature, error );
       if ( !error.isEmpty() )
       {
         QgsDebugMsg( error );
@@ -782,13 +783,38 @@ void QgsGrassVectorMapLayer::insertCats( QString &error )
   }
 }
 
-void QgsGrassVectorMapLayer::insertAttributes( int cat, QString &error )
+void QgsGrassVectorMapLayer::insertAttributes( int cat, const QgsFeature &feature, QString &error )
 {
   QgsDebugMsg( QString( "mField = %1 cat = %2" ).arg( mField ).arg( cat ) );
 
   if ( mHasTable )
   {
-    QString query = QString( "INSERT INTO %1 ( %2 ) VALUES ( %3 )" ).arg( mFieldInfo->table ).arg( mFieldInfo->key ).arg( cat );
+    QStringList names;
+    QStringList values;
+
+    names << mFieldInfo->key;
+    values << QString::number( cat );
+
+    if ( feature.isValid() && feature.fields() )
+    {
+      // append feature attributes if not null
+      for ( int i = 0; i < feature.fields()->size(); i++ )
+      {
+        QString name = feature.fields()->at( i ).name();
+        if ( name == mFieldInfo->key )
+        {
+          continue;
+        }
+        QVariant valueVariant = feature.attributes().value( i );
+        if ( !valueVariant.isNull() )
+        {
+          names << name;
+          values << quotedValue( valueVariant );
+        }
+      }
+    }
+
+    QString query = QString( "INSERT INTO %1 ( %2 ) VALUES ( %3 )" ).arg( mFieldInfo->table ).arg( names.join( ", " ) ).arg( values.join( "," ) );
     executeSql( query, error );
     if ( error.isEmpty() )
     {
@@ -802,6 +828,77 @@ void QgsGrassVectorMapLayer::insertAttributes( int cat, QString &error )
   }
 }
 
+void QgsGrassVectorMapLayer::updateAttributes( int cat, const QgsFeature &feature, QString &error, bool nullValues )
+{
+  QgsDebugMsg( QString( "mField = %1 cat = %2" ).arg( mField ).arg( cat ) );
+
+  if ( !mHasTable )
+  {
+    error = tr( "Table does not exit" );
+    return;
+  }
+  if ( !feature.isValid() || !feature.fields() )
+  {
+    error = tr( "Feature invalid" );
+    return;
+  }
+
+  QStringList updates;
+  QMap<int, QVariant> cacheUpdates;
+  // append feature attributes if not null
+  for ( int i = 0; i < feature.fields()->size(); i++ )
+  {
+    QString name = feature.fields()->at( i ).name();
+    if ( name == mFieldInfo->key )
+    {
+      continue;
+    }
+    QVariant valueVariant = feature.attributes().value( i );
+
+    int cacheIndex = mAttributeFields.indexFromName( name );
+
+    if ( valueVariant.isNull() && !nullValues )
+    {
+      // update feature null values by existing values
+      if ( cacheIndex != -1 )
+      {
+        feature.attributes()[i] = mAttributes[cat][cacheIndex];
+      }
+      continue;
+    }
+
+    updates << name + " = " + quotedValue( valueVariant );
+
+
+    if ( cacheIndex == -1 )
+    {
+      QgsDebugMsg( "cannot find cache index for attribute " + name );
+    }
+    else
+    {
+      cacheUpdates[cacheIndex] = valueVariant;
+    }
+  }
+
+  if ( updates.isEmpty() )
+  {
+    QgsDebugMsg( "nothing to update" );
+    return;
+  }
+
+  QString query = QString( "UPDATE %1 SET %2 WHERE %3 = %4" ).arg( mFieldInfo->table )
+                  .arg( updates.join( ", " ) ).arg( mFieldInfo->key ).arg( cat );
+
+  executeSql( query, error );
+  if ( error.isEmpty() )
+  {
+    Q_FOREACH ( int index, cacheUpdates.keys() )
+    {
+      mAttributes[cat][index] = cacheUpdates[index];
+    }
+  }
+}
+
 void QgsGrassVectorMapLayer::deleteAttribute( int cat, QString &error )
 {
   QgsDebugMsg( QString( "mField = %1 cat = %2" ).arg( mField ).arg( cat ) );
@@ -913,5 +1010,7 @@ void QgsGrassVectorMapLayer::changeAttributeValue( int cat, QgsField field, QVar
   }
   db_free_string( &dbstr );
 
+  // TODO: update cached attributes because another feature may share the same cat
+
   return;
 }
diff --git a/src/providers/grass/qgsgrassvectormaplayer.h b/src/providers/grass/qgsgrassvectormaplayer.h
@@ -23,6 +23,7 @@
 #include <QPair>
 
 #include "qgsfield.h"
+#include "qgsfeature.h"
 
 extern "C"
 {
@@ -65,6 +66,7 @@ class GRASS_LIB_EXPORT QgsGrassVectorMapLayer : public QObject
 
     bool hasTable() { return mHasTable; }
     int keyColumn() { return mKeyColumn; }
+    QString keyColumnName() { return mFieldInfo ? mFieldInfo->key : QString(); }
     QList< QPair<double, double> > minMax() { return mMinMax; }
     int userCount() { return mUsers; }
     void addUser();
@@ -96,7 +98,13 @@ class GRASS_LIB_EXPORT QgsGrassVectorMapLayer : public QObject
 
     /** Insert new attributes to the table (it does not check if attributes already exists)
      *   @param cat */
-    void insertAttributes( int cat, QString &error );
+    void insertAttributes( int cat, const QgsFeature &feature, QString &error );
+
+    /** Update existing record by values from feature.
+     *  @param cat
+     *  @param nullValues override all values, if false, only non empty values are used for update
+     */
+    void updateAttributes( int cat, const QgsFeature &feature, QString &error, bool nullValues = false );
 
     /** Delete attributes from the table
      *   @param cat