fix #2397 by clearing attribute maps when retrieving features

git-svn-id: http://svn.osgeo.org/qgis/trunk@12869 c8812cc2-4d05-0410-92ff-de0c093fc19c

src/app/attributetable/qgsattributetablemodel.cpp

@@ -32,24 +32,20 @@
 QgsAttributeTableModel::QgsAttributeTableModel( QgsVectorLayer *theLayer, QObject *parent )
     : QAbstractTableModel( parent )
 {
-  mLastRowId = -1;
-  mLastRow = NULL;
   mLayer = theLayer;
   mFeatureCount = mLayer->pendingFeatureCount();
   loadAttributes();
 
 
   connect( mLayer, SIGNAL( layerModified( bool ) ), this, SLOT( layerModified( bool ) ) );
-  //connect(mLayer, SIGNAL(attributeAdded(int)), this, SLOT( attributeAdded(int)));
-  //connect(mLayer, SIGNAL(attributeDeleted(int)), this, SLOT( attributeDeleted(int)));
   //connect(mLayer, SIGNAL(attributeValueChanged(int, int, const QVariant&)), this, SLOT( attributeValueChanged(int, int, const QVariant&)));
   //connect(mLayer, SIGNAL(featureDeleted(int)), this, SLOT( featureDeleted(int)));
   //connect(mLayer, SIGNAL(featureAdded(int)), this, SLOT( featureAdded(int)));
 
   loadLayer();
 }
 
-bool QgsAttributeTableModel::featureAtId( int fid )
+bool QgsAttributeTableModel::featureAtId( int fid ) const
 {
   return mLayer->featureAtId( fid, mFeat, false, true );
 }
@@ -201,14 +197,12 @@ void QgsAttributeTableModel::loadLayer()
     QgsDebugMsg( "ins" );
     ins = true;
     beginInsertRows( QModelIndex(), mFeatureCount, pendingFeatureCount - 1 );
-// QgsDebugMsg(QString("%1, %2").arg(mFeatureCount).arg(mLayer->pendingFeatureCount() - 1));
   }
   else if ( mFeatureCount > pendingFeatureCount )
   {
     QgsDebugMsg( "rm" );
     rm = true;
     beginRemoveRows( QModelIndex(), pendingFeatureCount, mFeatureCount - 1 );
-// QgsDebugMsg(QString("%1, %2").arg(mFeatureCount).arg(mLayer->pendingFeatureCount() -1));
   }
 
   mLayer->select( mAttributes, QgsRectangle(), false );
@@ -368,16 +362,13 @@ void QgsAttributeTableModel::sort( int column, Qt::SortOrder order )
 }
 
 QVariant QgsAttributeTableModel::data( const QModelIndex &index, int role ) const
-{
-  return (( QgsAttributeTableModel * ) this )->data( index, role );
-}
-
-QVariant QgsAttributeTableModel::data( const QModelIndex &index, int role )
 {
   if ( !index.isValid() || ( role != Qt::TextAlignmentRole && role != Qt::DisplayRole && role != Qt::EditRole ) )
     return QVariant();
 
-  QVariant::Type fldType = mLayer->pendingFields()[ mAttributes[index.column()] ].type();
+  int fieldId = mAttributes[ index.column()];
+
+  QVariant::Type fldType = mLayer->pendingFields()[ fieldId ].type();
   bool fldNumeric = ( fldType == QVariant::Int || fldType == QVariant::Double );
 
   if ( role == Qt::TextAlignmentRole )
@@ -389,21 +380,17 @@ QVariant QgsAttributeTableModel::data( const QModelIndex &index, int role )
   }
 
   // if we don't have the row in current cache, load it from layer first
-  if ( mLastRowId != rowToId( index.row() ) )
+  int rowId = rowToId( index.row() );
+  if ( mFeat.id() != rowId )
   {
-    bool res = featureAtId( rowToId( index.row() ) );
-
-    if ( !res )
+    if ( !featureAtId( rowId ) )
       return QVariant( "ERROR" );
-
-    mLastRowId = rowToId( index.row() );
-    mLastRow = ( QgsAttributeMap * ) & mFeat.attributeMap();
   }
 
