diff --git a/CHANGELOG.md b/CHANGELOG.md index 964be7df5..0b4a60ac0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## v1.20.0 + {release-date} +### Upgrade requirements +### Migrations and checks +#### 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 ### Upgrade requirements diff --git a/doc/rest.md b/doc/rest.md index 95dd8376f..14d37c162 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,9 @@ 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 + - exactly one of `file` or `db_connection` must be set + - 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/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/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 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..7a6a7e931 100644 --- a/src/layman/layer/rest_workspace_layers.py +++ b/src/layman/layer/rest_workspace_layers.py @@ -53,8 +53,26 @@ 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 + 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_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_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', '') @@ -91,13 +109,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 +199,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/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: diff --git a/src/layman/layer/util.py b/src/layman/layer/util.py index 070237574..fe8018e79 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 @@ -87,9 +88,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 @@ -103,7 +104,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() @@ -120,7 +121,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, @@ -257,3 +258,38 @@ 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, ) + 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, + 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 ab6214ade..dcaeafbb8 100644 --- a/src/layman/layer/util_test.py +++ b/src/layman/layer/util_test.py @@ -3,7 +3,10 @@ from psycopg2 import tz import pytest -from layman import settings +from db import ConnectionString +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 @@ -143,3 +146,91 @@ 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', + )), +]) +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 schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': '', + 'schema': '', + }, + }, + }), + (' ', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': ' ', + 'schema': '', + }, + }, + }), + ('_', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': '_', + 'schema': '', + }, + }, + }), + ('$^&*(', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': '$^&*(', + 'schema': '', + }, + }, + }), + ('ščžýžý', {'http_code': 400, + 'code': 2, + 'detail': {'parameter': 'db_connection', + 'message': 'Parameter `db_connection` is expected to have schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': 'ščžýžý', + 'schema': '', + }, + }, + }), +]) +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/test_tools/process_client.py b/test_tools/process_client.py index 279681938..02ac1a84c 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 @@ -364,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'): @@ -384,6 +386,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..4f8898c39 --- /dev/null +++ b/tests/dynamic_data/publications/layer_external_db/external_db_test.py @@ -0,0 +1,46 @@ +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': { + 'db_connection': 'postgresql://username:password@host:port/dbname?table=table_name&geo_column=geo_column_name', + }, + }, +} + + +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'] + }) diff --git a/tests/dynamic_data/publications/wrong_input/__init__.py b/tests/dynamic_data/publications/wrong_input/__init__.py index b8b8cdd22..133d5ccef 100644 --- a/tests/dynamic_data/publications/wrong_input/__init__.py +++ b/tests/dynamic_data/publications/wrong_input/__init__.py @@ -1334,6 +1334,89 @@ }, }, }, + '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'], + }, + }, + }, + }, + }, + '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 schema `postgresql`', + 'expected': 'postgresql://:@:/?table=&geo_column=', + 'found': { + 'db_connection': 'db_connection', + 'schema': '', + }, + }, + }, + }, + }, } VALIDATION_PATCH_ACTION = { 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