Skip to content

Commit 54a6586

Browse files
elpasoelpaso
authoredNov 6, 2018
Merge pull request #8427 from elpaso/handle-bad-layers3
Handle bad layers: fixes for groups and subset string
·
ltr-3_40final-3_6_0
2 parents 5ae2bae + 9af0719 commit 54a6586

File tree

7 files changed

+1255
-685
lines changed

7 files changed

+1255
-685
lines changed
 

‎src/app/qgisapp.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6951,10 +6951,25 @@ void QgisApp::changeDataSource( QgsMapLayer *layer )
69516951
if ( uri.isValid() )
69526952
{
69536953
bool layerIsValid( layer->isValid() );
6954+
// Store subset string form vlayer if we are fixing a bad layer
6955+
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
6956+
QString subsetString;
6957+
// Get the subset string directly from the data provider because
6958+
// layer's method will return a null string from invalid layers
6959+
if ( !layerIsValid && vlayer && vlayer->dataProvider() &&
6960+
vlayer->dataProvider()->supportsSubsetString() &&
6961+
!vlayer->dataProvider()->subsetString( ).isEmpty() )
6962+
{
6963+
subsetString = vlayer->dataProvider()->subsetString();
6964+
}
69546965
layer->setDataSource( uri.uri, layer->name(), uri.providerKey, QgsDataProvider::ProviderOptions() );
6955-
// Re-apply style
6966+
// Re-apply original style and subset string when fixing bad layers
69566967
if ( !( layerIsValid || layer->originalXmlProperties().isEmpty() ) )
69576968
{
6969+
if ( ! subsetString.isEmpty() )
6970+
{
6971+
vlayer->setSubsetString( subsetString );
6972+
}
69586973
QgsReadWriteContext context;
69596974
context.setPathResolver( QgsProject::instance()->pathResolver() );
69606975
context.setProjectTranslator( QgsProject::instance() );

‎src/core/layertree/qgslayertreeutils.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,8 @@ void QgsLayerTreeUtils::storeOriginalLayersProperties( QgsLayerTreeGroup *group,
311311
const QDomNodeList mlNodeList( doc->documentElement()
312312
.firstChildElement( QStringLiteral( "projectlayers" ) )
313313
.elementsByTagName( QStringLiteral( "maplayer" ) ) );
314-
for ( QgsLayerTreeNode *node : group->children() )
314+
315+
std::function<void ( QgsLayerTreeNode * )> _store = [ & ]( QgsLayerTreeNode * node )
315316
{
316317
if ( QgsLayerTree::isLayer( node ) )
317318
{
@@ -337,6 +338,19 @@ void QgsLayerTreeUtils::storeOriginalLayersProperties( QgsLayerTreeGroup *group,
337338
}
338339
}
339340
}
341+
else if ( QgsLayerTree::isGroup( node ) )
342+
{
343+
const QList<QgsLayerTreeNode *> constChildren( node->children( ) );
344+
for ( const auto &childNode : constChildren )
345+
{
346+
_store( childNode );
347+
}
348+
}
349+
};
350+
351+
for ( QgsLayerTreeNode *node : group->children() )
352+
{
353+
_store( node );
340354
}
341355
}
342356

‎src/core/qgsvectorlayer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,8 +1500,8 @@ void QgsVectorLayer::setDataSource( const QString &dataSource, const QString &ba
15001500
// Always set crs
15011501
setCoordinateSystem();
15021502

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

‎tests/src/python/test_qgsprojectbadlayers.py

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
QgsMapLayer,
2828
QgsRectangle,
2929
QgsDataProvider,
30+
QgsReadWriteContext,
3031
QgsCoordinateReferenceSystem,
3132
)
3233
from qgis.gui import (QgsLayerTreeMapCanvasBridge,
@@ -35,6 +36,7 @@
3536
from qgis.PyQt.QtGui import QFont, QColor
3637
from qgis.PyQt.QtTest import QSignalSpy
3738
from qgis.PyQt.QtCore import QT_VERSION_STR, QTemporaryDir, QSize
39+
from qgis.PyQt.QtXml import QDomDocument, QDomNode
3840

3941
from qgis.testing import start_app, unittest
4042
from utilities import (unitTestDataPath, renderMapToImage)
@@ -68,6 +70,34 @@ def getBaseMapSettings(cls):
6870
ms.setDestinationCrs(crs)
6971
return ms
7072

73+
def _change_data_source(self, layer, datasource, provider_key):
74+
"""Due to the fact that a project r/w context is not available inside
75+
the map layers classes, the original style and subset string restore
76+
happens in app, this function replicates app behavior"""
77+
78+
options = QgsDataProvider.ProviderOptions()
79+
80+
subset_string = ''
81+
if not layer.isValid():
82+
try:
83+
subset_string = layer.dataProvider().subsetString()
84+
except:
85+
pass
86+
87+
layer.setDataSource(datasource, layer.name(), provider_key, options)
88+
89+
if subset_string:
90+
layer.setSubsetString(subset_string)
91+
92+
self.assertTrue(layer.originalXmlProperties(), layer.name())
93+
context = QgsReadWriteContext()
94+
context.setPathResolver(QgsProject.instance().pathResolver())
95+
errorMsg = ''
96+
doc = QDomDocument()
97+
self.assertTrue(doc.setContent(layer.originalXmlProperties()))
98+
layer_node = QDomNode(doc.firstChild())
99+
self.assertTrue(layer.readSymbology(layer_node, errorMsg, context))
100+
71101
def test_project_roundtrip(self):
72102
"""Tests that a project with bad layers can be saved and restored"""
73103

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

93123
# Re-load the project, checking for the XML properties
124+
p.removeAllMapLayers()
94125
self.assertTrue(p.read(project_path))
95126
vector = list(p.mapLayersByName('lines'))[0]
96127
raster = list(p.mapLayersByName('raster'))[0]
@@ -109,6 +140,7 @@ def test_project_roundtrip(self):
109140
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'))
110141

111142
# Load the bad project
143+
p.removeAllMapLayers()
112144
self.assertTrue(p.read(bad_project_path))
113145
# Check layer is invalid
114146
vector = list(p.mapLayersByName('lines'))[0]
@@ -134,6 +166,7 @@ def test_project_roundtrip(self):
134166
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'))
135167

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

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

179213
# Load the bad project
214+
p.removeAllMapLayers()
180215
self.assertTrue(p.read(bad_project_path))
181216
point_a = list(p.mapLayersByName('point_a'))[0]
182217
point_b = list(p.mapLayersByName('point_b'))[0]
@@ -197,6 +232,7 @@ def _check_relations():
197232
_check_relations()
198233

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

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

233-
options = QgsDataProvider.ProviderOptions()
234270
temp_dir = QTemporaryDir()
235271
p = QgsProject.instance()
236272
for f in (
@@ -243,50 +279,71 @@ def testStyles(self):
243279

244280
project_path = os.path.join(temp_dir.path(), 'good_layers_test.qgs')
245281
p = QgsProject().instance()
282+
p.removeAllMapLayers()
246283
self.assertTrue(p.read(project_path))
247-
self.assertEqual(p.count(), 3)
284+
self.assertEqual(p.count(), 4)
248285

249286
ms = self.getBaseMapSettings()
287+
point_a_copy = list(p.mapLayersByName('point_a copy'))[0]
250288
point_a = list(p.mapLayersByName('point_a'))[0]
251289
point_b = list(p.mapLayersByName('point_b'))[0]
252290
raster = list(p.mapLayersByName('bad_layer_raster_test'))[0]
291+
self.assertTrue(point_a_copy.isValid())
253292
self.assertTrue(point_a.isValid())
254293
self.assertTrue(point_b.isValid())
255294
self.assertTrue(raster.isValid())
256295
ms.setExtent(QgsRectangle(2.81861, 41.98138, 2.81952, 41.9816))
257-
ms.setLayers([point_a, point_b, raster])
296+
ms.setLayers([point_a_copy, point_a, point_b, raster])
258297
image = renderMapToImage(ms)
259-
print(os.path.join(temp_dir.path(), 'expected.png'))
260298
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'expected.png'), 'PNG'))
261299

