Fix #10912 (joined attributes are not correctly propagated in nested joins)

This commit makes QgsVectorLayerJoinBuffer listen to changes in fields
of joined vector layers in order to update the cache and inform parent layer

python/core/qgsfield.sip

@@ -193,6 +193,11 @@ class QgsFields
     //! Utility function to return a list of QgsField instances
     QList<QgsField> toList() const;
 
+    //! @note added in 2.6
+    bool operator==( const QgsFields& other ) const;
+    //! @note added in 2.6
+    bool operator!=( const QgsFields& other ) const;
+
 /*  SIP_PYOBJECT __getitem__(int key);
 %MethodCode
   if (a0 = sipConvertFromSequenceIndex(a0, sipCpp->count()) < 0)

python/core/qgsvectorlayerjoinbuffer.sip

@@ -1,4 +1,4 @@
-class QgsVectorLayerJoinBuffer
+class QgsVectorLayerJoinBuffer : QObject
 {
 %TypeHeaderCode
 #include <qgsvectorlayerjoinbuffer.h>
@@ -39,4 +39,12 @@ class QgsVectorLayerJoinBuffer
       @param sourceFieldIndex Output: field's index in source layer */
     const QgsVectorJoinInfo* joinForFieldIndex( int index, const QgsFields& fields, int& sourceFieldIndex /Out/ ) const;
 
+    //! Create a copy of the join buffer
+    //! @note added in 2.6
+    QgsVectorLayerJoinBuffer* clone() const /Factory/;
+
+  signals:
+    //! Emitted whenever the list of joined fields changes (e.g. added join or joined layer's fields change)
+    //! @note added in 2.6
+    void joinedFieldsChanged();
 };

src/core/CMakeLists.txt

@@ -369,6 +369,7 @@ SET(QGIS_CORE_MOC_HDRS
   qgsnetworkaccessmanager.h
   qgsvectordataprovider.h
   qgsvectorlayercache.h
+  qgsvectorlayerjoinbuffer.h
   qgsgeometryvalidator.h
 
   composer/qgsaddremoveitemcommand.h

src/core/qgsfield.h

@@ -178,6 +178,11 @@ class CORE_EXPORT QgsFields
       Field(): origin( OriginUnknown ), originIndex( -1 ) {}
       Field( const QgsField& f, FieldOrigin o, int oi ): field( f ), origin( o ), originIndex( oi ) {}
 
+      //! @note added in 2.6
+      bool operator==( const Field& other ) const { return field == other.field && origin == other.origin && originIndex == other.originIndex; }
+      //! @note added in 2.6
+      bool operator!=( const Field& other ) const { return !( *this == other ); }
+
       QgsField field;      //!< field
       FieldOrigin origin;  //!< origin of the field
       int originIndex;     //!< index specific to the origin
@@ -238,6 +243,11 @@ class CORE_EXPORT QgsFields
     //! Utility function to return a list of QgsField instances
     QList<QgsField> toList() const;
 
+    //! @note added in 2.6
+    bool operator==( const QgsFields& other ) const { return mFields == other.mFields; }
+    //! @note added in 2.6
+    bool operator!=( const QgsFields& other ) const { return ! ( *this == other ); }
+
   protected:
     //! internal storage of the container
     QVector<Field> mFields;

src/core/qgsvectorlayer.cpp

@@ -1304,6 +1304,7 @@ bool QgsVectorLayer::readXml( const QDomNode& layer_node )
   if ( !mJoinBuffer )
   {
     mJoinBuffer = new QgsVectorLayerJoinBuffer();
+    connect( mJoinBuffer, SIGNAL( joinedFieldsChanged() ), this, SLOT( onJoinedFieldsChanged() ) );
   }
   mJoinBuffer->readXml( layer_node );
 
@@ -1366,6 +1367,7 @@ bool QgsVectorLayer::setDataProvider( QString const & provider )
       mWkbType = mDataProvider->geometryType();
 
       mJoinBuffer = new QgsVectorLayerJoinBuffer();
+      connect( mJoinBuffer, SIGNAL( joinedFieldsChanged() ), this, SLOT( onJoinedFieldsChanged() ) );
       mExpressionFieldBuffer = new QgsExpressionFieldBuffer();
       updateFields();
 
@@ -2782,6 +2784,8 @@ void QgsVectorLayer::updateFields()
   if ( !mDataProvider )
     return;
 
+  QgsFields oldFields = mUpdatedFields;
+
   mUpdatedFields = mDataProvider->fields();
 
   // added / removed fields
@@ -2795,7 +2799,8 @@ void QgsVectorLayer::updateFields()
   if ( mExpressionFieldBuffer )
     mExpressionFieldBuffer->updateFields( mUpdatedFields );
 
-  emit updatedFields();
+  if ( oldFields != mUpdatedFields )
+    emit updatedFields();
 }
 
 
