Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #53098 from domi4484/fixBufferedTransactionOrderLa… 
…yersMethod

Fix buffered transaction crash because of wrong orderLayers method
  • Loading branch information
@m-kuhn
m-kuhn committed May 24, 2023
2 parents eab811f + b2b0e38 commit 058a73d
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 70 deletions.
100 changes: 30 additions & 70 deletions src/core/vector/qgsvectorlayereditbuffergroup.cpp
View file
Expand Up @@ -181,6 +181,13 @@ bool QgsVectorLayerEditBufferGroup::commitChanges( QStringList &commitErrors, bo
{
for ( orderedLayersIterator = orderedLayers.constBegin(); orderedLayersIterator != orderedLayers.constEnd(); ++orderedLayersIterator )
{
if ( ( *orderedLayersIterator )->editBuffer() == nullptr )
{
commitErrors << tr( "ERROR: edit buffer of layer '%1' is not valid." ).arg( ( *orderedLayersIterator )->name() );
success = false;
break;
}

success = ( *orderedLayersIterator )->editBuffer()->commitChangesCheckGeometryTypeCompatibility( commitErrors );
if ( ! success )
break;
Expand Down Expand Up @@ -382,93 +389,46 @@ bool QgsVectorLayerEditBufferGroup::isEditing() const

QList<QgsVectorLayer *> QgsVectorLayerEditBufferGroup::orderLayersParentsToChildren( QSet<QgsVectorLayer *> layers )
{
QSet<QgsVectorLayer *> referencingLayers;
QSet<QgsVectorLayer *> referencedLayers;

{
const QList<QgsRelation> relations = QgsProject::instance()->relationManager()->relations().values();
for ( const QgsRelation &relation : relations )
{
referencingLayers.insert( relation.referencingLayer() );
referencedLayers.insert( relation.referencedLayer() );
}
}

QList<QgsVectorLayer *> orderedLayers;
QSet<QgsVectorLayer *> unorderedLayers = layers;

// Layers that are only parents
bool layerOrdered = true;
while ( ! unorderedLayers.isEmpty() && layerOrdered )
{
QSet<QgsVectorLayer *> onlyParents = referencedLayers - referencingLayers;
orderedLayers.append( onlyParents.values() );
}

// Other related layers
{
QSet<QgsVectorLayer *> intersection = referencedLayers;
intersection.intersect( referencingLayers );

QQueue<QgsVectorLayer *> otherLayersQueue;
otherLayersQueue.append( intersection.values() );
while ( ! otherLayersQueue.isEmpty() )
layerOrdered = false;
QSet<QgsVectorLayer *>::iterator unorderedLayerIterator = unorderedLayers.begin();
while ( unorderedLayerIterator != unorderedLayers.end() )
{
QgsVectorLayer *layer = otherLayersQueue.dequeue();
// Get referencing relation to find referenced layers
const QList<QgsRelation> referencingRelations = QgsProject::instance()->relationManager()->referencingRelations( *unorderedLayerIterator );

int insertIndex = -1;
const QList<QgsRelation> relations = QgsProject::instance()->relationManager()->referencingRelations( layer );
for ( const QgsRelation &relation : relations )
// If this layer references at least one modified layer continue
bool layerReferencingModifiedLayer = false;
for ( const QgsRelation &relation : referencingRelations )
{
QgsVectorLayer *referencedLayer = relation.referencedLayer();
int index = orderedLayers.indexOf( referencedLayer );
if ( index >= 0 )
{
insertIndex = std::max( insertIndex, index + 1 );
}
else
if ( unorderedLayers.contains( relation.referencedLayer() ) )
{
// Check if there is a circular relation
bool circularRelation = false;
const QList<QgsRelation> backRelations = QgsProject::instance()->relationManager()->referencingRelations( referencedLayer );
for ( const QgsRelation &backRelation : backRelations )
{
if ( backRelation.referencedLayer() == layer )
{
QgsLogger::warning( tr( "Circular relation between layers '%1' and '%2'. Correct saving order of layers can't be guaranteed" ).arg( layer->name(), referencedLayer->name() ) );
insertIndex = orderedLayers.size();
circularRelation = true;
break;
}
}

if ( !circularRelation )
{
insertIndex = -1;
break;
}
layerReferencingModifiedLayer = true;
break;
}
}

// No place found this cycle
if ( insertIndex == -1 )
if ( layerReferencingModifiedLayer )
{
otherLayersQueue.enqueue( layer );
++unorderedLayerIterator;
continue;
}

orderedLayers.insert( insertIndex, layer );
// No modified layer is referencing this layer
orderedLayers.append( *unorderedLayerIterator );
unorderedLayerIterator = unorderedLayers.erase( unorderedLayerIterator );
layerOrdered = true;
}
}

// Layers that are only children
{
QSet<QgsVectorLayer *> onlyChildren = referencingLayers - referencedLayers;
orderedLayers.append( onlyChildren.values() );
}

// Layers without relations (all other layers)
if ( ! unorderedLayers.isEmpty() )
{
QSet<QgsVectorLayer *> layersWithoutRelations = layers - referencedLayers;
layersWithoutRelations -= referencingLayers;
orderedLayers.append( layersWithoutRelations.values() );
QgsLogger::warning( tr( "Circular relation between some layers. Correct saving order of layers can't be guaranteed" ) );
orderedLayers.append( unorderedLayers.values() );
}

return orderedLayers;
Expand Down
148 changes: 148 additions & 0 deletions tests/src/python/test_qgsvectorlayereditbuffergroup.py
View file
Expand Up @@ -19,6 +19,8 @@
QgsFeature,
QgsGeometry,
QgsProject,
QgsRelation,
QgsRelationContext,
QgsVectorFileWriter,
QgsVectorLayer,
)
Expand All @@ -29,6 +31,10 @@

class TestQgsVectorLayerEditBufferGroup(unittest.TestCase):

def tearDown(self):
"""Run after each test."""
QgsProject.instance().removeAllMapLayers()

def testStartEditingCommitRollBack(self):

ml = QgsVectorLayer('Point?crs=epsg:4326&field=int:integer&field=int2:integer', 'test', 'memory')
Expand Down Expand Up @@ -150,6 +156,148 @@ def testSetBufferedGroupsAfterAutomaticGroups(self):

self.assertTrue(success)

def testReadOnlyLayers(self):

memoryLayer_a = QgsVectorLayer('Point?crs=epsg:4326&field=id:integer&field=id_b', 'test', 'memory')
self.assertTrue(memoryLayer_a.isValid())
memoryLayer_b = QgsVectorLayer('Point?crs=epsg:4326&field=id:integer', 'test', 'memory')
self.assertTrue(memoryLayer_b.isValid())

# Load 2 layer from a geopackage
d = QTemporaryDir()
options = QgsVectorFileWriter.SaveVectorOptions()
options.driverName = 'GPKG'
options.layerName = 'layer_a'
err, msg, newFileName, newLayer = QgsVectorFileWriter.writeAsVectorFormatV3(memoryLayer_a, os.path.join(d.path(), 'test_EditBufferGroupReadOnly.gpkg'), QgsCoordinateTransformContext(), options)

options.layerName = 'layer_b'
options.actionOnExistingFile = QgsVectorFileWriter.CreateOrOverwriteLayer
err, msg, newFileName, newLayer = QgsVectorFileWriter.writeAsVectorFormatV3(memoryLayer_b, os.path.join(d.path(), 'test_EditBufferGroupReadOnly.gpkg'), QgsCoordinateTransformContext(), options)

layer_a = QgsVectorLayer(newFileName + '|layername=layer_a')
self.assertTrue(layer_a.isValid())
layer_b = QgsVectorLayer(newFileName + '|layername=layer_b')
self.assertTrue(layer_b.isValid())
layer_b.setReadOnly(True)

project = QgsProject.instance()
project.addMapLayers([layer_a, layer_b])

relationContext = QgsRelationContext(project)
relation = QgsRelation(relationContext)
relation.setId('relation')
relation.setName('Relation Number One')
relation.setReferencingLayer(layer_a.id())
relation.setReferencedLayer(layer_b.id())
relation.addFieldPair("id_b", "id")

self.assertEqual(relation.validationError(), "")
self.assertTrue(relation.isValid())

project.relationManager().addRelation(relation)

project.setTransactionMode(Qgis.TransactionMode.BufferedGroups)
project.startEditing()

editBufferGroup = project.editBufferGroup()
self.assertTrue(editBufferGroup.isEditing())

f = QgsFeature(layer_a.fields())
f.setAttribute('id', 123)
f.setAttribute('id_b', 1)
f.setGeometry(QgsGeometry.fromWkt('point(7 45)'))
self.assertTrue(layer_a.addFeatures([f]))
self.assertEqual(len(editBufferGroup.modifiedLayers()), 1)
self.assertIn(layer_a, editBufferGroup.modifiedLayers())

# Check feature in layer edit buffer but not in provider till commit
self.assertEqual(layer_a.featureCount(), 1)
self.assertEqual(layer_a.dataProvider().featureCount(), 0)

success, commitErrors = editBufferGroup.commitChanges(True)
self.assertTrue(success)
self.assertFalse(editBufferGroup.isEditing())

def testCircularRelations(self):

memoryLayer_a = QgsVectorLayer('Point?crs=epsg:4326&field=id:integer&field=id_b', 'test', 'memory')
self.assertTrue(memoryLayer_a.isValid())
memoryLayer_b = QgsVectorLayer('Point?crs=epsg:4326&field=id:integer&field=id_a', 'test', 'memory')
self.assertTrue(memoryLayer_b.isValid())

# Load 2 layer from a geopackage
d = QTemporaryDir()
options = QgsVectorFileWriter.SaveVectorOptions()
options.driverName = 'GPKG'
options.layerName = 'layer_a'
err, msg, newFileName, newLayer = QgsVectorFileWriter.writeAsVectorFormatV3(memoryLayer_a, os.path.join(d.path(), 'test_EditBufferGroupCircularRelations.gpkg'), QgsCoordinateTransformContext(), options)

options.layerName = 'layer_b'
options.actionOnExistingFile = QgsVectorFileWriter.CreateOrOverwriteLayer
err, msg, newFileName, newLayer = QgsVectorFileWriter.writeAsVectorFormatV3(memoryLayer_b, os.path.join(d.path(), 'test_EditBufferGroupCircularRelations.gpkg'), QgsCoordinateTransformContext(), options)

layer_a = QgsVectorLayer(newFileName + '|layername=layer_a')
self.assertTrue(layer_a.isValid())
layer_b = QgsVectorLayer(newFileName + '|layername=layer_b')
self.assertTrue(layer_b.isValid())

project = QgsProject.instance()
project.addMapLayers([layer_a, layer_b])

relationContext = QgsRelationContext(project)

relation_ab = QgsRelation(relationContext)
relation_ab.setId('relation_ab')
relation_ab.setName('Relation a b')
relation_ab.setReferencingLayer(layer_a.id())
relation_ab.setReferencedLayer(layer_b.id())
relation_ab.addFieldPair("id_b", "id")
self.assertEqual(relation_ab.validationError(), "")
self.assertTrue(relation_ab.isValid())
project.relationManager().addRelation(relation_ab)

relation_ba = QgsRelation(relationContext)
relation_ba.setId('relation_ba')
relation_ba.setName('Relation b a')
relation_ba.setReferencingLayer(layer_b.id())
relation_ba.setReferencedLayer(layer_a.id())
relation_ba.addFieldPair("id_a", "id")
self.assertEqual(relation_ba.validationError(), "")
self.assertTrue(relation_ba.isValid())
project.relationManager().addRelation(relation_ba)

project.setTransactionMode(Qgis.TransactionMode.BufferedGroups)
project.startEditing()

editBufferGroup = project.editBufferGroup()
self.assertTrue(editBufferGroup.isEditing())

f = QgsFeature(layer_a.fields())
f.setAttribute('id', 123)
f.setAttribute('id_b', 1)
f.setGeometry(QgsGeometry.fromWkt('point(7 45)'))
self.assertTrue(layer_a.addFeatures([f]))
self.assertEqual(len(editBufferGroup.modifiedLayers()), 1)
self.assertIn(layer_a, editBufferGroup.modifiedLayers())

f = QgsFeature(layer_b.fields())
f.setAttribute('id', 1)
f.setAttribute('id_a', 123)
f.setGeometry(QgsGeometry.fromWkt('point(8 46)'))
self.assertTrue(layer_b.addFeatures([f]))
self.assertEqual(len(editBufferGroup.modifiedLayers()), 2)
self.assertIn(layer_b, editBufferGroup.modifiedLayers())

# Check feature in layer edit buffer but not in provider till commit
self.assertEqual(layer_a.featureCount(), 1)
self.assertEqual(layer_a.dataProvider().featureCount(), 0)
self.assertEqual(layer_b.featureCount(), 1)
self.assertEqual(layer_b.dataProvider().featureCount(), 0)

success, commitErrors = editBufferGroup.commitChanges(True)
self.assertTrue(success)
self.assertFalse(editBufferGroup.isEditing())


if __name__ == '__main__':
unittest.main()

0 comments on commit 058a73d

Please sign in to comment.