From d10524d2ff56530397f482d9aa6d8c376bd472af Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 21 Nov 2023 12:21:59 +1000 Subject: [PATCH] Don't try to upload empty vector layers --- felt/core/layer_exporter.py | 42 +++++++++++++++++++++----------- felt/core/map_uploader.py | 26 ++++++++++++++------ felt/plugin.py | 8 +++--- felt/test/test_layer_exporter.py | 8 +++--- 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/felt/core/layer_exporter.py b/felt/core/layer_exporter.py index a36807f..87de317 100644 --- a/felt/core/layer_exporter.py +++ b/felt/core/layer_exporter.py @@ -18,7 +18,10 @@ import zipfile from dataclasses import dataclass from pathlib import Path -from typing import Optional +from typing import ( + Optional, + Tuple +) from qgis.PyQt.QtCore import ( QVariant, @@ -27,7 +30,6 @@ from qgis.PyQt.QtXml import ( QDomDocument ) - from qgis.core import ( QgsFeedback, QgsMapLayer, @@ -53,8 +55,8 @@ ) from .enums import LayerExportResult -from .layer_style import LayerStyle from .exceptions import LayerPackagingException +from .layer_style import LayerStyle from .logger import Logger @@ -87,20 +89,32 @@ def __del__(self): self.temp_dir.cleanup() @staticmethod - def can_export_layer(layer: QgsMapLayer) -> bool: + def can_export_layer(layer: QgsMapLayer) -> Tuple[bool, str]: """ - Returns True if a layer can be exported + Returns True if a layer can be exported, and an explanatory + string if not """ if isinstance(layer, QgsVectorLayer): - return True + # Vector layers must have some features + if layer.featureCount() == 0: + return False, 'Layer is empty' + + return True, '' if isinstance(layer, QgsRasterLayer): - return layer.providerType() in ( - 'gdal', - 'virtualraster' + if layer.providerType() in ( + 'gdal', + 'virtualraster' + ): + return True, '' + + return False, '{} raster layers are not yet supported'.format( + layer.providerType() ) - return False + return False, '{} layers are not yet supported'.format( + layer.__class__.__name__ + ) @staticmethod def representative_layer_style(layer: QgsVectorLayer) -> LayerStyle: @@ -167,8 +181,8 @@ def generate_file_name(self, suffix: str) -> str: """ Generates a temporary file name with the given suffix """ - return (Path(str(self.temp_dir.name)) / ('qgis_export_' + - (uuid.uuid4().hex + suffix))).as_posix() + file_name = 'qgis_export_' + uuid.uuid4().hex + suffix + return (Path(str(self.temp_dir.name)) / file_name).as_posix() def export_layer_for_felt( self, @@ -269,7 +283,7 @@ def export_vector_layer( { 'type': Logger.PACKAGING_VECTOR, 'error': 'Error packaging layer: {}'.format(error_message) - } + } ) raise LayerPackagingException(error_message) @@ -393,7 +407,7 @@ def export_raster_layer( { 'type': Logger.PACKAGING_RASTER, 'error': 'Error packaging layer: {}'.format(error_message) - } + } ) raise LayerPackagingException(error_message) diff --git a/felt/core/map_uploader.py b/felt/core/map_uploader.py index 4332cca..86e71fb 100644 --- a/felt/core/map_uploader.py +++ b/felt/core/map_uploader.py @@ -18,7 +18,8 @@ from pathlib import Path from typing import ( Optional, - List + List, + Tuple ) from qgis.PyQt.QtCore import ( @@ -71,7 +72,7 @@ def __init__(self, ) project = project or QgsProject.instance() - self.unsupported_layers = [] + self.unsupported_layers: List[Tuple[str, str]] = [] if layers: self.current_map_crs = QgsCoordinateReferenceSystem('EPSG:4326') self.current_map_extent = QgsMapLayerUtils.combinedExtent( @@ -101,7 +102,7 @@ def __init__(self, self.layers = [ layer.clone() for layer in visible_layers if - LayerExporter.can_export_layer(layer) + LayerExporter.can_export_layer(layer)[0] ] self._build_unsupported_layer_details(project, visible_layers) @@ -149,11 +150,14 @@ def _build_unsupported_layer_details(self, these to users and to Felt """ unsupported_layer_type_count = defaultdict(int) + unsupported_layer_names = set() for layer in layers: - if LayerExporter.can_export_layer(layer): + can_export, reason = LayerExporter.can_export_layer(layer) + if can_export: continue - self.unsupported_layers.append(layer.name()) + unsupported_layer_names.add(layer.name()) + self.unsupported_layers.append((layer.name(), reason)) if layer.type() == QgsMapLayer.PluginLayer: id_string = layer.pluginLayerType() else: @@ -169,8 +173,8 @@ def _build_unsupported_layer_details(self, for layer_tree_layer in project.layerTreeRoot().findLayers(): if layer_tree_layer.isVisible() and \ not layer_tree_layer.layer() and \ - not layer_tree_layer.name() in self.unsupported_layers: - self.unsupported_layers.append(layer_tree_layer.name()) + not layer_tree_layer.name() in unsupported_layer_names: + self.unsupported_layers.append((layer_tree_layer.name(), '')) def default_map_title(self) -> str: """ @@ -199,7 +203,13 @@ def warning_message(self) -> Optional[str]: msg = '