-  if ( !mLastRow )
+  if ( mFeat.id() != rowId )
     return QVariant( "ERROR" );
 
-  const QVariant &val = ( *mLastRow )[ mAttributes[index.column()] ];
+  const QVariant &val = mFeat.attributeMap()[ fieldId ];
 
   if ( val.isNull() )
   {
@@ -428,26 +415,21 @@ QVariant QgsAttributeTableModel::data( const QModelIndex &index, int role )
 
 bool QgsAttributeTableModel::setData( const QModelIndex &index, const QVariant &value, int role )
 {
-  if ( !index.isValid() || role != Qt::EditRole )
-    return false;
-
-  if ( !mLayer->isEditable() )
+  if ( !index.isValid() || role != Qt::EditRole || !mLayer->isEditable() )
     return false;
 
-  bool res = featureAtId( rowToId( index.row() ) );
-
-  if ( res )
+  int rowId = rowToId( index.row() );
+  if ( mFeat.id() == rowId || featureAtId( rowId ) )
   {
-    mLastRowId = rowToId( index.row() );
-    mLastRow = ( QgsAttributeMap * ) & mFeat.attributeMap();
+    int fieldId = mAttributes[ index.column()];
 
     disconnect( mLayer, SIGNAL( layerModified( bool ) ), this, SLOT( layerModified( bool ) ) );
 
     mLayer->beginEditCommand( tr( "Attribute changed" ) );
-    mLayer->changeAttributeValue( rowToId( index.row() ), mAttributes[ index.column()], value, true );
+    mLayer->changeAttributeValue( rowId, fieldId, value, true );
     mLayer->endEditCommand();
 
-    ( *mLastRow )[ mAttributes[index.column()] ] = value;
+    mFeat.changeAttribute( fieldId, value );
 
     connect( mLayer, SIGNAL( layerModified( bool ) ), this, SLOT( layerModified( bool ) ) );
   }
@@ -456,6 +438,7 @@ bool QgsAttributeTableModel::setData( const QModelIndex &index, const QVariant &
     return false;
 
   emit dataChanged( index, index );
+
   return true;
 }
 
@@ -466,7 +449,8 @@ Qt::ItemFlags QgsAttributeTableModel::flags( const QModelIndex &index ) const
 
   Qt::ItemFlags flags = QAbstractItemModel::flags( index );
 
-  if ( mLayer->isEditable() )
+  if ( mLayer->isEditable() &&
+       mLayer->editType( mAttributes[ index.column()] ) != QgsVectorLayer::Immutable )
     flags |= Qt::ItemIsEditable;
 
   return flags;

src/app/attributetable/qgsattributetablemodel.h

@@ -185,10 +185,9 @@ class QgsAttributeTableModel: public QAbstractTableModel
     QgsVectorLayer *mLayer;
     int mFeatureCount;
     int mFieldCount;
-    mutable int mLastRowId;
+
     mutable QgsFeature mFeat;
 
-    mutable QgsAttributeMap *mLastRow;
     QgsAttributeList mAttributes;
     QMap< int, const QMap<QString, QVariant> * > mValueMaps;
 
@@ -200,6 +199,7 @@ class QgsAttributeTableModel: public QAbstractTableModel
      * Initializes id <-> row maps
      */
     void initIdMaps();
+
     /**
      * Loads the layer into the model
      */
@@ -215,9 +215,7 @@ class QgsAttributeTableModel: public QAbstractTableModel
      * @param fid feature id
      * @return feature exists
      */
-    virtual bool featureAtId( int fid );
-
-    QVariant data( const QModelIndex &index, int role );
+    virtual bool featureAtId( int fid ) const;
 };
 
 

src/core/qgsfeature.cpp

@@ -109,6 +109,12 @@ void QgsFeature::setAttributeMap( const QgsAttributeMap& attributes )
   mAttributes = attributes;
 }
 
