Invalid join cache when layer is modified (fix #11140)

python/core/qgsvectorlayer.sip

@@ -8,6 +8,7 @@ struct QgsVectorJoinInfo
 %TypeHeaderCode
 #include "qgsvectorlayer.h"
 %End
+  QgsVectorJoinInfo();
 
   /** Join field in the target layer*/
   QString targetFieldName;
@@ -17,6 +18,8 @@ struct QgsVectorJoinInfo
   QString joinFieldName;
   /** True if the join is cached in virtual memory*/
   bool memoryCache;
+  /** True if the cached join attributes need to be updated*/
+  bool cacheDirty;
   /** Cache for joined attributes to provide fast lookup (size is 0 if no memory caching)
    * @note not available in python bindings
    */

src/core/qgsvectorlayer.h

@@ -75,6 +75,13 @@ typedef QList<QgsPointV2> QgsPointSequenceV2;
 
 struct CORE_EXPORT QgsVectorJoinInfo
 {
+  QgsVectorJoinInfo()
+      : memoryCache( false )
+      , cacheDirty( true )
+      , targetFieldIndex( -1 )
+      , joinFieldIndex( -1 )
+  {}
+
   /** Join field in the target layer*/
   QString targetFieldName;
   /** Source layer*/
@@ -83,6 +90,9 @@ struct CORE_EXPORT QgsVectorJoinInfo
   QString joinFieldName;
   /** True if the join is cached in virtual memory*/
   bool memoryCache;
+  /** True if the cached join attributes need to be updated*/
+  bool cacheDirty;
+
   /** Cache for joined attributes to provide fast lookup (size is 0 if no memory caching)
    * @note not available in python bindings
    */

src/core/qgsvectorlayerfeatureiterator.cpp

@@ -31,7 +31,10 @@ QgsVectorLayerFeatureSource::QgsVectorLayerFeatureSource( QgsVectorLayer *layer
 {
   mProviderFeatureSource = layer->dataProvider()->featureSource();
   mFields = layer->fields();
+
+  layer->createJoinCaches();
   mJoinBuffer = layer->mJoinBuffer->clone();
+
   mExpressionFieldBuffer = new QgsExpressionFieldBuffer( *layer->mExpressionFieldBuffer );
   mCrsId = layer->crs().srsid();
 

src/core/qgsvectorlayerjoinbuffer.cpp

@@ -87,6 +87,7 @@ bool QgsVectorLayerJoinBuffer::addJoin( const QgsVectorJoinInfo& joinInfo )
   if ( QgsVectorLayer* vl = qobject_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( joinInfo.joinLayerId ) ) )
   {
     connect( vl, SIGNAL( updatedFields() ), this, SLOT( joinedLayerUpdatedFields() ), Qt::UniqueConnection );
+    connect( vl, SIGNAL( layerModified() ), this, SLOT( joinedLayerModified() ), Qt::UniqueConnection );
   }
 
   emit joinedFieldsChanged();
@@ -118,7 +119,7 @@ bool QgsVectorLayerJoinBuffer::removeJoin( const QString& joinLayerId )
 void QgsVectorLayerJoinBuffer::cacheJoinLayer( QgsVectorJoinInfo& joinInfo )
 {
   //memory cache not required or already done
-  if ( !joinInfo.memoryCache || !joinInfo.cachedAttributes.isEmpty() )
+  if ( !joinInfo.memoryCache || !joinInfo.cacheDirty )
   {
     return;
   }
@@ -175,6 +176,7 @@ void QgsVectorLayerJoinBuffer::cacheJoinLayer( QgsVectorJoinInfo& joinInfo )
         joinInfo.cachedAttributes.insert( key, attrs2 );
       }
     }
+    joinInfo.cacheDirty = false;
   }
 }
 
@@ -260,11 +262,15 @@ void QgsVectorLayerJoinBuffer::createJoinCaches()
   QList< QgsVectorJoinInfo >::iterator joinIt = mVectorJoins.begin();
   for ( ; joinIt != mVectorJoins.end(); ++joinIt )
   {
+    if ( joinIt->memoryCache && joinIt->cacheDirty )
       cacheJoinLayer( *joinIt );
 
     // make sure we are connected to the joined layer
     if ( QgsVectorLayer* vl = qobject_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( joinIt->joinLayerId ) ) )
+    {
       connect( vl, SIGNAL( updatedFields() ), this, SLOT( joinedLayerUpdatedFields() ), Qt::UniqueConnection );
+      connect( vl, SIGNAL( layerModified() ), this, SLOT( joinedLayerModified() ), Qt::UniqueConnection );
+    }
   }
 }
 
@@ -329,6 +335,7 @@ void QgsVectorLayerJoinBuffer::readXml( const QDomNode& layer_node )
       info.joinLayerId = infoElem.attribute( "joinLayerId" );
       info.targetFieldName = infoElem.attribute( "targetFieldName" );
       info.memoryCache = infoElem.attribute( "memoryCache" ).toInt();
+      info.cacheDirty = true;
 
       info.joinFieldIndex = infoElem.attribute( "joinField" ).toInt();   //for compatibility with 1.x
       info.targetFieldIndex = infoElem.attribute( "targetField" ).toInt();   //for compatibility with 1.x
@@ -397,6 +404,9 @@ QgsVectorLayerJoinBuffer* QgsVectorLayerJoinBuffer::clone() const
 
 void QgsVectorLayerJoinBuffer::joinedLayerUpdatedFields()
 {
+  // TODO - check - this whole method is probably not needed anymore,
+  // since the cache handling is covered by joinedLayerModified()
+
   QgsVectorLayer* joinedLayer = qobject_cast<QgsVectorLayer*>( sender() );
   Q_ASSERT( joinedLayer );
 
@@ -412,3 +422,18 @@ void QgsVectorLayerJoinBuffer::joinedLayerUpdatedFields()
 
   emit joinedFieldsChanged();
 }
