Skip to content

Commit

Permalink
Merge pull request #8427 from elpaso/handle-bad-layers3
Browse files Browse the repository at this point in the history
Handle bad layers: fixes for groups and subset string
  • Loading branch information
elpaso authored Nov 6, 2018
2 parents 5ae2bae + 9af0719 commit 54a6586
Show file tree
Hide file tree
Showing 7 changed files with 1,255 additions and 685 deletions.
17 changes: 16 additions & 1 deletion src/app/qgisapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6951,10 +6951,25 @@ void QgisApp::changeDataSource( QgsMapLayer *layer )
if ( uri.isValid() )
{
bool layerIsValid( layer->isValid() );
// Store subset string form vlayer if we are fixing a bad layer
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
QString subsetString;
// Get the subset string directly from the data provider because
// layer's method will return a null string from invalid layers
if ( !layerIsValid && vlayer && vlayer->dataProvider() &&
vlayer->dataProvider()->supportsSubsetString() &&
!vlayer->dataProvider()->subsetString( ).isEmpty() )
{
subsetString = vlayer->dataProvider()->subsetString();
}
layer->setDataSource( uri.uri, layer->name(), uri.providerKey, QgsDataProvider::ProviderOptions() );
// Re-apply style
// Re-apply original style and subset string when fixing bad layers
if ( !( layerIsValid || layer->originalXmlProperties().isEmpty() ) )
{
if ( ! subsetString.isEmpty() )
{
vlayer->setSubsetString( subsetString );
}
QgsReadWriteContext context;
context.setPathResolver( QgsProject::instance()->pathResolver() );
context.setProjectTranslator( QgsProject::instance() );
Expand Down
16 changes: 15 additions & 1 deletion src/core/layertree/qgslayertreeutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,8 @@ void QgsLayerTreeUtils::storeOriginalLayersProperties( QgsLayerTreeGroup *group,
const QDomNodeList mlNodeList( doc->documentElement()
.firstChildElement( QStringLiteral( "projectlayers" ) )
.elementsByTagName( QStringLiteral( "maplayer" ) ) );
for ( QgsLayerTreeNode *node : group->children() )

std::function<void ( QgsLayerTreeNode * )> _store = [ & ]( QgsLayerTreeNode * node )
{
if ( QgsLayerTree::isLayer( node ) )
{
Expand All @@ -337,6 +338,19 @@ void QgsLayerTreeUtils::storeOriginalLayersProperties( QgsLayerTreeGroup *group,
}
}
}
else if ( QgsLayerTree::isGroup( node ) )
{
const QList<QgsLayerTreeNode *> constChildren( node->children( ) );
for ( const auto &childNode : constChildren )
{
_store( childNode );
}
}
};

for ( QgsLayerTreeNode *node : group->children() )
{
_store( node );
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/core/qgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1500,8 +1500,8 @@ void QgsVectorLayer::setDataSource( const QString &dataSource, const QString &ba
// Always set crs
setCoordinateSystem();

// reset style if loading default style, style is missing, or geometry type has changed
if ( !renderer() || !legend() || geomType != geometryType() || loadDefaultStyleFlag )
// reset style if loading default style, style is missing, or geometry type is has changed (and layer is valid)
if ( !renderer() || !legend() || ( mValid && geomType != geometryType() ) || loadDefaultStyleFlag )
{
bool defaultLoadedFlag = false;

Expand Down
79 changes: 68 additions & 11 deletions tests/src/python/test_qgsprojectbadlayers.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
QgsMapLayer,
QgsRectangle,
QgsDataProvider,
QgsReadWriteContext,
QgsCoordinateReferenceSystem,
)
from qgis.gui import (QgsLayerTreeMapCanvasBridge,
Expand All @@ -35,6 +36,7 @@
from qgis.PyQt.QtGui import QFont, QColor
from qgis.PyQt.QtTest import QSignalSpy
from qgis.PyQt.QtCore import QT_VERSION_STR, QTemporaryDir, QSize
from qgis.PyQt.QtXml import QDomDocument, QDomNode

from qgis.testing import start_app, unittest
from utilities import (unitTestDataPath, renderMapToImage)
Expand Down Expand Up @@ -68,6 +70,34 @@ def getBaseMapSettings(cls):
ms.setDestinationCrs(crs)
return ms

def _change_data_source(self, layer, datasource, provider_key):
"""Due to the fact that a project r/w context is not available inside
the map layers classes, the original style and subset string restore
happens in app, this function replicates app behavior"""

options = QgsDataProvider.ProviderOptions()

subset_string = ''
if not layer.isValid():
try:
subset_string = layer.dataProvider().subsetString()
except:
pass

layer.setDataSource(datasource, layer.name(), provider_key, options)

if subset_string:
layer.setSubsetString(subset_string)

self.assertTrue(layer.originalXmlProperties(), layer.name())
context = QgsReadWriteContext()
context.setPathResolver(QgsProject.instance().pathResolver())
errorMsg = ''
doc = QDomDocument()
self.assertTrue(doc.setContent(layer.originalXmlProperties()))
layer_node = QDomNode(doc.firstChild())
self.assertTrue(layer.readSymbology(layer_node, errorMsg, context))

def test_project_roundtrip(self):
"""Tests that a project with bad layers can be saved and restored"""

Expand All @@ -91,6 +121,7 @@ def test_project_roundtrip(self):
self.assertTrue(p.write(project_path))

# Re-load the project, checking for the XML properties
p.removeAllMapLayers()
self.assertTrue(p.read(project_path))
vector = list(p.mapLayersByName('lines'))[0]
raster = list(p.mapLayersByName('raster'))[0]
Expand All @@ -109,6 +140,7 @@ def test_project_roundtrip(self):
outfile.write(infile.read().replace('./lines.shp', './lines-BAD_SOURCE.shp').replace('band1_byte_ct_epsg4326_copy.tif', 'band1_byte_ct_epsg4326_copy-BAD_SOURCE.tif'))

# Load the bad project
p.removeAllMapLayers()
self.assertTrue(p.read(bad_project_path))
# Check layer is invalid
vector = list(p.mapLayersByName('lines'))[0]
Expand All @@ -134,6 +166,7 @@ def test_project_roundtrip(self):
outfile.write(infile.read().replace('./lines-BAD_SOURCE.shp', './lines.shp').replace('band1_byte_ct_epsg4326_copy-BAD_SOURCE.tif', 'band1_byte_ct_epsg4326_copy.tif'))

# Load the good project
p.removeAllMapLayers()
self.assertTrue(p.read(good_project_path))
# Check layer is valid
vector = list(p.mapLayersByName('lines'))[0]
Expand All @@ -153,6 +186,7 @@ def test_project_relations(self):

# Load the good project
project_path = os.path.join(temp_dir.path(), 'relation_reference_test.qgs')
p.removeAllMapLayers()
self.assertTrue(p.read(project_path))
point_a = list(p.mapLayersByName('point_a'))[0]
point_b = list(p.mapLayersByName('point_b'))[0]
Expand All @@ -177,6 +211,7 @@ def _check_relations():
outfile.write(infile.read().replace('./relation_reference_test.gpkg', './relation_reference_test-BAD_SOURCE.gpkg'))

# Load the bad project
p.removeAllMapLayers()
self.assertTrue(p.read(bad_project_path))
point_a = list(p.mapLayersByName('point_a'))[0]
point_b = list(p.mapLayersByName('point_b'))[0]
Expand All @@ -197,6 +232,7 @@ def _check_relations():
_check_relations()

# Reload the bad project
p.removeAllMapLayers()
self.assertTrue(p.read(bad_project_path))
point_a = list(p.mapLayersByName('point_a'))[0]
point_b = list(p.mapLayersByName('point_b'))[0]
Expand All @@ -218,6 +254,7 @@ def _check_relations():
outfile.write(infile.read().replace('./relation_reference_test-BAD_SOURCE.gpkg', './relation_reference_test.gpkg'))

# Load the fixed project
p.removeAllMapLayers()
self.assertTrue(p.read(bad_project_path_fixed))
point_a = list(p.mapLayersByName('point_a'))[0]
point_b = list(p.mapLayersByName('point_b'))[0]
Expand All @@ -230,7 +267,6 @@ def _check_relations():
def testStyles(self):
"""Test that styles for rasters and vectors are kept when setDataSource is called"""

options = QgsDataProvider.ProviderOptions()
temp_dir = QTemporaryDir()
p = QgsProject.instance()
for f in (
Expand All @@ -243,50 +279,71 @@ def testStyles(self):

project_path = os.path.join(temp_dir.path(), 'good_layers_test.qgs')
p = QgsProject().instance()
p.removeAllMapLayers()
self.assertTrue(p.read(project_path))
self.assertEqual(p.count(), 3)
self.assertEqual(p.count(), 4)

ms = self.getBaseMapSettings()
point_a_copy = list(p.mapLayersByName('point_a copy'))[0]
point_a = list(p.mapLayersByName('point_a'))[0]
point_b = list(p.mapLayersByName('point_b'))[0]
raster = list(p.mapLayersByName('bad_layer_raster_test'))[0]
self.assertTrue(point_a_copy.isValid())
self.assertTrue(point_a.isValid())
self.assertTrue(point_b.isValid())
self.assertTrue(raster.isValid())
ms.setExtent(QgsRectangle(2.81861, 41.98138, 2.81952, 41.9816))
ms.setLayers([point_a, point_b, raster])
ms.setLayers([point_a_copy, point_a, point_b, raster])
image = renderMapToImage(ms)
print(os.path.join(temp_dir.path(), 'expected.png'))
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'expected.png'), 'PNG'))

point_a_source = point_a.publicSource()
point_b_source = point_b.publicSource()
raster_source = raster.publicSource()
point_a.setDataSource(point_a_source, point_a.name(), 'ogr', options)
point_b.setDataSource(point_b_source, point_b.name(), 'ogr', options)
raster.setDataSource(raster_source, raster.name(), 'gdal', options)
self._change_data_source(point_a, point_a_source, 'ogr')
# Attention: we are not passing the subset string here:
self._change_data_source(point_a_copy, point_a_source, 'ogr')
self._change_data_source(point_b, point_b_source, 'ogr')
self._change_data_source(raster, raster_source, 'gdal')
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'actual.png'), 'PNG'))

