From 838ff4c6b475290ee54f10b07115db9791deb4c1 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 21 Nov 2023 12:21:59 +1000 Subject: [PATCH 1/3] Don't try to upload empty vector layers --- felt/core/layer_exporter.py | 34 +++++++++++++++++++++----------- felt/core/map_uploader.py | 26 ++++++++++++++++-------- felt/plugin.py | 8 +++++--- felt/test/test_layer_exporter.py | 8 +++++--- 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/felt/core/layer_exporter.py b/felt/core/layer_exporter.py index 4a56a6b..2605493 100644 --- a/felt/core/layer_exporter.py +++ b/felt/core/layer_exporter.py @@ -61,8 +61,8 @@ ) from .enums import LayerExportResult -from .layer_style import LayerStyle from .exceptions import LayerPackagingException +from .layer_style import LayerStyle from .logger import Logger @@ -108,20 +108,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: @@ -188,8 +200,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, @@ -296,7 +308,7 @@ def export_vector_layer( { 'type': Logger.PACKAGING_VECTOR, '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 eb40562..0ccefb6 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): """ From 6b68ed0922dff85b39ceb18612782e064103300a Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 21 Nov 2023 12:40:51 +1000 Subject: [PATCH 2/3] Report data provider for unsupported layers And ensure we don't include empty layer details in usage reports --- felt/core/__init__.py | 3 ++- felt/core/enums.py | 20 ++++++++++++++++++++ felt/core/layer_exporter.py | 27 ++++++++++++++++++--------- felt/core/map_uploader.py | 13 +++++++++---- felt/plugin.py | 6 ++++-- felt/test/test_layer_exporter.py | 13 ++++++++----- 6 files changed, 61 insertions(+), 21 deletions(-) diff --git a/felt/core/__init__.py b/felt/core/__init__.py index 6d100ad..6df5332 100644 --- a/felt/core/__init__.py +++ b/felt/core/__init__.py @@ -6,7 +6,8 @@ AuthState, ObjectType, LayerExportResult, - UsageType + UsageType, + LayerSupport ) from .auth import OAuthWorkflow # noqa from .map import Map # noqa diff --git a/felt/core/enums.py b/felt/core/enums.py index 0ee429c..f1f5bbe 100644 --- a/felt/core/enums.py +++ b/felt/core/enums.py @@ -55,6 +55,26 @@ class LayerExportResult(Enum): Canceled = auto() +class LayerSupport(Enum): + """ + Reasons why a layer is not supported + """ + Supported = auto() + NotImplementedProvider = auto() + NotImplementedLayerType = auto() + EmptyLayer = auto() + + def should_report(self) -> bool: + """ + Returns True if the layer support should be reported to Felt + usage API + """ + return self not in ( + LayerSupport.Supported, + LayerSupport.EmptyLayer + ) + + class UsageType(Enum): """ Usage types for reporting plugin usage diff --git a/felt/core/layer_exporter.py b/felt/core/layer_exporter.py index 2605493..abe2536 100644 --- a/felt/core/layer_exporter.py +++ b/felt/core/layer_exporter.py @@ -60,7 +60,10 @@ QgsRasterRange ) -from .enums import LayerExportResult +from .enums import ( + LayerExportResult, + LayerSupport +) from .exceptions import LayerPackagingException from .layer_style import LayerStyle from .logger import Logger @@ -108,7 +111,8 @@ def __del__(self): self.temp_dir.cleanup() @staticmethod - def can_export_layer(layer: QgsMapLayer) -> Tuple[bool, str]: + def can_export_layer(layer: QgsMapLayer) \ + -> Tuple[LayerSupport, str]: """ Returns True if a layer can be exported, and an explanatory string if not @@ -116,23 +120,28 @@ def can_export_layer(layer: QgsMapLayer) -> Tuple[bool, str]: if isinstance(layer, QgsVectorLayer): # Vector layers must have some features if layer.featureCount() == 0: - return False, 'Layer is empty' + return LayerSupport.EmptyLayer, 'Layer is empty' - return True, '' + return LayerSupport.Supported, '' if isinstance(layer, QgsRasterLayer): if layer.providerType() in ( 'gdal', 'virtualraster' ): - return True, '' + return LayerSupport.Supported, '' - return False, '{} raster layers are not yet supported'.format( - layer.providerType() + return ( + LayerSupport.NotImplementedProvider, + '{} raster layers are not yet supported'.format( + layer.providerType()) ) - return False, '{} layers are not yet supported'.format( - layer.__class__.__name__ + return ( + LayerSupport.NotImplementedLayerType, + '{} layers are not yet supported'.format( + layer.__class__.__name__ + ) ) @staticmethod diff --git a/felt/core/map_uploader.py b/felt/core/map_uploader.py index 86e71fb..5e169e6 100644 --- a/felt/core/map_uploader.py +++ b/felt/core/map_uploader.py @@ -54,6 +54,7 @@ from .map_utils import MapUtils from .multi_step_feedback import MultiStepFeedback from .s3_upload_parameters import S3UploadParameters +from .enums import LayerSupport class MapUploaderTask(QgsTask): @@ -102,7 +103,8 @@ def __init__(self, self.layers = [ layer.clone() for layer in visible_layers if - LayerExporter.can_export_layer(layer)[0] + LayerExporter.can_export_layer(layer)[ + 0] == LayerSupport.Supported ] self._build_unsupported_layer_details(project, visible_layers) @@ -152,8 +154,8 @@ def _build_unsupported_layer_details(self, unsupported_layer_type_count = defaultdict(int) unsupported_layer_names = set() for layer in layers: - can_export, reason = LayerExporter.can_export_layer(layer) - if can_export: + support, reason = LayerExporter.can_export_layer(layer) + if not support.should_report(): continue unsupported_layer_names.add(layer.name()) @@ -161,7 +163,10 @@ def _build_unsupported_layer_details(self, if layer.type() == QgsMapLayer.PluginLayer: id_string = layer.pluginLayerType() else: - id_string = str(layer.__class__.__name__) + id_string = '{}:{}'.format( + layer.__class__.__name__, + layer.providerType() + ) unsupported_layer_type_count[id_string] = ( unsupported_layer_type_count[id_string] + 1) diff --git a/felt/plugin.py b/felt/plugin.py index 98a5005..11f3e1b 100644 --- a/felt/plugin.py +++ b/felt/plugin.py @@ -40,7 +40,8 @@ from .core import ( AuthState, - LayerExporter + LayerExporter, + LayerSupport ) from .gui import ( @@ -241,7 +242,8 @@ def _layer_tree_view_context_menu_about_to_show(self, menu: QMenu): if layer is None: return - if LayerExporter.can_export_layer(layer)[0]: + if (LayerExporter.can_export_layer(layer)[0] == + LayerSupport.Supported): menus = [action for action in menu.children() if isinstance(action, QMenu) and action.objectName() == 'exportMenu'] diff --git a/felt/test/test_layer_exporter.py b/felt/test/test_layer_exporter.py index 0ccefb6..9025a6f 100644 --- a/felt/test/test_layer_exporter.py +++ b/felt/test/test_layer_exporter.py @@ -30,7 +30,8 @@ from .utilities import get_qgis_app from ..core import ( LayerExporter, - LayerExportResult + LayerExportResult, + LayerSupport ) QGIS_APP = get_qgis_app() @@ -48,20 +49,22 @@ 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)[0]) + self.assertEqual( + LayerExporter.can_export_layer(layer)[0], LayerSupport.Supported) file = str(TEST_DATA_PATH / 'dem.tif') layer = QgsRasterLayer(file, 'test') self.assertTrue(layer.isValid()) - self.assertTrue(LayerExporter.can_export_layer(layer)[0]) + self.assertEqual( + LayerExporter.can_export_layer(layer)[0], LayerSupport.Supported) 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') - can_export, reason = LayerExporter.can_export_layer(layer) - self.assertFalse(can_export) + support, reason = LayerExporter.can_export_layer(layer) + self.assertEqual(support, LayerSupport.NotImplementedProvider) self.assertEqual(reason, 'wms raster layers are not yet supported') def test_file_name(self): From 2bd47489938345d447b188006d3ae7f704b69006 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 21 Nov 2023 13:20:28 +1000 Subject: [PATCH 3/3] Add support for (some) XYZ tile layers Take care to catch unsuported {q} token in layer uris --- felt/core/api_client.py | 34 +++++++++++++++++++ felt/core/layer_exporter.py | 54 +++++++++++++++++++++++++++++- felt/core/map_uploader.py | 52 ++++++++++++++++++++--------- felt/test/test_layer_exporter.py | 57 ++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 17 deletions(-) diff --git a/felt/core/api_client.py b/felt/core/api_client.py index 9241bad..7500424 100644 --- a/felt/core/api_client.py +++ b/felt/core/api_client.py @@ -55,6 +55,7 @@ class FeltApiClient: CREATE_MAP_ENDPOINT = '/maps' CREATE_LAYER_ENDPOINT = '/maps/{}/layers' FINISH_LAYER_ENDPOINT = '/maps/{}/layers/{}/finish_upload' + URL_IMPORT_ENDPOINT = '/maps/{}/layers/url_import' USAGE_ENDPOINT = '/internal/reports' RECENT_MAPS_ENDPOINT = '/maps/recent' @@ -187,6 +188,39 @@ def create_map(self, # pylint: enable=unused-argument + def url_import_to_map(self, + map_id: str, + name: str, + layer_url: str, + blocking: bool = False, + feedback: Optional[QgsFeedback] = None) \ + -> Union[QNetworkReply, QgsNetworkReplyContent]: + """ + Prepares a layer upload + """ + request = self._build_request( + self.URL_IMPORT_ENDPOINT.format(map_id), + {'Content-Type': 'application/json'} + ) + + request_params = { + 'name': name, + 'layer_url': layer_url + } + + json_data = json.dumps(request_params) + if blocking: + return QgsNetworkAccessManager.instance().blockingPost( + request, + json_data.encode(), + feedback=feedback + ) + + return QgsNetworkAccessManager.instance().post( + request, + json_data.encode() + ) + def prepare_layer_upload(self, map_id: str, name: str, diff --git a/felt/core/layer_exporter.py b/felt/core/layer_exporter.py index abe2536..272243f 100644 --- a/felt/core/layer_exporter.py +++ b/felt/core/layer_exporter.py @@ -13,13 +13,15 @@ # This will get replaced with a git SHA1 when you do a git archive __revision__ = '$Format:%H$' +import json +import math import tempfile import uuid import zipfile -import math from dataclasses import dataclass from pathlib import Path from typing import ( + Dict, Optional, List, Tuple @@ -34,6 +36,7 @@ ) from qgis.core import ( + QgsDataSourceUri, QgsFeedback, QgsMapLayer, QgsVectorLayer, @@ -60,6 +63,7 @@ QgsRasterRange ) +from .api_client import API_CLIENT from .enums import ( LayerExportResult, LayerSupport @@ -67,6 +71,7 @@ from .exceptions import LayerPackagingException from .layer_style import LayerStyle from .logger import Logger +from .map import Map @dataclass @@ -131,6 +136,19 @@ def can_export_layer(layer: QgsMapLayer) \ ): return LayerSupport.Supported, '' + if layer.providerType() == 'wms': + ds = QgsDataSourceUri() + ds.setEncodedUri(layer.source()) + if ds.param('type') == 'xyz': + url = ds.param('url') + if '{q}' in url: + return ( + LayerSupport.NotImplementedProvider, + '{q} token in XYZ tile layers not supported' + ) + + return LayerSupport.Supported, '' + return ( LayerSupport.NotImplementedProvider, '{} raster layers are not yet supported'.format( @@ -144,6 +162,40 @@ def can_export_layer(layer: QgsMapLayer) \ ) ) + @staticmethod + def layer_import_url(layer: QgsMapLayer) -> Optional[str]: + """ + Returns the layer URL if the URL import method should be used to add + a layer + """ + if isinstance(layer, QgsRasterLayer): + if layer.providerType() == 'wms': + ds = QgsDataSourceUri() + ds.setEncodedUri(layer.source()) + if ds.param('type') == 'xyz': + url = ds.param('url') + if '{q}' not in url: + return url + + return None + + @staticmethod + def import_from_url(layer: QgsMapLayer, target_map: Map, + feedback: Optional[QgsFeedback] = None) -> Dict: + """ + Imports a layer from URI to the given map + """ + layer_url = LayerExporter.layer_import_url(layer) + + reply = API_CLIENT.url_import_to_map( + map_id=target_map.id, + name=layer.name(), + layer_url=layer_url, + blocking=True, + feedback=feedback + ) + return json.loads(reply.content().data().decode()) + @staticmethod def representative_layer_style(layer: QgsVectorLayer) -> LayerStyle: """ diff --git a/felt/core/map_uploader.py b/felt/core/map_uploader.py index 5e169e6..69ed9d1 100644 --- a/felt/core/map_uploader.py +++ b/felt/core/map_uploader.py @@ -308,27 +308,47 @@ def run(self): if self.isCanceled(): return False - self.status_changed.emit( - self.tr('Exporting {}').format(layer.name()) - ) - try: - result = exporter.export_layer_for_felt( + if LayerExporter.layer_import_url(layer): + result = LayerExporter.import_from_url( layer, - multi_step_feedback - ) - except LayerPackagingException as e: + self.associated_map, + multi_step_feedback) + + if 'errors' in result: + self.error_string = self.tr( + 'Error occurred while exporting layer {}: {}').format( + layer.name(), + result['errors'][0]['detail'] + ) + self.status_changed.emit(self.error_string) + + return False + layer.moveToThread(None) - self.error_string = self.tr( - 'Error occurred while exporting layer {}: {}').format( - layer.name(), - e + else: + + self.status_changed.emit( + self.tr('Exporting {}').format(layer.name()) ) - self.status_changed.emit(self.error_string) + try: + result = exporter.export_layer_for_felt( + layer, + multi_step_feedback + ) + except LayerPackagingException as e: + layer.moveToThread(None) + self.error_string = self.tr( + 'Error occurred while exporting layer {}: {}').format( + layer.name(), + e + ) + self.status_changed.emit(self.error_string) - return False + return False + + layer.moveToThread(None) + to_upload[layer] = result - layer.moveToThread(None) - to_upload[layer] = result multi_step_feedback.step_finished() if self.isCanceled(): diff --git a/felt/test/test_layer_exporter.py b/felt/test/test_layer_exporter.py index 9025a6f..02d581e 100644 --- a/felt/test/test_layer_exporter.py +++ b/felt/test/test_layer_exporter.py @@ -64,9 +64,66 @@ def test_can_export_layer(self): '&zmax=19&zmin=0', 'test', 'wms') support, reason = LayerExporter.can_export_layer(layer) + self.assertEqual(support, LayerSupport.Supported) + + layer = QgsRasterLayer( + 'http-header:referer=&type=xyz&url=http://ecn.t3.tiles.' + 'virtualearth.net/tiles/a%7Bq%7D.jpeg?g%3D1&zmax=18&zmin=0', + 'test', 'wms') + support, reason = LayerExporter.can_export_layer(layer) + self.assertEqual(support, LayerSupport.NotImplementedProvider) + self.assertEqual(reason, '{q} token in XYZ tile layers not supported') + + layer = QgsRasterLayer( + 'crs=EPSG:4326&dpiMode=7&format=image/png&layers=' + 'wofs_summary_clear&styles&' + 'tilePixelRatio=0&url=https://ows.dea.ga.gov.au/', + 'test', 'wms') + support, reason = LayerExporter.can_export_layer(layer) self.assertEqual(support, LayerSupport.NotImplementedProvider) self.assertEqual(reason, 'wms raster layers are not yet supported') + def test_use_url_import_method(self): + """ + Test determining if layers should use the URL import method + """ + file = str(TEST_DATA_PATH / 'points.gpkg') + layer = QgsVectorLayer(file, 'test') + self.assertTrue(layer.isValid()) + self.assertFalse( + LayerExporter.layer_import_url(layer)) + + file = str(TEST_DATA_PATH / 'dem.tif') + layer = QgsRasterLayer(file, 'test') + self.assertTrue(layer.isValid()) + self.assertFalse( + LayerExporter.layer_import_url(layer)) + + 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.assertEqual( + LayerExporter.layer_import_url(layer), + 'https://tile.openstreetmap.org/{z}/{x}/{y}.png' + ) + + layer = QgsRasterLayer( + 'http-header:referer=&type=xyz&url=http://ecn.t3.tiles.' + 'virtualearth.net/tiles/a%7Bq%7D.jpeg?g%3D1&zmax=18&zmin=0', + 'test', 'wms') + self.assertFalse( + LayerExporter.layer_import_url(layer)) + + layer = QgsRasterLayer( + 'crs=EPSG:4326&dpiMode=7&format=image/png&layers=' + 'wofs_summary_clear&styles&tilePixelRatio=0' + '&url=https://ows.dea.ga.gov.au/', + 'test', 'wms') + self.assertFalse( + LayerExporter.layer_import_url(layer)) + def test_file_name(self): """ Test building temporary file names