@@ -3563,6 +3568,12 @@ void QgsVectorLayer::onRelationsLoaded()
   }
 }
 
+void QgsVectorLayer::onJoinedFieldsChanged()
+{
+  // some of the fields of joined layers have changed -> we need to update this layer's fields too
+  updateFields();
+}
+
 QgsVectorLayer::ValueRelationData QgsVectorLayer::valueRelation( int idx )
 {
   if ( editorWidgetV2( idx ) == "ValueRelation" )

src/core/qgsvectorlayer.h

@@ -1641,6 +1641,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
 
   private slots:
     void onRelationsLoaded();
+    void onJoinedFieldsChanged();
 
   protected:
     /** Set the extent */

src/core/qgsvectorlayerfeatureiterator.cpp

@@ -27,7 +27,7 @@ QgsVectorLayerFeatureSource::QgsVectorLayerFeatureSource( QgsVectorLayer *layer
 {
   mProviderFeatureSource = layer->dataProvider()->featureSource();
   mFields = layer->pendingFields();
-  mJoinBuffer = new QgsVectorLayerJoinBuffer( *layer->mJoinBuffer );
+  mJoinBuffer = layer->mJoinBuffer->clone();
   mExpressionFieldBuffer = new QgsExpressionFieldBuffer( *layer->mExpressionFieldBuffer );
 
   mCanBeSimplified = layer->hasGeometryType() && layer->geometryType() != QGis::Point;

src/core/qgsvectorlayerjoinbuffer.cpp

@@ -39,8 +39,18 @@ void QgsVectorLayerJoinBuffer::addJoin( const QgsVectorJoinInfo& joinInfo )
   {
     cacheJoinLayer( mVectorJoins.last() );
   }
+
+  // Wait for notifications about changed fields in joined layer to propagate them.
+  // During project load the joined layers possibly do not exist yet so the connection will not be created,
+  // but then QgsProject makes sure to call createJoinCaches() which will do the connection.
+  // Unique connection makes sure we do not respond to one layer's update more times (in case of multiple join)
+  if ( QgsVectorLayer* vl = qobject_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( joinInfo.joinLayerId ) ) )
+    connect( vl, SIGNAL( updatedFields() ), this, SLOT( joinedLayerUpdatedFields() ), Qt::UniqueConnection );
+
+  emit joinedFieldsChanged();
 }
 
+
 void QgsVectorLayerJoinBuffer::removeJoin( const QString& joinLayerId )
 {
   for ( int i = 0; i < mVectorJoins.size(); ++i )
@@ -50,6 +60,11 @@ void QgsVectorLayerJoinBuffer::removeJoin( const QString& joinLayerId )
       mVectorJoins.removeAt( i );
     }
   }
+
+  if ( QgsVectorLayer* vl = qobject_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( joinLayerId ) ) )
+    disconnect( vl, SIGNAL( updatedFields() ), this, SLOT( joinedLayerUpdatedFields() ) );
+
+  emit joinedFieldsChanged();
 }
 
 void QgsVectorLayerJoinBuffer::cacheJoinLayer( QgsVectorJoinInfo& joinInfo )