+/**Clear attribute map for this feature*/
+void QgsFeature::clearAttributeMap()
+{
+  mAttributes.clear();
+}
+
 /**
  * Add an attribute to the map
  */

src/core/qgsfeature.h

@@ -93,6 +93,11 @@ class CORE_EXPORT QgsFeature
     /**Sets all the attributes in one go*/
     void setAttributeMap( const QgsAttributeMap& attributeMap );
 
+    /** Clear attribute map
+     * added in 1.5
+     */
+    void clearAttributeMap();
+
     /**
      * Add an attribute to the map
      */

src/core/qgsvectorlayer.cpp

@@ -1434,7 +1434,7 @@ bool QgsVectorLayer::setSubsetString( QString subset )
   return res;
 }
 
-void QgsVectorLayer::updateFeatureAttributes( QgsFeature &f )
+void QgsVectorLayer::updateFeatureAttributes( QgsFeature &f, bool all )
 {
   // do not update when we aren't in editing mode
   if ( !mEditable )
@@ -1455,7 +1455,7 @@ void QgsVectorLayer::updateFeatureAttributes( QgsFeature &f )
 
   // null/add all attributes that were added, but don't exist in the feature yet
   for ( QgsFieldMap::const_iterator it = mUpdatedFields.begin(); it != mUpdatedFields.end(); it++ )
-    if ( !map.contains( it.key() ) )
+    if ( !map.contains( it.key() ) && ( all || mFetchAttributes.contains( it.key() ) ) )
       f.changeAttribute( it.key(), QVariant( QString::null ) );
 }
 
@@ -1490,16 +1490,16 @@ void QgsVectorLayer::select( QgsAttributeList attributes, QgsRectangle rect, boo
     if ( mEditable )
     {
       // fetch only available field from provider
-      QgsAttributeList provAttributes;
+      mFetchProvAttributes.clear();
       for ( QgsAttributeList::iterator it = mFetchAttributes.begin(); it != mFetchAttributes.end(); it++ )
       {
         if ( !mUpdatedFields.contains( *it ) || mAddedAttributeIds.contains( *it ) )
           continue;
 
-        provAttributes << *it;
+        mFetchProvAttributes << *it;
       }
 
-      mDataProvider->select( provAttributes, rect, fetchGeometries, useIntersect );
+      mDataProvider->select( mFetchProvAttributes, rect, fetchGeometries, useIntersect );
     }
     else
       mDataProvider->select( mFetchAttributes, rect, fetchGeometries, useIntersect );
@@ -1564,7 +1564,7 @@ bool QgsVectorLayer::nextFeature( QgsFeature &f )
           {
             // retrieve attributes from provider
             QgsFeature tmp;
-            mDataProvider->featureAtId( fid, tmp, false, mDataProvider->attributeIndexes() );
+            mDataProvider->featureAtId( fid, tmp, false, mFetchProvAttributes );
             updateFeatureAttributes( tmp );
             f.setAttributeMap( tmp.attributeMap() );
           }
@@ -1666,10 +1666,9 @@ bool QgsVectorLayer::featureAtId( int featureId, QgsFeature& f, bool fetchGeomet
         // retrieve attributes from provider
         QgsFeature tmp;
         mDataProvider->featureAtId( featureId, tmp, false, mDataProvider->attributeIndexes() );
-        updateFeatureAttributes( tmp );
         f.setAttributeMap( tmp.attributeMap() );
       }
-      updateFeatureAttributes( f );
+      updateFeatureAttributes( f, true );
     }
     return true;
   }