self.assertTrue(filecmp.cmp(os.path.join(temp_dir.path(), 'actual.png'), os.path.join(temp_dir.path(), 'expected.png')), False)

# Now build a bad project
p.removeAllMapLayers()
bad_project_path = os.path.join(temp_dir.path(), 'bad_layers_test.qgs')
with open(project_path, 'r') as infile:
with open(bad_project_path, 'w+') as outfile:
outfile.write(infile.read().replace('./bad_layers_test.', './bad_layers_test-BAD_SOURCE.').replace('bad_layer_raster_test.tiff', 'bad_layer_raster_test-BAD_SOURCE.tiff'))

p.removeAllMapLayers()
self.assertTrue(p.read(bad_project_path))
self.assertEqual(p.count(), 3)
self.assertEqual(p.count(), 4)
point_a_copy = list(p.mapLayersByName('point_a copy'))[0]
point_a = list(p.mapLayersByName('point_a'))[0]
point_b = list(p.mapLayersByName('point_b'))[0]
raster = list(p.mapLayersByName('bad_layer_raster_test'))[0]
self.assertFalse(point_a.isValid())
self.assertFalse(point_a_copy.isValid())
self.assertFalse(point_b.isValid())
self.assertFalse(raster.isValid())
ms.setLayers([point_a_copy, point_a, point_b, raster])
image = renderMapToImage(ms)
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'bad.png'), 'PNG'))
self.assertFalse(filecmp.cmp(os.path.join(temp_dir.path(), 'bad.png'), os.path.join(temp_dir.path(), 'expected.png')), False)

self._change_data_source(point_a, point_a_source, 'ogr')
# We are not passing the subset string!!
self._change_data_source(point_a_copy, point_a_source, 'ogr')
self._change_data_source(point_b, point_b_source, 'ogr')
self._change_data_source(raster, raster_source, 'gdal')
self.assertTrue(point_a.isValid())
self.assertTrue(point_a_copy.isValid())
self.assertTrue(point_b.isValid())
self.assertTrue(raster.isValid())

point_a.setDataSource(point_a_source, point_a.name(), 'ogr', options)
point_b.setDataSource(point_b_source, point_b.name(), 'ogr', options)
raster.setDataSource(raster_source, raster.name(), 'gdal', options)
ms.setLayers([point_a_copy, point_a, point_b, raster])
image = renderMapToImage(ms)
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'actual_fixed.png'), 'PNG'))

self.assertTrue(filecmp.cmp(os.path.join(temp_dir.path(), 'actual_fixed.png'), os.path.join(temp_dir.path(), 'expected.png')), False)
Expand Down
Binary file modified tests/testdata/projects/bad_layers_test.gpkg
Binary file not shown.
Loading

0 comments on commit 54a6586

Please sign in to comment.