+
+void QgsVectorLayerJoinBuffer::joinedLayerModified()
+{
+  QgsVectorLayer* joinedLayer = qobject_cast<QgsVectorLayer*>( sender() );
+  Q_ASSERT( joinedLayer );
+
+  // recache the joined layer
+  for ( QgsVectorJoinList::iterator it = mVectorJoins.begin(); it != mVectorJoins.end(); ++it )
+  {
+    if ( joinedLayer->id() == it->joinLayerId )
+    {
+      it->cacheDirty = true;
+    }
+  }
+}

src/core/qgsvectorlayerjoinbuffer.h

@@ -90,6 +90,8 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer : public QObject
   private slots:
     void joinedLayerUpdatedFields();
 
+    void joinedLayerModified();
+
   private:
 
     QgsVectorLayer* mLayer;

tests/src/core/testqgsvectorlayerjoinbuffer.cpp

@@ -57,6 +57,8 @@ class TestVectorLayerJoinBuffer : public QObject
     void testJoinTwoTimes_data();
     void testJoinTwoTimes();
     void testJoinLayerDefinitionFile();
+    void testCacheUpdate_data();
+    void testCacheUpdate();
 
   private:
     QList<QString> mProviders;
@@ -519,6 +521,87 @@ void TestVectorLayerJoinBuffer::testJoinLayerDefinitionFile()
   QVERIFY( vLayer->fieldNameIndex( joinInfo.prefix + "value" ) >= 0 );
 }
 
+void TestVectorLayerJoinBuffer::testCacheUpdate_data()
+{
+  QTest::addColumn<bool>( "useCache" );
+  QTest::newRow( "cache" ) << true;
+  QTest::newRow( "no cache" ) << false;
+}
+
+void TestVectorLayerJoinBuffer::testCacheUpdate()
+{
+  QFETCH( bool, useCache );
+
+  QgsVectorLayer* vlA = new QgsVectorLayer( "Point?field=id_a:integer", "cacheA", "memory" );
+  QVERIFY( vlA->isValid() );
+  QgsVectorLayer* vlB = new QgsVectorLayer( "Point?field=id_b:integer&field=value_b", "cacheB", "memory" );
+  QVERIFY( vlB->isValid() );
+  QgsMapLayerRegistry::instance()->addMapLayer( vlA );
+  QgsMapLayerRegistry::instance()->addMapLayer( vlB );
+
+  QgsFeature fA1( vlA->dataProvider()->fields(), 1 );
+  fA1.setAttribute( "id_a", 1 );
+  QgsFeature fA2( vlA->dataProvider()->fields(), 2 );
+  fA2.setAttribute( "id_a", 2 );
+
+  vlA->dataProvider()->addFeatures( QgsFeatureList() << fA1 << fA2 );
+
+  QgsFeature fB1( vlB->dataProvider()->fields(), 1 );
+  fB1.setAttribute( "id_b", 1 );
+  fB1.setAttribute( "value_b", 11 );
+  QgsFeature fB2( vlB->dataProvider()->fields(), 2 );
+  fB2.setAttribute( "id_b", 2 );
+  fB2.setAttribute( "value_b", 12 );
+
+  vlB->dataProvider()->addFeatures( QgsFeatureList() << fB1 << fB2 );
+
+  QgsVectorJoinInfo joinInfo;
+  joinInfo.targetFieldName = "id_a";
+  joinInfo.joinLayerId = vlB->id();
+  joinInfo.joinFieldName = "id_b";
+  joinInfo.memoryCache = useCache;
+  joinInfo.prefix = "B_";
+  vlA->addJoin( joinInfo );
+
+  QgsFeatureIterator fi = vlA->getFeatures();
+  fi.nextFeature( fA1 );
+  QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
+  QCOMPARE( fA1.attribute( "B_value_b" ).toInt(), 11 );
+  fi.nextFeature( fA2 );
+  QCOMPARE( fA2.attribute( "id_a" ).toInt(), 2 );
+  QCOMPARE( fA2.attribute( "B_value_b" ).toInt(), 12 );
+
+  // change value in join target layer
+  vlB->startEditing();
+  vlB->changeAttributeValue( 1, 1, 111 );
+  vlB->changeAttributeValue( 2, 0, 3 );
+  vlB->commitChanges();
+
+  fi = vlA->getFeatures();
+  fi.nextFeature( fA1 );
+  QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
+  QCOMPARE( fA1.attribute( "B_value_b" ).toInt(), 111 );
+  fi.nextFeature( fA2 );
+  QCOMPARE( fA2.attribute( "id_a" ).toInt(), 2 );
+  QVERIFY( fA2.attribute( "B_value_b" ).isNull() );
+
+  // change value in joined layer
+  vlA->startEditing();
+  vlA->changeAttributeValue( 2, 0, 3 );
+  vlA->commitChanges();
+
+  fi = vlA->getFeatures();
+  fi.nextFeature( fA1 );
+  QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
+  QCOMPARE( fA1.attribute( "B_value_b" ).toInt(), 111 );
+  fi.nextFeature( fA2 );
+  QCOMPARE( fA2.attribute( "id_a" ).toInt(), 3 );
+  QCOMPARE( fA2.attribute( "B_value_b" ).toInt(), 12 );
+
+  QgsMapLayerRegistry::instance()->removeMapLayer( vlA );
+  QgsMapLayerRegistry::instance()->removeMapLayer( vlB );
+}
+
 
 QTEST_MAIN( TestVectorLayerJoinBuffer )
 #include "testqgsvectorlayerjoinbuffer.moc"