@@ -121,6 +136,10 @@ void QgsVectorLayerJoinBuffer::createJoinCaches()
   for ( ; joinIt != mVectorJoins.end(); ++joinIt )
   {
     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 );
   }
 }
 
@@ -188,3 +207,28 @@ const QgsVectorJoinInfo* QgsVectorLayerJoinBuffer::joinForFieldIndex( int index,
 
   return &( mVectorJoins[sourceJoinIndex] );
 }
+
+QgsVectorLayerJoinBuffer* QgsVectorLayerJoinBuffer::clone() const
+{
+  QgsVectorLayerJoinBuffer* cloned = new QgsVectorLayerJoinBuffer;
+  cloned->mVectorJoins = mVectorJoins;
+  return cloned;
+}
+
+void QgsVectorLayerJoinBuffer::joinedLayerUpdatedFields()
+{
+  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->cachedAttributes.clear();
+      cacheJoinLayer( *it );
+    }
+  }
+
+  emit joinedFieldsChanged();
+}

src/core/qgsvectorlayerjoinbuffer.h

@@ -25,11 +25,13 @@
 #include <QString>
 
 
+typedef QList< QgsVectorJoinInfo > QgsVectorJoinList;
 
 
 /**Manages joined fields for a vector layer*/
-class CORE_EXPORT QgsVectorLayerJoinBuffer
+class CORE_EXPORT QgsVectorLayerJoinBuffer : public QObject
 {
+    Q_OBJECT
   public:
     QgsVectorLayerJoinBuffer();
     ~QgsVectorLayerJoinBuffer();
@@ -58,7 +60,7 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer
     /**Quick way to test if there is any join at all*/
     bool containsJoins() const { return !mVectorJoins.isEmpty(); }
 
-    const QList< QgsVectorJoinInfo >& vectorJoins() const { return mVectorJoins; }
+    const QgsVectorJoinList& vectorJoins() const { return mVectorJoins; }
 
     /**Finds the vector join for a layer field index.
       @param index this layers attribute index
@@ -66,10 +68,22 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer
       @param sourceFieldIndex Output: field's index in source layer */
     const QgsVectorJoinInfo* joinForFieldIndex( int index, const QgsFields& fields, int& sourceFieldIndex ) const;
 
+    //! Create a copy of the join buffer
+    //! @note added in 2.6
+    QgsVectorLayerJoinBuffer* clone() const;
+
+  signals:
+    //! Emitted whenever the list of joined fields changes (e.g. added join or joined layer's fields change)
+    //! @note added in 2.6
+    void joinedFieldsChanged();
+
+  private slots:
+    void joinedLayerUpdatedFields();
+
   private:
 
     /**Joined vector layers*/
-    QList< QgsVectorJoinInfo > mVectorJoins;
+    QgsVectorJoinList mVectorJoins;
 
     /**Caches attributes of join layer in memory if QgsVectorJoinInfo.memoryCache is true (and the cache is not already there)*/
     void cacheJoinLayer( QgsVectorJoinInfo& joinInfo );

tests/src/core/CMakeLists.txt

@@ -133,3 +133,4 @@ ADD_QGIS_TEST(colorschemeregistry testqgscolorschemeregistry.cpp)
 ADD_QGIS_TEST(colorscheme testqgscolorscheme.cpp)
 ADD_QGIS_TEST(networkcontentfetcher testqgsnetworkcontentfetcher.cpp )
 ADD_QGIS_TEST(legendrenderertest testqgslegendrenderer.cpp )
+ADD_QGIS_TEST(vectorlayerjoinbuffer testqgsvectorlayerjoinbuffer.cpp )

tests/src/core/testqgsvectorlayerjoinbuffer.cpp

@@ -0,0 +1,197 @@
+/***************************************************************************
+    testqgsvectorlayerjoinbuffer.cpp
+     --------------------------------------
+    Date                 : September 2014
+    Copyright            : (C) 2014 Martin Dobias
+    Email                : wonder.sk at gmail dot com
+ ***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+
+#include <QtTest>
+#include <QObject>
+
+//qgis includes...
+#include <qgsvectorlayer.h>
+#include <qgsvectordataprovider.h>
+#include <qgsapplication.h>
+#include <qgsvectorlayerjoinbuffer.h>
+#include <qgsmaplayerregistry.h>
+
+/** @ingroup UnitTests
+ * This is a unit test for the vector layer join buffer
+ *
+ * @see QgsVectorLayerJoinBuffer
+ */
+class TestVectorLayerJoinBuffer: public QObject
+{
+    Q_OBJECT
+
+  private slots:
+    void initTestCase();      // will be called before the first testfunction is executed.
+    void cleanupTestCase();   // will be called after the last testfunction was executed.
+    void init();              // will be called before each testfunction is executed.
+    void cleanup();           // will be called after every testfunction.
+
+    void testCacheBasic();
+    void testCacheTransitive();
+
+  private:
+    QgsVectorLayer* mLayerA;
+    QgsVectorLayer* mLayerB;
+    QgsVectorLayer* mLayerC;
+};
+
+// runs before all tests
+void TestVectorLayerJoinBuffer::initTestCase()
+{
+  QgsApplication::init();
+  QgsApplication::initQgis();
+
+  // LAYER A //
+
+  mLayerA = new QgsVectorLayer( "Point?field=id_a:integer", "A", "memory" );
+  QVERIFY( mLayerA->isValid() );
+  QVERIFY( mLayerA->pendingFields().count() == 1 );
+
+  QgsFeature fA1( mLayerA->dataProvider()->fields(), 1 );
+  fA1.setAttribute( "id_a", 1 );
+  QgsFeature fA2( mLayerA->dataProvider()->fields(), 2 );
+  fA2.setAttribute( "id_a", 2 );
+  mLayerA->dataProvider()->addFeatures( QgsFeatureList() << fA1 << fA2 );
+  QVERIFY( mLayerA->pendingFeatureCount() == 2 );
+
+  // LAYER B //
+
+  mLayerB = new QgsVectorLayer( "Point?field=id_b:integer&field=value_b", "B", "memory" );
+  QVERIFY( mLayerB->isValid() );
+  QVERIFY( mLayerB->pendingFields().count() == 2 );
+
+  QgsFeature fB1( mLayerB->dataProvider()->fields(), 1 );
+  fB1.setAttribute( "id_b", 1 );
+  fB1.setAttribute( "value_b", 11 );
+  QgsFeature fB2( mLayerB->dataProvider()->fields(), 2 );
+  fB2.setAttribute( "id_b", 2 );
+  fB2.setAttribute( "value_b", 12 );
+  mLayerB->dataProvider()->addFeatures( QgsFeatureList() << fB1 << fB2 );
+  QVERIFY( mLayerB->pendingFeatureCount() == 2 );
+
+  // LAYER C //
+
+  mLayerC = new QgsVectorLayer( "Point?field=id_c:integer&field=value_c", "C", "memory" );
+  QVERIFY( mLayerC->isValid() );
+  QVERIFY( mLayerC->pendingFields().count() == 2 );
+
+  QgsFeature fC1( mLayerC->dataProvider()->fields(), 1 );
+  fC1.setAttribute( "id_c", 1 );
+  fC1.setAttribute( "value_c", 101 );
+  mLayerC->dataProvider()->addFeatures( QgsFeatureList() << fC1 );
+  QVERIFY( mLayerC->pendingFeatureCount() == 1 );
+
+  QgsMapLayerRegistry::instance()->addMapLayer( mLayerA );
+  QgsMapLayerRegistry::instance()->addMapLayer( mLayerB );
+  QgsMapLayerRegistry::instance()->addMapLayer( mLayerC );
+}
+
+void TestVectorLayerJoinBuffer::init()
+{
+}
+
+void TestVectorLayerJoinBuffer::cleanup()
+{
+}
+
+void TestVectorLayerJoinBuffer::cleanupTestCase()
+{
+  QgsMapLayerRegistry::instance()->removeAllMapLayers();
+}
+
+void TestVectorLayerJoinBuffer::testCacheBasic()
+{
+  QVERIFY( mLayerA->pendingFields().count() == 1 );
+
+  QgsVectorJoinInfo joinInfo;
+  joinInfo.targetFieldName = "id_a";
+  joinInfo.joinLayerId = mLayerB->id();
+  joinInfo.joinFieldName = "id_b";
+  // memory provider does not implement setSubsetString() so direct join does not work!
+  joinInfo.memoryCache = true;
+  mLayerA->addJoin( joinInfo );
+
+  QVERIFY( mLayerA->pendingFields().count() == 2 );
+
+  QgsFeatureIterator fi = mLayerA->getFeatures();
+  QgsFeature fA1, fA2;
+  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 );
+
+  mLayerA->removeJoin( mLayerB->id() );
+
+  QVERIFY( mLayerA->pendingFields().count() == 1 );
+}
+
+void TestVectorLayerJoinBuffer::testCacheTransitive()
+{
+  // test join A -> B -> C
+  // first we join A -> B and after that B -> C
+  // layer A should automatically update to include joined data from C
+
+  QVERIFY( mLayerA->pendingFields().count() == 1 ); // id_a
+
+  // add join A -> B
+
+  QgsVectorJoinInfo joinInfo1;
+  joinInfo1.targetFieldName = "id_a";
+  joinInfo1.joinLayerId = mLayerB->id();
+  joinInfo1.joinFieldName = "id_b";
+  // memory provider does not implement setSubsetString() so direct join does not work!
+  joinInfo1.memoryCache = true;
+  mLayerA->addJoin( joinInfo1 );
+  QVERIFY( mLayerA->pendingFields().count() == 2 ); // id_a, B_value_b
+
+  // add join B -> C
+
+  QgsVectorJoinInfo joinInfo2;
+  joinInfo2.targetFieldName = "id_b";
+  joinInfo2.joinLayerId = mLayerC->id();
+  joinInfo2.joinFieldName = "id_c";
+  // memory provider does not implement setSubsetString() so direct join does not work!
+  joinInfo2.memoryCache = true;
+  mLayerB->addJoin( joinInfo2 );
+  QVERIFY( mLayerB->pendingFields().count() == 3 ); // id_b, value_b, C_value_c
+
+  // now layer A must include also data from layer C
+  QVERIFY( mLayerA->pendingFields().count() == 3 ); // id_a, B_value_b, B_C_value_c
+
+  QgsFeatureIterator fi = mLayerA->getFeatures();
+  QgsFeature fA1;
+  fi.nextFeature( fA1 );
+  QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
+  QCOMPARE( fA1.attribute( "B_value_b" ).toInt(), 11 );
+  QCOMPARE( fA1.attribute( "B_C_value_c" ).toInt(), 101 );
+
+  // test that layer A gets updated when layer C changes its fields
+  mLayerC->addExpressionField( "123", QgsField( "dummy", QVariant::Int ) );
+  QVERIFY( mLayerA->pendingFields().count() == 4 ); // id_a, B_value_b, B_C_value_c, B_C_dummy
+  mLayerC->removeExpressionField( 0 );
+
+  // cleanup
+  mLayerA->removeJoin( mLayerB->id() );
+  mLayerB->removeJoin( mLayerC->id() );
+}
+
+
+QTEST_MAIN( TestVectorLayerJoinBuffer )
+#include "moc_testqgsvectorlayerjoinbuffer.cxx"
+
+