' + self.tr('The following layers are not supported ' 'and won\'t be uploaded:') + '

' return msg diff --git a/felt/plugin.py b/felt/plugin.py index 38fd177..98a5005 100644 --- a/felt/plugin.py +++ b/felt/plugin.py @@ -241,7 +241,7 @@ def _layer_tree_view_context_menu_about_to_show(self, menu: QMenu): if layer is None: return - if LayerExporter.can_export_layer(layer): + if LayerExporter.can_export_layer(layer)[0]: menus = [action for action in menu.children() if isinstance(action, QMenu) and action.objectName() == 'exportMenu'] @@ -269,5 +269,7 @@ def _update_action_enabled_states(self): has_layers = bool(QgsProject.instance().mapLayers()) is_authorizing = AUTHORIZATION_MANAGER.status == AuthState.Authorizing allowed_to_export = has_layers and not is_authorizing - self.share_map_to_felt_action.setEnabled(allowed_to_export) - self.create_map_action.setEnabled(allowed_to_export) + if self.share_map_to_felt_action: + self.share_map_to_felt_action.setEnabled(allowed_to_export) + if self.create_map_action: + self.create_map_action.setEnabled(allowed_to_export) diff --git a/felt/test/test_layer_exporter.py b/felt/test/test_layer_exporter.py index 15be143..b9e731e 100644 --- a/felt/test/test_layer_exporter.py +++ b/felt/test/test_layer_exporter.py @@ -48,19 +48,21 @@ def test_can_export_layer(self): file = str(TEST_DATA_PATH / 'points.gpkg') layer = QgsVectorLayer(file, 'test') self.assertTrue(layer.isValid()) - self.assertTrue(LayerExporter.can_export_layer(layer)) + self.assertTrue(LayerExporter.can_export_layer(layer)[0]) file = str(TEST_DATA_PATH / 'dem.tif') layer = QgsRasterLayer(file, 'test') self.assertTrue(layer.isValid()) - self.assertTrue(LayerExporter.can_export_layer(layer)) + self.assertTrue(LayerExporter.can_export_layer(layer)[0]) layer = QgsRasterLayer( 'crs=EPSG:3857&format&type=xyz&url=' 'https://tile.openstreetmap.org/%7Bz%7D/%7Bx%7D/%7By%7D.png' '&zmax=19&zmin=0', 'test', 'wms') - self.assertFalse(LayerExporter.can_export_layer(layer)) + can_export, reason = LayerExporter.can_export_layer(layer) + self.assertFalse(can_export) + self.assertEqual(reason, 'wms raster layers are not yet supported') def test_file_name(self): """