Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Followup #10912 - detect cycles in joins and reject joins that would …
…create cycle

Cycle would otherwise cause infinite loop when updating fields and it does not make sense
  • Loading branch information
wonder-sk committed Sep 16, 2014
1 parent f73b0b6 commit de48dad
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 19 deletions.
5 changes: 3 additions & 2 deletions python/core/qgsvectorlayer.sip
Expand Up @@ -279,8 +279,9 @@ class QgsVectorLayer : QgsMapLayer

/** Joins another vector layer to this layer
@param joinInfo join object containing join layer id, target and source field
@note added in 1.7 */
void addJoin( const QgsVectorJoinInfo& joinInfo );
@note added in 1.7
@note since 2.6 returns bool indicating whether the join can be added */
bool addJoin( const QgsVectorJoinInfo& joinInfo );

/** Removes a vector layer join
@note added in 1.7 */
Expand Down
7 changes: 4 additions & 3 deletions python/core/qgsvectorlayerjoinbuffer.sip
Expand Up @@ -4,12 +4,13 @@ class QgsVectorLayerJoinBuffer : QObject
#include <qgsvectorlayerjoinbuffer.h>
%End
public:
QgsVectorLayerJoinBuffer();
QgsVectorLayerJoinBuffer( QgsVectorLayer* layer = 0 );
~QgsVectorLayerJoinBuffer();

/**Joins another vector layer to this layer
@param joinInfo join object containing join layer id, target and source field */
void addJoin( const QgsVectorJoinInfo& joinInfo );
@param joinInfo join object containing join layer id, target and source field
@return (since 2.6) whether the join was successfully added */
bool addJoin( const QgsVectorJoinInfo& joinInfo );

/**Removes a vector layer join*/
void removeJoin( const QString& joinLayerId );
Expand Down
10 changes: 4 additions & 6 deletions src/core/qgsvectorlayer.cpp
Expand Up @@ -1298,7 +1298,7 @@ bool QgsVectorLayer::readXml( const QDomNode& layer_node )
//load vector joins
if ( !mJoinBuffer )
{
mJoinBuffer = new QgsVectorLayerJoinBuffer();
mJoinBuffer = new QgsVectorLayerJoinBuffer( this );
connect( mJoinBuffer, SIGNAL( joinedFieldsChanged() ), this, SLOT( onJoinedFieldsChanged() ) );
}
mJoinBuffer->readXml( layer_node );
Expand Down Expand Up @@ -1361,7 +1361,7 @@ bool QgsVectorLayer::setDataProvider( QString const & provider )
// get and store the feature type
mWkbType = mDataProvider->geometryType();

mJoinBuffer = new QgsVectorLayerJoinBuffer();
mJoinBuffer = new QgsVectorLayerJoinBuffer( this );
connect( mJoinBuffer, SIGNAL( joinedFieldsChanged() ), this, SLOT( onJoinedFieldsChanged() ) );
mExpressionFieldBuffer = new QgsExpressionFieldBuffer();
updateFields();
Expand Down Expand Up @@ -2736,10 +2736,9 @@ int QgsVectorLayer::fieldNameIndex( const QString& fieldName ) const
return pendingFields().fieldNameIndex( fieldName );
}

void QgsVectorLayer::addJoin( const QgsVectorJoinInfo& joinInfo )
bool QgsVectorLayer::addJoin( const QgsVectorJoinInfo& joinInfo )
{
mJoinBuffer->addJoin( joinInfo );
updateFields();
return mJoinBuffer->addJoin( joinInfo );
}

void QgsVectorLayer::checkJoinLayerRemove( QString theLayerId )
Expand All @@ -2750,7 +2749,6 @@ void QgsVectorLayer::checkJoinLayerRemove( QString theLayerId )
void QgsVectorLayer::removeJoin( const QString& joinLayerId )
{
mJoinBuffer->removeJoin( joinLayerId );
updateFields();
}

const QList< QgsVectorJoinInfo >& QgsVectorLayer::vectorJoins() const
Expand Down
5 changes: 3 additions & 2 deletions src/core/qgsvectorlayer.h
Expand Up @@ -640,8 +640,9 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer

/** Joins another vector layer to this layer
@param joinInfo join object containing join layer id, target and source field
@note added in 1.7 */
void addJoin( const QgsVectorJoinInfo& joinInfo );
@note added in 1.7
@note since 2.6 returns bool indicating whether the join can be added */
bool addJoin( const QgsVectorJoinInfo& joinInfo );

/** Removes a vector layer join
@note added in 1.7 */
Expand Down
47 changes: 44 additions & 3 deletions src/core/qgsvectorlayerjoinbuffer.cpp
Expand Up @@ -22,18 +22,58 @@

#include <QDomElement>

QgsVectorLayerJoinBuffer::QgsVectorLayerJoinBuffer()
QgsVectorLayerJoinBuffer::QgsVectorLayerJoinBuffer( QgsVectorLayer* layer )
: mLayer( layer )
{
}

QgsVectorLayerJoinBuffer::~QgsVectorLayerJoinBuffer()
{
}

