Skip to content
Permalink
Browse files

Merge pull request #9620 from elpaso/bugfix-badlayers-storage

Bugfix badlayers storage
  • Loading branch information
elpaso committed Mar 28, 2019
2 parents 20ec29a + bd9b373 commit 5b5cc50b9e93359398d2f4f54c82a79e339a26b2
@@ -120,6 +120,13 @@ The layersAdded() and layerWasAdded() signals will always be emitted.

:param layers: A list of layer which should be added to the store.

.. note::

If a layer with the same id is already in the store it is not added again,
but if the validity of the layer has changed from ``False`` to ``True``, the
layer data source is updated to the new one.


:return: a list of the map layers that were added
successfully. If a layer already exists in the store,
it will not be part of the returned list.
@@ -366,7 +366,6 @@ void QgsHandleBadLayers::apply()
QHash<QString, QString> baseChange;



for ( int i = 0; i < mLayerList->rowCount(); i++ )
{
int idx = mLayerList->item( i, 0 )->data( Qt::UserRole ).toInt();
@@ -405,27 +404,27 @@ void QgsHandleBadLayers::apply()
datasource = datasource.replace( basepath, baseChange[basepath] );


bool dataSourceChanged { false };
bool dataSourceFixed { false };
const QString layerId { node.namedItem( QStringLiteral( "id" ) ).toElement().text() };
const QString provider { node.namedItem( QStringLiteral( "provider" ) ).toElement().text() };

// Try first to change the datasource of the existing layers, this will
// maintain the current status (checked/unchecked) and group
if ( QgsProject::instance()->mapLayer( layerId ) )
{
QgsDataProvider::ProviderOptions options;
QgsMapLayer *mapLayer = QgsProject::instance()->mapLayer( layerId );
if ( mapLayer )
{
QgsDataProvider::ProviderOptions options;
mapLayer->setDataSource( datasource, name, provider, options );
dataSourceChanged = mapLayer->isValid();
dataSourceFixed = mapLayer->isValid();
}
}

// If the data source was changed successfully, remove the bad layer from the dialog
// otherwise, try to set the new datasource in the XML node and reload the layer,
// finally marks with red all remaining bad layers.
if ( dataSourceChanged )
if ( dataSourceFixed )
{
mLayerList->removeRow( i-- );
}
@@ -447,7 +446,8 @@ void QgsHandleBadLayers::apply()
}
}

