From 0a5ad7358122d1f5e7bf72d293ebad1e30265498 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 13 Jun 2016 22:08:36 +1000 Subject: [PATCH] Invalid join cache when layer is modified (fix #11140) --- python/core/qgsvectorlayer.sip | 3 + src/core/qgsvectorlayer.h | 10 +++ src/core/qgsvectorlayerfeatureiterator.cpp | 3 + src/core/qgsvectorlayerjoinbuffer.cpp | 29 ++++++- src/core/qgsvectorlayerjoinbuffer.h | 2 + .../src/core/testqgsvectorlayerjoinbuffer.cpp | 83 +++++++++++++++++++ 6 files changed, 128 insertions(+), 2 deletions(-) diff --git a/python/core/qgsvectorlayer.sip b/python/core/qgsvectorlayer.sip index aff4a20f449..c98839a8a1c 100644 --- a/python/core/qgsvectorlayer.sip +++ b/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 */ diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h index 6d21e354059..025a7729c37 100644 --- a/src/core/qgsvectorlayer.h +++ b/src/core/qgsvectorlayer.h @@ -75,6 +75,13 @@ typedef QList 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 */ diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp index ac6c55a5719..c479bb4ee67 100644 --- a/src/core/qgsvectorlayerfeatureiterator.cpp +++ b/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(); diff --git a/src/core/qgsvectorlayerjoinbuffer.cpp b/src/core/qgsvectorlayerjoinbuffer.cpp index 7054403f79d..3f71f5bec26 100644 --- a/src/core/qgsvectorlayerjoinbuffer.cpp +++ b/src/core/qgsvectorlayerjoinbuffer.cpp @@ -87,6 +87,7 @@ bool QgsVectorLayerJoinBuffer::addJoin( const QgsVectorJoinInfo& joinInfo ) if ( QgsVectorLayer* vl = qobject_cast( 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 ) { - cacheJoinLayer( *joinIt ); + if ( joinIt->memoryCache && joinIt->cacheDirty ) + cacheJoinLayer( *joinIt ); // make sure we are connected to the joined layer if ( QgsVectorLayer* vl = qobject_cast( 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( sender() ); Q_ASSERT( joinedLayer ); @@ -412,3 +422,18 @@ void QgsVectorLayerJoinBuffer::joinedLayerUpdatedFields() emit joinedFieldsChanged(); } + +void QgsVectorLayerJoinBuffer::joinedLayerModified() +{ + QgsVectorLayer* joinedLayer = qobject_cast( 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; + } + } +} diff --git a/src/core/qgsvectorlayerjoinbuffer.h b/src/core/qgsvectorlayerjoinbuffer.h index e3104d0973e..85c1e69768b 100644 --- a/src/core/qgsvectorlayerjoinbuffer.h +++ b/src/core/qgsvectorlayerjoinbuffer.h @@ -90,6 +90,8 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer : public QObject private slots: void joinedLayerUpdatedFields(); + void joinedLayerModified(); + private: QgsVectorLayer* mLayer; diff --git a/tests/src/core/testqgsvectorlayerjoinbuffer.cpp b/tests/src/core/testqgsvectorlayerjoinbuffer.cpp index 70dee061955..f239a872b9b 100644 --- a/tests/src/core/testqgsvectorlayerjoinbuffer.cpp +++ b/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 mProviders; @@ -519,6 +521,87 @@ void TestVectorLayerJoinBuffer::testJoinLayerDefinitionFile() QVERIFY( vLayer->fieldNameIndex( joinInfo.prefix + "value" ) >= 0 ); } +void TestVectorLayerJoinBuffer::testCacheUpdate_data() +{ + QTest::addColumn( "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"