Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix buffered transaction crash because of wrong orderLayers method
  • Loading branch information
@domi4484
domi4484 committed May 23, 2023
1 parent 0811919 commit 7d250a4
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 73 deletions.
100 changes: 27 additions & 73 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,40 @@ 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 layers
const QList<QgsRelation> referencedRelations = QgsProject::instance()->relationManager()->referencedRelations( *unorderedLayerIterator );

int insertIndex = -1;
const QList<QgsRelation> relations = QgsProject::instance()->relationManager()->referencingRelations( layer );
for ( const QgsRelation &relation : relations )
// If at least one modified layer references this layer continue
for ( const QgsRelation &relation : referencedRelations )
{
QgsVectorLayer *referencedLayer = relation.referencedLayer();
int index = orderedLayers.indexOf( referencedLayer );
if ( index >= 0 )
if ( unorderedLayers.contains( relation.referencingLayer() ) )
{
insertIndex = std::max( insertIndex, index + 1 );
}
else
{
// 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;
}
++unorderedLayerIterator;
continue;
}
}

// No place found this cycle
if ( insertIndex == -1 )
{
otherLayersQueue.enqueue( layer );
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
68 changes: 68 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,68 @@ 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())


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

0 comments on commit 7d250a4

Please sign in to comment.