// Final cleanup: remove any bad layer (it should not be any btw)
// Final cleanup: remove any remaining bad layer
// (there should not be any at this point)
if ( mLayerList->rowCount() == 0 )
{
QList<QgsMapLayer *> toRemove;
@@ -74,6 +74,11 @@ QList<QgsMapLayer *> QgsMapLayerStore::addMapLayers( const QList<QgsMapLayer *>
QgsDebugMsg( QStringLiteral( "Cannot add null layers" ) );
continue;
}
// If the layer is already in the store but its validity has flipped to TRUE reset data source
if ( mMapLayers.contains( myLayer->id() ) && ! mMapLayers[myLayer->id()]->isValid() && myLayer->isValid() && myLayer->dataProvider() )
{
mMapLayers[myLayer->id()]->setDataSource( myLayer->dataProvider()->dataSourceUri(), myLayer->name(), myLayer->providerType(), QgsDataProvider::ProviderOptions() );
}
//check the layer is not already registered!
if ( !mMapLayers.contains( myLayer->id() ) )
{
@@ -149,6 +149,11 @@ class CORE_EXPORT QgsMapLayerStore : public QObject
* If you specify FALSE here you have take care of deleting
* the layers yourself. Not available in Python.
*
*
* \note If a layer with the same id is already in the store it is not added again,
* but if the validity of the layer has changed from FALSE to TRUE, the
* layer data source is updated to the new one.
*
* \returns a list of the map layers that were added
* successfully. If a layer already exists in the store,
* it will not be part of the returned list.
@@ -944,30 +944,30 @@ bool QgsProject::addLayer( const QDomElement &layerElem, QList<QDomNode> &broken
{
QString type = layerElem.attribute( QStringLiteral( "type" ) );
QgsDebugMsgLevel( "Layer type is " + type, 4 );
QgsMapLayer *mapLayer = nullptr;
std::unique_ptr<QgsMapLayer> mapLayer;

if ( type == QLatin1String( "vector" ) )
{
mapLayer = new QgsVectorLayer;
mapLayer = qgis::make_unique<QgsVectorLayer>();

// apply specific settings to vector layer
if ( QgsVectorLayer *vl = qobject_cast<QgsVectorLayer *>( mapLayer ) )
if ( QgsVectorLayer *vl = qobject_cast<QgsVectorLayer *>( mapLayer.get() ) )
{
vl->setReadExtentFromXml( mTrustLayerMetadata );
}
}
else if ( type == QLatin1String( "raster" ) )
{
mapLayer = new QgsRasterLayer;
mapLayer = qgis::make_unique<QgsRasterLayer>();
}
else if ( type == QLatin1String( "mesh" ) )
{
mapLayer = new QgsMeshLayer;
mapLayer = qgis::make_unique<QgsMeshLayer>();
}
else if ( type == QLatin1String( "plugin" ) )
{
QString typeName = layerElem.attribute( QStringLiteral( "name" ) );
mapLayer = QgsApplication::pluginLayerRegistry()->createLayer( typeName );
mapLayer.reset( QgsApplication::pluginLayerRegistry()->createLayer( typeName ) );
}

if ( !mapLayer )
@@ -978,13 +978,19 @@ bool QgsProject::addLayer( const QDomElement &layerElem, QList<QDomNode> &broken

Q_CHECK_PTR( mapLayer ); // NOLINT

// This is tricky: to avoid a leak we need to check if the layer was already in the store
// because if it was, the newly created layer will not be added to the store and it would leak.
const QString layerId { layerElem.namedItem( QStringLiteral( "id" ) ).toElement().text() };
Q_ASSERT( ! layerId.isEmpty() );
const bool layerWasStored { layerStore()->mapLayer( layerId ) };

// have the layer restore state that is stored in Dom node
bool layerIsValid = mapLayer->readLayerXml( layerElem, context ) && mapLayer->isValid();
QList<QgsMapLayer *> newLayers;
newLayers << mapLayer;
newLayers << mapLayer.get();
if ( layerIsValid )
{
emit readMapLayer( mapLayer, layerElem );
emit readMapLayer( mapLayer.get(), layerElem );
addMapLayers( newLayers );
}
else
@@ -995,6 +1001,14 @@ bool QgsProject::addLayer( const QDomElement &layerElem, QList<QDomNode> &broken
QgsDebugMsg( "Unable to load " + type + " layer" );
brokenNodes.push_back( layerElem );
}

// It should be safe to delete the layer now if layer was stored, because all the store
// had to to was to reset the data source in case the validity changed.
if ( ! layerWasStored )
{
mapLayer.release();
}

return layerIsValid;
}

@@ -12,14 +12,25 @@
# This will get replaced with a git SHA1 when you do a git archive
__revision__ = '$Format:%H$'

from qgis.core import QgsMapLayerStore, QgsVectorLayer, QgsMapLayer
import os
from qgis.core import (
QgsMapLayerStore,
QgsVectorLayer,
QgsMapLayer,
QgsDataProvider,
QgsProject,
QgsReadWriteContext,
)
from qgis.testing import start_app, unittest
from qgis.PyQt.QtCore import QT_VERSION_STR
from qgis.PyQt import sip
from qgis.PyQt.QtTest import QSignalSpy
from qgis.PyQt.QtXml import QDomDocument, QDomNode
from time import sleep
from utilities import unitTestDataPath

start_app()
TEST_DATA_DIR = unitTestDataPath()


def createLayer(name):
@@ -529,6 +540,38 @@ def testTransferLayers(self):
self.assertEqual(len(store1.mapLayers()), 3)
self.assertEqual(store1.mapLayers(), {l1.id(): l1, l2.id(): l2, l3.id(): l3})

def testLayerDataSourceReset(self):
"""When adding a layer with the same id to the store make sure
the data source is also updated in case the layer validity has
changed from False to True"""

p = QgsProject()
store = p.layerStore()
vl1 = createLayer('valid')
vl2 = QgsVectorLayer('/not_a_valid_path.shp', 'invalid', 'ogr')
self.assertTrue(vl1.isValid())
self.assertFalse(vl2.isValid())
store.addMapLayers([vl1, vl2])
self.assertEqual(store.validCount(), 1)
self.assertEqual(len(store.mapLayers()), 2)

# Re-add the bad layer
store.addMapLayers([vl2])
self.assertEqual(store.validCount(), 1)
self.assertEqual(len(store.mapLayers()), 2)

doc = QDomDocument()
doc.setContent('<maplayer><provider encoding="UTF-8">ogr</provider><layername>fixed</layername><id>%s</id></maplayer>' % vl2.id())
layer_node = QDomNode(doc.firstChild())
self.assertTrue(vl2.writeXml(layer_node, doc, QgsReadWriteContext()))
datasource_node = doc.createElement("datasource")
datasource_node.appendChild(doc.createTextNode(os.path.join(TEST_DATA_DIR, 'points.shp')))
layer_node.appendChild(datasource_node)
p.readLayer(layer_node)
self.assertEqual(store.validCount(), 2)
self.assertEqual(len(store.mapLayers()), 2)
self.assertEqual(store.mapLayers()[vl2.id()].name(), 'fixed')


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

0 comments on commit 5b5cc50

Please sign in to comment.
You can’t perform that action at this time.