262300
point_a_source = point_a.publicSource()
263301
point_b_source = point_b.publicSource()
264302
raster_source = raster.publicSource()
265-
point_a.setDataSource(point_a_source, point_a.name(), 'ogr', options)
266-
point_b.setDataSource(point_b_source, point_b.name(), 'ogr', options)
267-
raster.setDataSource(raster_source, raster.name(), 'gdal', options)
303+
self._change_data_source(point_a, point_a_source, 'ogr')
304+
# Attention: we are not passing the subset string here:
305+
self._change_data_source(point_a_copy, point_a_source, 'ogr')
306+
self._change_data_source(point_b, point_b_source, 'ogr')
307+
self._change_data_source(raster, raster_source, 'gdal')
268308
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'actual.png'), 'PNG'))
269309

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

272312
# Now build a bad project
313+
p.removeAllMapLayers()
273314
bad_project_path = os.path.join(temp_dir.path(), 'bad_layers_test.qgs')
274315
with open(project_path, 'r') as infile:
275316
with open(bad_project_path, 'w+') as outfile:
276317
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'))
277318

319+
p.removeAllMapLayers()
278320
self.assertTrue(p.read(bad_project_path))
279-
self.assertEqual(p.count(), 3)
321+
self.assertEqual(p.count(), 4)
322+
point_a_copy = list(p.mapLayersByName('point_a copy'))[0]
280323
point_a = list(p.mapLayersByName('point_a'))[0]
281324
point_b = list(p.mapLayersByName('point_b'))[0]
282325
raster = list(p.mapLayersByName('bad_layer_raster_test'))[0]
283326
self.assertFalse(point_a.isValid())
327+
self.assertFalse(point_a_copy.isValid())
284328
self.assertFalse(point_b.isValid())
285329
self.assertFalse(raster.isValid())
330+
ms.setLayers([point_a_copy, point_a, point_b, raster])
331+
image = renderMapToImage(ms)
332+
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'bad.png'), 'PNG'))
333+
self.assertFalse(filecmp.cmp(os.path.join(temp_dir.path(), 'bad.png'), os.path.join(temp_dir.path(), 'expected.png')), False)
334+
335+
self._change_data_source(point_a, point_a_source, 'ogr')
336+
# We are not passing the subset string!!
337+
self._change_data_source(point_a_copy, point_a_source, 'ogr')
338+
self._change_data_source(point_b, point_b_source, 'ogr')
339+
self._change_data_source(raster, raster_source, 'gdal')
340+
self.assertTrue(point_a.isValid())
341+
self.assertTrue(point_a_copy.isValid())
342+
self.assertTrue(point_b.isValid())
343+
self.assertTrue(raster.isValid())
286344

287-
point_a.setDataSource(point_a_source, point_a.name(), 'ogr', options)
288-
point_b.setDataSource(point_b_source, point_b.name(), 'ogr', options)
289-
raster.setDataSource(raster_source, raster.name(), 'gdal', options)
345+
ms.setLayers([point_a_copy, point_a, point_b, raster])
346+
image = renderMapToImage(ms)
290347
self.assertTrue(image.save(os.path.join(temp_dir.path(), 'actual_fixed.png'), 'PNG'))
291348

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

‎tests/testdata/projects/bad_layers_test.gpkg

0 Bytes
Binary file not shown.

‎tests/testdata/projects/bad_layers_test.qgs

Lines changed: 577 additions & 335 deletions
Large diffs are not rendered by default.

‎tests/testdata/projects/good_layers_test.qgs

Lines changed: 577 additions & 335 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)
Please sign in to comment.