void QgsVectorLayerJoinBuffer::addJoin( const QgsVectorJoinInfo& joinInfo )
static QList<QgsVectorLayer*> _outEdges( QgsVectorLayer* vl )
{
QList<QgsVectorLayer*> lst;
foreach ( const QgsVectorJoinInfo& info, vl->vectorJoins() )
{
if ( QgsVectorLayer* joinVl = qobject_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( info.joinLayerId ) ) )
lst << joinVl;
}
return lst;
}

static bool _hasCycleDFS( QgsVectorLayer* n, QHash<QgsVectorLayer*, int>& mark )
{
if ( mark.value( n ) == 1 ) // temporary
return true;
if ( mark.value( n ) == 0 ) // not visited
{
mark[n] = 1; // temporary
foreach ( QgsVectorLayer* m, _outEdges(n) )
{
if ( _hasCycleDFS( m, mark ) )
return true;
}
mark[n] = 2; // permanent
}
return false;
}


bool QgsVectorLayerJoinBuffer::addJoin( const QgsVectorJoinInfo& joinInfo )
{
mVectorJoins.push_back( joinInfo );

// run depth-first search to detect cycles in the graph of joins between layers.
// any cycle would cause infinite recursion when updating fields
QHash<QgsVectorLayer*, int> markDFS;
if ( mLayer && _hasCycleDFS( mLayer, markDFS ) )
{
// we have to reject this one
mVectorJoins.pop_back();
return false;
}

//cache joined layer to virtual memory if specified by user
if ( joinInfo.memoryCache )
{
Expand All @@ -48,6 +88,7 @@ void QgsVectorLayerJoinBuffer::addJoin( const QgsVectorJoinInfo& joinInfo )
connect( vl, SIGNAL( updatedFields() ), this, SLOT( joinedLayerUpdatedFields() ), Qt::UniqueConnection );

emit joinedFieldsChanged();
return true;
}


Expand Down Expand Up @@ -315,7 +356,7 @@ const QgsVectorJoinInfo* QgsVectorLayerJoinBuffer::joinForFieldIndex( int index,

QgsVectorLayerJoinBuffer* QgsVectorLayerJoinBuffer::clone() const
{
QgsVectorLayerJoinBuffer* cloned = new QgsVectorLayerJoinBuffer;
QgsVectorLayerJoinBuffer* cloned = new QgsVectorLayerJoinBuffer( mLayer );
cloned->mVectorJoins = mVectorJoins;
return cloned;
}
Expand Down
9 changes: 6 additions & 3 deletions src/core/qgsvectorlayerjoinbuffer.h
Expand Up @@ -33,12 +33,13 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer : public QObject
{
Q_OBJECT
public:
QgsVectorLayerJoinBuffer();
QgsVectorLayerJoinBuffer( QgsVectorLayer* layer = 0 );
~QgsVectorLayerJoinBuffer();

/**Joins another vector layer to this layer
@param joinInfo join object containing join layer id, target and source field */
void addJoin( const QgsVectorJoinInfo& joinInfo );
@param joinInfo join object containing join layer id, target and source field
@return (since 2.6) whether the join was successfully added */
bool addJoin( const QgsVectorJoinInfo& joinInfo );

/**Removes a vector layer join*/
void removeJoin( const QString& joinLayerId );
Expand Down Expand Up @@ -90,6 +91,8 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer : public QObject

private:

QgsVectorLayer* mLayer;

/**Joined vector layers*/
QgsVectorJoinList mVectorJoins;

Expand Down
28 changes: 28 additions & 0 deletions tests/src/core/testqgsvectorlayerjoinbuffer.cpp
Expand Up @@ -42,6 +42,7 @@ class TestVectorLayerJoinBuffer: public QObject
void testJoinBasic_data();
void testJoinBasic();
void testJoinTransitive();
void testJoinDetectCycle();
void testJoinSubset_data();
void testJoinSubset();

Expand Down Expand Up @@ -201,6 +202,33 @@ void TestVectorLayerJoinBuffer::testJoinTransitive()
}


void TestVectorLayerJoinBuffer::testJoinDetectCycle()
{
// if A joins B and B joins A, we may get to an infinite loop if the case is not handled properly

QgsVectorJoinInfo joinInfo;
joinInfo.targetFieldName = "id_a";
joinInfo.joinLayerId = mLayerB->id();
joinInfo.joinFieldName = "id_b";
joinInfo.memoryCache = true;
mLayerA->addJoin( joinInfo );

QgsVectorJoinInfo joinInfo2;
joinInfo2.targetFieldName = "id_b";
joinInfo2.joinLayerId = mLayerA->id();
joinInfo2.joinFieldName = "id_a";
joinInfo2.memoryCache = true;
bool res = mLayerB->addJoin( joinInfo2 );

QVERIFY( !res );

// the join in layer B must be rejected
QVERIFY( mLayerB->vectorJoins().count() == 0 );

mLayerA->removeJoin( mLayerB->id() );
}


void TestVectorLayerJoinBuffer::testJoinSubset_data()
{
QTest::addColumn<bool>( "memoryCache" );
Expand Down

0 comments on commit de48dad

Please sign in to comment.