From 67b229c87ce2b30d2879240445453b72dcd99377 Mon Sep 17 00:00:00 2001 From: index-git Date: Fri, 6 Jan 2023 11:37:45 +0100 Subject: [PATCH 01/10] Start to develop version v1.20.0 --- CHANGELOG.md | 8 ++++++++ version.txt | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 964be7df5..d1c3840e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## v1.20.0 + {release-date} +### Upgrade requirements +### Migrations and checks +#### Schema migrations +#### Data migrations +### Changes + ## v1.19.0 2023-01-11 ### Upgrade requirements diff --git a/version.txt b/version.txt index 5d6da8945..ba901f783 100644 --- a/version.txt +++ b/version.txt @@ -1,2 +1,2 @@ -v1.19.0 -2023-01-11T11:00:00Z +v1.20.0-dev +2023-01-11T11:00:01Z From 99747ac1de4b0732807549535091e2e1bddf7138 Mon Sep 17 00:00:00 2001 From: index-git Date: Fri, 6 Jan 2023 09:30:13 +0100 Subject: [PATCH 02/10] Pass file_type as param to clear_publication_info --- src/layman/layer/util.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/layman/layer/util.py b/src/layman/layer/util.py index 070237574..9228a7666 100644 --- a/src/layman/layer/util.py +++ b/src/layman/layer/util.py @@ -87,9 +87,9 @@ def get_layer_info(workspace, layername, context=None): return filled_partial_info -def clear_publication_info(layer_info): +def clear_publication_info(layer_info, file_type): clear_info = common_clear_publication_info(layer_info) - if clear_info['file']['file_type'] != settings.FILE_TYPE_RASTER: + if file_type != settings.FILE_TYPE_RASTER: clear_info.pop('image_mosaic') return clear_info @@ -120,7 +120,7 @@ def get_complete_layer_info(workspace=None, layername=None, cached=False): complete_info.update(partial_info) complete_info['sld'] = complete_info['style'] - complete_info = clear_publication_info(complete_info) + complete_info = clear_publication_info(complete_info, file_type) complete_info.pop('layman_metadata') complete_info['layman_metadata'] = {'publication_status': layman_util.get_publication_status(workspace, LAYER_TYPE, layername, From 507e1062ad4c3c2d1204f629a5a76cce4e205e6c Mon Sep 17 00:00:00 2001 From: index-git Date: Thu, 5 Jan 2023 11:03:10 +0100 Subject: [PATCH 03/10] Add db_connection param to POST Workspace Layers --- CHANGELOG.md | 1 + doc/rest.md | 8 ++-- src/layman/layer/filesystem/util.py | 3 ++ src/layman/layer/rest_workspace_layers.py | 11 ++--- src/layman/layer/util.py | 2 +- test_tools/process_client.py | 7 ++- .../layer_external_db/__init__.py | 0 .../layer_external_db/external_db_test.py | 47 +++++++++++++++++++ 8 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 tests/dynamic_data/publications/layer_external_db/__init__.py create mode 100644 tests/dynamic_data/publications/layer_external_db/external_db_test.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d1c3840e3..0b4a60ac0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ #### Schema migrations #### Data migrations ### Changes +- [#703](https://github.com/LayerManager/layman/issues/703) Endpoint [POST Workspace Layers](doc/rest.md#post-workspace-layers) support new body parameter *db_connection*. ## v1.19.0 2023-01-11 diff --git a/doc/rest.md b/doc/rest.md index 95dd8376f..22e6d466d 100644 --- a/doc/rest.md +++ b/doc/rest.md @@ -88,9 +88,9 @@ Have the same request parameters and response structure and headers as [GET Laye Publish vector or raster data file as new layer of WMS, in case of vector also new feature type of WFS. Processing chain consists of few steps: -- save file to workspace directory within Layman data directory +- save files (if sent) to workspace directory within Layman data directory - save basic information (name, title, access_rights) into PostgreSQL -- for vector layers import the vector file to PostgreSQL database as new table into workspace schema +- for vector layers import vector file (if sent) to PostgreSQL database as new table into workspace schema - files with invalid byte sequence are first converted to GeoJSON, then cleaned with iconv, and finally imported to database. - for raster layers normalize and compress raster file to GeoTIFF with overviews (pyramids); NoData values are normalized as transparent - for vector layers publish the vector table as new layer (feature type) within appropriate WFS workspaces of GeoServer @@ -121,7 +121,8 @@ Check [Asynchronous file upload](async-file-upload.md) example. Content-Type: `multipart/form-data`, `application/x-www-form-urlencoded` Body parameters: -- **file**, file(s) or file name(s) +- *file*, file(s) or file name(s) + - exactly one of `file` or `db_connection` must be set - one of following options is expected: - GeoJSON file - ShapeFile files (at least three files: .shp, .shx, .dbf) @@ -143,6 +144,7 @@ Body parameters: - if published file has empty bounding box (i.e. no features), its bounding box on WMS/WFS endpoint is set to the whole World - attribute names are [laundered](https://gdal.org/drivers/vector/pg.html#layer-creation-options) to be safely stored in DB - if QML style is used in this request, it must list all attributes contained in given data file +- *db_connection*, string - *name*, string - computer-friendly identifier of the layer - must be unique among all layers of one workspace diff --git a/src/layman/layer/filesystem/util.py b/src/layman/layer/filesystem/util.py index 0d09559ed..605bfe64b 100644 --- a/src/layman/layer/filesystem/util.py +++ b/src/layman/layer/filesystem/util.py @@ -33,6 +33,9 @@ def __init__(self, *, sent_streams=None, sent_paths=None, saved_paths=None): object.__setattr__(self, '_sent_paths', sent_paths) object.__setattr__(self, '_saved_paths', saved_paths) + def __bool__(self): + return bool(self.raw_paths) + @property def sent_streams(self): # pylint: disable=no-member diff --git a/src/layman/layer/rest_workspace_layers.py b/src/layman/layer/rest_workspace_layers.py index 2dfe69692..548b6f896 100644 --- a/src/layman/layer/rest_workspace_layers.py +++ b/src/layman/layer/rest_workspace_layers.py @@ -53,8 +53,7 @@ def post(workspace): if len(filename) > 0 ] input_files = fs_util.InputFiles(sent_streams=sent_file_streams, sent_paths=sent_file_paths) - if len(input_files.raw_paths) == 0: - raise LaymanError(1, {'parameter': 'file'}) + db_connection = request.form.get('db_connection', '') # NAME unsafe_layername = request.form.get('name', '') @@ -91,13 +90,13 @@ def post(workspace): enable_more_main_files = time_regex is not None # FILE NAMES - use_chunk_upload = not input_files.sent_streams - if not (use_chunk_upload and input_files.is_one_archive): + use_chunk_upload = bool(input_files.sent_paths) + if not (use_chunk_upload and input_files.is_one_archive) and input_files: input_file.check_filenames(workspace, layername, input_files, check_crs, enable_more_main_files=enable_more_main_files, time_regex=time_regex, slugified_time_regex=slugified_time_regex, name_input_file_by_layer=name_input_file_by_layer) - file_type = input_file.get_file_type(input_files.raw_or_archived_main_file_path) + file_type = input_file.get_file_type(input_files.raw_or_archived_main_file_path) if not db_connection else settings.FILE_TYPE_VECTOR # TITLE if len(request.form.get('title', '')) > 0: @@ -181,7 +180,7 @@ def post(workspace): task_options.update({ 'check_crs': check_crs, }) - else: + elif input_files: try: input_file.save_layer_files(workspace, layername, input_files, check_crs, overview_resampling, name_input_file_by_layer=name_input_file_by_layer) except BaseException as exc: diff --git a/src/layman/layer/util.py b/src/layman/layer/util.py index 9228a7666..6f874ad74 100644 --- a/src/layman/layer/util.py +++ b/src/layman/layer/util.py @@ -103,7 +103,7 @@ def get_complete_layer_info(workspace=None, layername=None, cached=False): if not any(partial_info): raise LaymanError(15, {'layername': layername}) - file_type = partial_info['file']['file_type'] + file_type = partial_info['_file_type'] item_keys = get_layer_info_keys(file_type) complete_info = dict() diff --git a/test_tools/process_client.py b/test_tools/process_client.py index 279681938..a7364ce44 100644 --- a/test_tools/process_client.py +++ b/test_tools/process_client.py @@ -303,6 +303,7 @@ def publish_workspace_publication(publication_type, name, *, file_paths=None, + db_connection=None, headers=None, access_rights=None, title=None, @@ -324,13 +325,13 @@ def publish_workspace_publication(publication_type, headers = headers or {} publication_type_def = PUBLICATION_TYPES_DEF[publication_type] - # map layers must not be set together with file_paths assert not map_layers or not file_paths + assert not map_layers or not db_connection assert not (not with_chunks and do_not_upload_chunks) assert not (check_response_fn and do_not_upload_chunks) # because check_response_fn is not called when do_not_upload_chunks - file_paths = [publication_type_def.source_path] if file_paths is None and not map_layers else file_paths + file_paths = [publication_type_def.source_path] if file_paths is None and db_connection is None and not map_layers else file_paths if style_file or with_chunks or compress or compress_settings or overview_resampling: assert publication_type == LAYER_TYPE @@ -384,6 +385,8 @@ def publish_workspace_publication(publication_type, data['overview_resampling'] = overview_resampling if time_regex: data['time_regex'] = time_regex + if db_connection: + data['db_connection'] = db_connection response = requests.post(r_url, files=files, data=data, diff --git a/tests/dynamic_data/publications/layer_external_db/__init__.py b/tests/dynamic_data/publications/layer_external_db/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/dynamic_data/publications/layer_external_db/external_db_test.py b/tests/dynamic_data/publications/layer_external_db/external_db_test.py new file mode 100644 index 000000000..215f84dfb --- /dev/null +++ b/tests/dynamic_data/publications/layer_external_db/external_db_test.py @@ -0,0 +1,47 @@ +import os +import pytest + +from test_tools import process_client +from tests import EnumTestTypes, Publication, EnumTestKeys +from tests.dynamic_data import base_test + +DIRECTORY = os.path.dirname(os.path.abspath(__file__)) + +pytest_generate_tests = base_test.pytest_generate_tests + +TEST_CASES = { + 'external_vector_sld': { + 'rest_params': { + 'file_paths': [], + 'db_connection': 'db_connection_string', + }, + }, +} + + +class TestLayer(base_test.TestSingleRestPublication): + + workspace = 'dynamic_test_workspace_layer_external_db' + + publication_type = process_client.LAYER_TYPE + + test_cases = [base_test.TestCaseType(key=key, + type=value.get(EnumTestKeys.TYPE, EnumTestTypes.MANDATORY), + params=value, + marks=[pytest.mark.xfail(reason="Not yet implemented.")] + if value.get('xfail') else [] + ) for key, value in TEST_CASES.items()] + + rest_parametrization = { + 'method': [ + base_test.RestMethodType('post_publication', 'post'), + ], + } + + # pylint: disable=unused-argument + @staticmethod + def test_style_xml(layer: Publication, key, params, rest_method): + """Parametrized using pytest_generate_tests""" + rest_method(layer, params={ + **params['rest_params'] + }) From 2d29f6eb88efbb22974022479b083e96a3a25a0c Mon Sep 17 00:00:00 2001 From: index-git Date: Fri, 6 Jan 2023 08:55:53 +0100 Subject: [PATCH 04/10] Validate POST combination of file and db_connection parameters --- doc/rest.md | 1 + src/layman/layer/rest_workspace_layers.py | 15 +++++ .../publications/wrong_input/__init__.py | 59 +++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/doc/rest.md b/doc/rest.md index 22e6d466d..4bd4b4614 100644 --- a/doc/rest.md +++ b/doc/rest.md @@ -145,6 +145,7 @@ Body parameters: - attribute names are [laundered](https://gdal.org/drivers/vector/pg.html#layer-creation-options) to be safely stored in DB - if QML style is used in this request, it must list all attributes contained in given data file - *db_connection*, string + - exactly one of `file` or `db_connection` must be set - *name*, string - computer-friendly identifier of the layer - must be unique among all layers of one workspace diff --git a/src/layman/layer/rest_workspace_layers.py b/src/layman/layer/rest_workspace_layers.py index 548b6f896..fc50a6357 100644 --- a/src/layman/layer/rest_workspace_layers.py +++ b/src/layman/layer/rest_workspace_layers.py @@ -54,6 +54,21 @@ def post(workspace): ] input_files = fs_util.InputFiles(sent_streams=sent_file_streams, sent_paths=sent_file_paths) db_connection = request.form.get('db_connection', '') + if not input_files and not db_connection: + raise LaymanError(1, { + 'parameters': ['file', 'db_connection'], + 'message': 'Both `file` and `db_connection` parameters are empty', + 'expected': 'One of the parameters is filled.', + }) + if input_files and db_connection: + raise LaymanError(48, { + 'parameters': ['file', 'db_connection'], + 'message': 'Both `file` and `db_connection` parameters are filled', + 'expected': 'Only one of the parameters is fulfilled.', + 'found': { + 'file': input_files.raw_paths, + 'db_connection': db_connection, + }}) # NAME unsafe_layername = request.form.get('name', '') diff --git a/tests/dynamic_data/publications/wrong_input/__init__.py b/tests/dynamic_data/publications/wrong_input/__init__.py index b8b8cdd22..778e780ae 100644 --- a/tests/dynamic_data/publications/wrong_input/__init__.py +++ b/tests/dynamic_data/publications/wrong_input/__init__.py @@ -1334,6 +1334,65 @@ }, }, }, + 'none_file_none_db_connect': { + KEY_PUBLICATION_TYPE: process_client.LAYER_TYPE, + KEY_ACTION_PARAMS: { + 'file_paths': [], + 'db_connection': '', + 'compress': False, + 'with_chunks': False, + }, + consts.KEY_EXCEPTION: LaymanError, + KEY_EXPECTED_EXCEPTION: { + KEY_DEFAULT: {'http_code': 400, + 'sync': True, + 'code': 1, + 'message': 'Missing parameter', + 'detail': { + 'parameters': ['file', 'db_connection'], + 'message': 'Both `file` and `db_connection` parameters are empty', + 'expected': 'One of the parameters is filled.', + }, + }, + }, + }, + 'file_and_db_connect': { + KEY_PUBLICATION_TYPE: process_client.LAYER_TYPE, + KEY_ACTION_PARAMS: { + 'file_paths': ['sample/layman.layer/small_layer.geojson'], + 'db_connection': 'postgresql://username:password@host:port/dbname?table=table_name&geo_column=geo_column_name', + }, + consts.KEY_EXCEPTION: LaymanError, + KEY_EXPECTED_EXCEPTION: { + KEY_DEFAULT: {'http_code': 400, + 'sync': True, + 'code': 48, + 'message': 'Wrong combination of parameters', + 'detail': { + 'parameters': ['file', 'db_connection'], + 'message': 'Both `file` and `db_connection` parameters are filled', + 'expected': 'Only one of the parameters is fulfilled.', + 'found': { + 'file': ['small_layer.geojson'], + 'db_connection': 'postgresql://username:password@host:port/dbname?table=table_name&geo_column=geo_column_name', + }}, + }, + frozenset([('compress', True), ('with_chunks', False)]): { + 'detail': { + 'found': { + 'file': ['temporary_zip_file.zip'], + }, + }, + }, + frozenset([('compress', True), ('with_chunks', True)]): { + 'detail': { + 'found': { + 'file': ['temporary_zip_file.zip'], + }, + }, + }, + }, + }, } VALIDATION_PATCH_ACTION = { From b185bb318c17dafb4365f05b6fb1823a82220af1 Mon Sep 17 00:00:00 2001 From: index-git Date: Thu, 5 Jan 2023 16:46:14 +0100 Subject: [PATCH 05/10] Delete duplicated test --- src/layman/layer/rest_workspace_test.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/layman/layer/rest_workspace_test.py b/src/layman/layer/rest_workspace_test.py index d8efd0456..6d02d487a 100644 --- a/src/layman/layer/rest_workspace_test.py +++ b/src/layman/layer/rest_workspace_test.py @@ -166,16 +166,6 @@ def test_wrong_value_of_layername(client): assert resp_json['detail']['parameter'] == 'layername' -@pytest.mark.usefixtures('app_context', 'ensure_layman') -def test_no_file(client): - response = client.post(url_for('rest_workspace_layers.post', workspace='testuser1')) - assert response.status_code == 400 - resp_json = response.get_json() - # print('resp_json', resp_json) - assert resp_json['code'] == 1 - assert resp_json['detail']['parameter'] == 'file' - - @pytest.mark.usefixtures('app_context', 'ensure_layman') def test_workspace_schema_conflict(client): if len(settings.PG_NON_USER_SCHEMAS) == 0: From 324cf5e51180f6f7dbe2ef1029b594066c0a1a2c Mon Sep 17 00:00:00 2001 From: index-git Date: Fri, 6 Jan 2023 15:46:55 +0100 Subject: [PATCH 06/10] Rename test file --- src/db/{db_test.py => util_test.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/db/{db_test.py => util_test.py} (100%) diff --git a/src/db/db_test.py b/src/db/util_test.py similarity index 100% rename from src/db/db_test.py rename to src/db/util_test.py From 884f90cf320d6d379eeaf8c3c73c773fed312abe Mon Sep 17 00:00:00 2001 From: index-git Date: Fri, 6 Jan 2023 16:35:21 +0100 Subject: [PATCH 07/10] Create util to parse db_connection param --- src/db/__init__.py | 10 ++++++++ src/layman/layer/util.py | 14 ++++++++++- src/layman/layer/util_test.py | 44 +++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/db/__init__.py b/src/db/__init__.py index a4a212e22..55dbff53b 100644 --- a/src/db/__init__.py +++ b/src/db/__init__.py @@ -1,3 +1,13 @@ +from dataclasses import dataclass + + +@dataclass +class ConnectionString: + url: str + table: str + geo_column: str + + # It's expected to be set from another module # Example: # PG_CONN = { diff --git a/src/layman/layer/util.py b/src/layman/layer/util.py index 6f874ad74..5b2dd97b8 100644 --- a/src/layman/layer/util.py +++ b/src/layman/layer/util.py @@ -1,8 +1,9 @@ from functools import wraps, partial +from urllib import parse import re from flask import current_app, request, g - +from db import ConnectionString from layman import LaymanError, patch_mode, util as layman_util, settings from layman.util import call_modules_fn, get_providers_from_source_names, get_internal_sources, \ to_safe_name, url_for @@ -257,3 +258,14 @@ def get_same_or_missing_prop_names(workspace, layername): md_comparison = get_metadata_comparison(workspace, layername) prop_names = get_syncable_prop_names() return metadata_common.get_same_or_missing_prop_names(prop_names, md_comparison) + + +def parse_and_validate_connection_string(connection_string): + connection = parse.urlparse(connection_string, ) + params = parse.parse_qs(connection.query) + url = connection._replace(query='').geturl() + result = ConnectionString(url=url, + table=params.get('table', [None])[0], + geo_column=params.get('geo_column', [None])[0], + ) + return result diff --git a/src/layman/layer/util_test.py b/src/layman/layer/util_test.py index ab6214ade..c1953d2c3 100644 --- a/src/layman/layer/util_test.py +++ b/src/layman/layer/util_test.py @@ -3,7 +3,9 @@ from psycopg2 import tz import pytest +from db import ConnectionString from layman import settings +from . import util from .util import to_safe_layer_name, fill_in_partial_info_statuses @@ -143,3 +145,45 @@ def successful(): filled_info = fill_in_partial_info_statuses(publication_info, chain_info) assert filled_info == expected_info, f'filled_info={filled_info}, expected_info={expected_info}' + + +@pytest.mark.parametrize('connection_string, exp_result', [ + ('postgresql://user:password@postgresql:5432/dbname?table=table_name&geo_column=wkb_geometry', ConnectionString( + url='postgresql://user:password@postgresql:5432/dbname', + table='table_name', + geo_column='wkb_geometry', + )), + ('postgresql://postgresql', ConnectionString( + url='postgresql://postgresql', + table=None, + geo_column=None, + )), + ('', ConnectionString( + url='', + table=None, + geo_column=None, + )), + (' ', ConnectionString( + url=' ', + table=None, + geo_column=None, + )), + ('_', ConnectionString( + url='_', + table=None, + geo_column=None, + )), + ('$^&*(', ConnectionString( + url='$^&*(', + table=None, + geo_column=None, + )), + ('ščžýžý', ConnectionString( + url='ščžýžý', + table=None, + geo_column=None, + )), +]) +def test_parse_connection_string(connection_string, exp_result): + result = util.parse_and_validate_connection_string(connection_string) + assert result == exp_result From 02cd68c4da01cb04b2b7f66ed9f2d98053d9c89e Mon Sep 17 00:00:00 2001 From: index-git Date: Fri, 6 Jan 2023 16:46:08 +0100 Subject: [PATCH 08/10] Parse db_connection in POST --- src/layman/layer/rest_workspace_layers.py | 12 ++++++++---- test_tools/process_client.py | 13 +++++++------ .../layer_external_db/external_db_test.py | 3 +-- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/layman/layer/rest_workspace_layers.py b/src/layman/layer/rest_workspace_layers.py index fc50a6357..7a6a7e931 100644 --- a/src/layman/layer/rest_workspace_layers.py +++ b/src/layman/layer/rest_workspace_layers.py @@ -53,23 +53,27 @@ def post(workspace): if len(filename) > 0 ] input_files = fs_util.InputFiles(sent_streams=sent_file_streams, sent_paths=sent_file_paths) - db_connection = request.form.get('db_connection', '') - if not input_files and not db_connection: + + # DB_CONNECTION + db_connection_string = request.form.get('db_connection', '') + if not input_files and not db_connection_string: raise LaymanError(1, { 'parameters': ['file', 'db_connection'], 'message': 'Both `file` and `db_connection` parameters are empty', 'expected': 'One of the parameters is filled.', }) - if input_files and db_connection: + if input_files and db_connection_string: raise LaymanError(48, { 'parameters': ['file', 'db_connection'], 'message': 'Both `file` and `db_connection` parameters are filled', 'expected': 'Only one of the parameters is fulfilled.', 'found': { 'file': input_files.raw_paths, - 'db_connection': db_connection, + 'db_connection': db_connection_string, }}) + db_connection = util.parse_and_validate_connection_string(db_connection_string) if db_connection_string else None + # NAME unsafe_layername = request.form.get('name', '') if len(unsafe_layername) == 0: diff --git a/test_tools/process_client.py b/test_tools/process_client.py index a7364ce44..02ac1a84c 100644 --- a/test_tools/process_client.py +++ b/test_tools/process_client.py @@ -365,12 +365,13 @@ def publish_workspace_publication(publication_type, if not do_not_post_name: data['name'] = name data['title'] = title - if not with_chunks: - for file_path in file_paths: - assert os.path.isfile(file_path), file_path - files = [('file', (os.path.basename(fp), open(fp, 'rb'))) for fp in file_paths] - else: - data['file'] = [os.path.basename(file) for file in file_paths] + if file_paths: + if not with_chunks: + for file_path in file_paths: + assert os.path.isfile(file_path), file_path + files = [('file', (os.path.basename(fp), open(fp, 'rb'))) for fp in file_paths] + else: + data['file'] = [os.path.basename(file) for file in file_paths] if style_file: files.append(('style', (os.path.basename(style_file), open(style_file, 'rb')))) if access_rights and access_rights.get('read'): diff --git a/tests/dynamic_data/publications/layer_external_db/external_db_test.py b/tests/dynamic_data/publications/layer_external_db/external_db_test.py index 215f84dfb..4f8898c39 100644 --- a/tests/dynamic_data/publications/layer_external_db/external_db_test.py +++ b/tests/dynamic_data/publications/layer_external_db/external_db_test.py @@ -12,8 +12,7 @@ TEST_CASES = { 'external_vector_sld': { 'rest_params': { - 'file_paths': [], - 'db_connection': 'db_connection_string', + 'db_connection': 'postgresql://username:password@host:port/dbname?table=table_name&geo_column=geo_column_name', }, }, } From 4655ffe7f4bbdf03eb3e156343a42b6382147bd8 Mon Sep 17 00:00:00 2001 From: index-git Date: Wed, 11 Jan 2023 10:26:14 +0100 Subject: [PATCH 09/10] Assert db_connection has url, table and geo_column --- doc/rest.md | 1 + src/layman/layer/util.py | 13 ++ src/layman/layer/util_test.py | 119 +++++++++++++----- .../publications/wrong_input/__init__.py | 25 ++++ 4 files changed, 127 insertions(+), 31 deletions(-) diff --git a/doc/rest.md b/doc/rest.md index 4bd4b4614..000b9c0c3 100644 --- a/doc/rest.md +++ b/doc/rest.md @@ -146,6 +146,7 @@ Body parameters: - if QML style is used in this request, it must list all attributes contained in given data file - *db_connection*, string - exactly one of `file` or `db_connection` must be set + - format `postgresql://:@:/?table=&geo_column=` is expected query parameters `table` and `geo_column` specified - *name*, string - computer-friendly identifier of the layer - must be unique among all layers of one workspace diff --git a/src/layman/layer/util.py b/src/layman/layer/util.py index 5b2dd97b8..6f3832f30 100644 --- a/src/layman/layer/util.py +++ b/src/layman/layer/util.py @@ -268,4 +268,17 @@ def parse_and_validate_connection_string(connection_string): table=params.get('table', [None])[0], geo_column=params.get('geo_column', [None])[0], ) + if not all([result.url, result.table, result.geo_column]): + raise LaymanError(2, { + 'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': connection_string, + 'url': result.url, + 'table': result.table, + 'geo_column': result.geo_column, + } + }) + return result diff --git a/src/layman/layer/util_test.py b/src/layman/layer/util_test.py index c1953d2c3..012a0b79d 100644 --- a/src/layman/layer/util_test.py +++ b/src/layman/layer/util_test.py @@ -4,7 +4,8 @@ import pytest from db import ConnectionString -from layman import settings +from layman import settings, LaymanError +from test_tools import util as test_util from . import util from .util import to_safe_layer_name, fill_in_partial_info_statuses @@ -153,37 +154,93 @@ def successful(): table='table_name', geo_column='wkb_geometry', )), - ('postgresql://postgresql', ConnectionString( - url='postgresql://postgresql', - table=None, - geo_column=None, - )), - ('', ConnectionString( - url='', - table=None, - geo_column=None, - )), - (' ', ConnectionString( - url=' ', - table=None, - geo_column=None, - )), - ('_', ConnectionString( - url='_', - table=None, - geo_column=None, - )), - ('$^&*(', ConnectionString( - url='$^&*(', - table=None, - geo_column=None, - )), - ('ščžýžý', ConnectionString( - url='ščžýžý', - table=None, - geo_column=None, - )), ]) def test_parse_connection_string(connection_string, exp_result): result = util.parse_and_validate_connection_string(connection_string) assert result == exp_result + + +@pytest.mark.parametrize('connection_string, exp_error', [ + ('postgresql://postgresql', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': 'postgresql://postgresql', + 'url': 'postgresql://postgresql', + 'table': None, + 'geo_column': None, + }, + }, + }), + ('', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': '', + 'url': '', + 'table': None, + 'geo_column': None, + }, + }, + }), + (' ', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': ' ', + 'url': ' ', + 'table': None, + 'geo_column': None, + }, + }, + }), + ('_', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': '_', + 'url': '_', + 'table': None, + 'geo_column': None, + }, + }, + }), + ('$^&*(', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': '$^&*(', + 'url': '$^&*(', + 'table': None, + 'geo_column': None, + }, + }, + }), + ('ščžýžý', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': 'ščžýžý', + 'url': 'ščžýžý', + 'table': None, + 'geo_column': None, + }, + }, + }), +]) +def test_validate_connection_string(connection_string, exp_error): + with pytest.raises(LaymanError) as exc_info: + util.parse_and_validate_connection_string(connection_string) + test_util.assert_error(exp_error, exc_info) diff --git a/tests/dynamic_data/publications/wrong_input/__init__.py b/tests/dynamic_data/publications/wrong_input/__init__.py index 778e780ae..e1c0a848b 100644 --- a/tests/dynamic_data/publications/wrong_input/__init__.py +++ b/tests/dynamic_data/publications/wrong_input/__init__.py @@ -1393,6 +1393,31 @@ }, }, }, + 'partial_db_connect': { + KEY_PUBLICATION_TYPE: process_client.LAYER_TYPE, + KEY_ACTION_PARAMS: { + 'db_connection': 'db_connection', + 'compress': False, + 'with_chunks': False, + }, + consts.KEY_EXCEPTION: LaymanError, + KEY_EXPECTED_EXCEPTION: { + KEY_DEFAULT: {'http_code': 400, + 'sync': True, + 'code': 2, + 'message': 'Wrong parameter value', + 'detail': { + 'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'url': 'db_connection', + 'table': None, + 'geo_column': None, + }}, + }, + }, + }, } VALIDATION_PATCH_ACTION = { From 8575f80d8fb7d76737819fcca351d51cec6e2ab5 Mon Sep 17 00:00:00 2001 From: index-git Date: Thu, 12 Jan 2023 10:18:56 +0100 Subject: [PATCH 10/10] Assert db_connection schema is `postgresql` --- doc/rest.md | 2 +- src/layman/layer/util.py | 11 +++++++ src/layman/layer/util_test.py | 30 +++++++------------ .../publications/wrong_input/__init__.py | 17 +++++------ 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/doc/rest.md b/doc/rest.md index 000b9c0c3..14d37c162 100644 --- a/doc/rest.md +++ b/doc/rest.md @@ -146,7 +146,7 @@ Body parameters: - if QML style is used in this request, it must list all attributes contained in given data file - *db_connection*, string - exactly one of `file` or `db_connection` must be set - - format `postgresql://:@:/?table=&geo_column=` is expected query parameters `table` and `geo_column` specified + - format `postgresql://:@:/?table=&geo_column=` is expected with schema `postgresql` and query parameters `table` and `geo_column` specified - *name*, string - computer-friendly identifier of the layer - must be unique among all layers of one workspace diff --git a/src/layman/layer/util.py b/src/layman/layer/util.py index 6f3832f30..fe8018e79 100644 --- a/src/layman/layer/util.py +++ b/src/layman/layer/util.py @@ -262,6 +262,17 @@ def get_same_or_missing_prop_names(workspace, layername): def parse_and_validate_connection_string(connection_string): connection = parse.urlparse(connection_string, ) + if connection.scheme != 'postgresql': + raise LaymanError(2, { + 'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': connection_string, + 'schema': connection.scheme, + } + }) + params = parse.parse_qs(connection.query) url = connection._replace(query='').geturl() result = ConnectionString(url=url, diff --git a/src/layman/layer/util_test.py b/src/layman/layer/util_test.py index 012a0b79d..dcaeafbb8 100644 --- a/src/layman/layer/util_test.py +++ b/src/layman/layer/util_test.py @@ -177,65 +177,55 @@ def test_parse_connection_string(connection_string, exp_result): ('', {'http_code': 400, 'code': 2, 'detail': {'parameter': 'db_connection', - 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', 'expected': 'postgresql://:@:/?table=&geo_column=', 'found': { 'db_connection': '', - 'url': '', - 'table': None, - 'geo_column': None, + 'schema': '', }, }, }), (' ', {'http_code': 400, 'code': 2, 'detail': {'parameter': 'db_connection', - 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', 'expected': 'postgresql://:@:/?table=&geo_column=', 'found': { 'db_connection': ' ', - 'url': ' ', - 'table': None, - 'geo_column': None, + 'schema': '', }, }, }), ('_', {'http_code': 400, 'code': 2, 'detail': {'parameter': 'db_connection', - 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', 'expected': 'postgresql://:@:/?table=&geo_column=', 'found': { 'db_connection': '_', - 'url': '_', - 'table': None, - 'geo_column': None, + 'schema': '', }, }, }), ('$^&*(', {'http_code': 400, 'code': 2, 'detail': {'parameter': 'db_connection', - 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', 'expected': 'postgresql://:@:/?table=&geo_column=', 'found': { 'db_connection': '$^&*(', - 'url': '$^&*(', - 'table': None, - 'geo_column': None, + 'schema': '', }, }, }), ('ščžýžý', {'http_code': 400, 'code': 2, 'detail': {'parameter': 'db_connection', - 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', 'expected': 'postgresql://:@:/?table=&geo_column=', 'found': { 'db_connection': 'ščžýžý', - 'url': 'ščžýžý', - 'table': None, - 'geo_column': None, + 'schema': '', }, }, }), diff --git a/tests/dynamic_data/publications/wrong_input/__init__.py b/tests/dynamic_data/publications/wrong_input/__init__.py index e1c0a848b..133d5ccef 100644 --- a/tests/dynamic_data/publications/wrong_input/__init__.py +++ b/tests/dynamic_data/publications/wrong_input/__init__.py @@ -1406,15 +1406,14 @@ 'sync': True, 'code': 2, 'message': 'Wrong parameter value', - 'detail': { - 'parameter': 'db_connection', - 'message': 'Parameter `db_connection` is expected to have `url` part and `table` and `geo_column` query parameters', - 'expected': 'postgresql://:@:/?table=&geo_column=', - 'found': { - 'url': 'db_connection', - 'table': None, - 'geo_column': None, - }}, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': 'db_connection', + 'schema': '', + }, + }, }, }, },