@@ -1696,7 +1695,7 @@ bool QgsVectorLayer::featureAtId( int featureId, QgsFeature& f, bool fetchGeomet
   {
     if ( mDataProvider->featureAtId( featureId, f, fetchGeometries, mDataProvider->attributeIndexes() ) )
     {
-      updateFeatureAttributes( f );
+      updateFeatureAttributes( f, true );
       return true;
     }
   }

src/core/qgsvectorlayer.h

@@ -643,7 +643,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
     static int currentVertexMarkerSize();
 
     /**Update feature with uncommited attribute updates*/
-    void updateFeatureAttributes( QgsFeature &f );
+    void updateFeatureAttributes( QgsFeature &f, bool all = false );
 
     /**Update feature with uncommited geometry updates*/
     void updateFeatureGeometry( QgsFeature &f );
@@ -774,6 +774,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
     bool mFetching;
     QgsRectangle mFetchRect;
     QgsAttributeList mFetchAttributes;
+    QgsAttributeList mFetchProvAttributes;
     bool mFetchGeometry;
 
     QSet<int> mFetchConsidered;

src/providers/grass/qgsgrassprovider.cpp

@@ -327,6 +327,7 @@ bool QgsGrassProvider::nextFeature( QgsFeature& feature )
 #endif
 
   feature.setFeatureId( id );
+  feature.clearAttributeMap();
 
   // TODO int may be 64 bits (memcpy)
   if ( type & ( GV_POINTS | GV_LINES ) ) /* points or lines */
@@ -491,7 +492,7 @@ void QgsGrassProvider::select( QgsAttributeList fetchAttributes,
 
     Polygon = Vect_new_line_struct();
 
-    // Using z coor -PORT_DOUBLE_MAX/PORT_DOUBLE_MAX we cover 3D, Vect_select_lines_by_polygon is 
+    // Using z coor -PORT_DOUBLE_MAX/PORT_DOUBLE_MAX we cover 3D, Vect_select_lines_by_polygon is
     // using dig_line_box to get the box, it is not perfect, Vect_select_lines_by_polygon
     // should clarify better how 2D/3D is treated
     Vect_append_point( Polygon, rect.xMinimum(), rect.yMinimum(), -PORT_DOUBLE_MAX );

src/providers/ogr/qgsogrprovider.cpp

@@ -387,6 +387,7 @@ bool QgsOgrProvider::featureAtId( int featureId,
     return false;
 
   feature.setFeatureId( OGR_F_GetFID( fet ) );
+  feature.clearAttributeMap();
   // skip features without geometry
   if ( OGR_F_GetGeometryRef( fet ) == NULL && !mFetchFeaturesWithoutGeom )
   {
@@ -451,6 +452,7 @@ bool QgsOgrProvider::nextFeature( QgsFeature& feature )
     OGRFeatureDefnH featureDefinition = OGR_F_GetDefnRef( fet );
     QString featureTypeName = featureDefinition ? QString( OGR_FD_GetName( featureDefinition ) ) : QString( "" );
     feature.setFeatureId( OGR_F_GetFID( fet ) );
+    feature.clearAttributeMap();
     feature.setTypeName( featureTypeName );
 
     /* fetch geometry */

src/providers/postgres/qgspostgresprovider.cpp

@@ -553,6 +553,7 @@ bool QgsPostgresProvider::getFeature( PGresult *queryResult, int row, bool fetch
     }
 
     feature.setFeatureId( oid );
+    feature.clearAttributeMap();
 
     int col;  // first attribute column after geometry
 

src/providers/spatialite/qgsspatialiteprovider.cpp

@@ -258,6 +258,8 @@ bool QgsSpatiaLiteProvider::featureAtId( int featureId, QgsFeature & feature, bo
       feature.setGeometryAndOwnership( 0, 0 );
     }
 
+    feature.clearAttributeMap();
+
     int ic;
     int n_columns = sqlite3_column_count( stmt );
     for ( ic = 0; ic < n_columns; ic++ )
@@ -380,6 +382,8 @@ bool QgsSpatiaLiteProvider::nextFeature( QgsFeature & feature )
       feature.setGeometryAndOwnership( 0, 0 );
     }
 
+    feature.clearAttributeMap();
+
     int ic;
     int n_columns = sqlite3_column_count( sqliteStatement );
     for ( ic = 0; ic < n_columns; ic++ )