From 3bbc8ca387fd93c9b2b00459386cb588758e225d Mon Sep 17 00:00:00 2001 From: index-git Date: Mon, 13 Feb 2023 11:24:44 +0100 Subject: [PATCH 1/4] Process_client.patch supports asserts skip --- test_tools/process_client.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/test_tools/process_client.py b/test_tools/process_client.py index 247789616..55443a37e 100644 --- a/test_tools/process_client.py +++ b/test_tools/process_client.py @@ -173,29 +173,31 @@ def patch_workspace_publication(publication_type, overview_resampling=None, do_not_upload_chunks=False, time_regex=None, + skip_asserts=False, ): 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 + if not skip_asserts: + # 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 + 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 = [] if file_paths is None and not map_layers else file_paths + assert not (time_regex and publication_type == MAP_TYPE) + assert not (publication_type == LAYER_TYPE and crs and not file_paths) - if style_file or with_chunks or compress or compress_settings or overview_resampling: - assert publication_type == LAYER_TYPE - if map_layers or native_extent: - assert publication_type == MAP_TYPE + if style_file or with_chunks or compress or compress_settings or overview_resampling: + assert publication_type == LAYER_TYPE + if map_layers or native_extent: + assert publication_type == MAP_TYPE - # Compress settings can be used only with compress option - assert not compress_settings or compress + # Compress settings can be used only with compress option + assert not compress_settings or compress - assert not (time_regex and publication_type == MAP_TYPE) - assert not (publication_type == LAYER_TYPE and crs and not file_paths) + file_paths = [] if file_paths is None and not map_layers else file_paths with app.app_context(): r_url = url_for(publication_type_def.patch_workspace_publication_url, From 6582258375bfe33bb414cc39eb788f0feb35ae95 Mon Sep 17 00:00:00 2001 From: index-git Date: Mon, 13 Feb 2023 11:02:20 +0100 Subject: [PATCH 2/4] POST/PATCH Layer do not accept `crs` parameter without `file` parameter --- CHANGELOG.md | 1 + doc/rest.md | 3 +- src/layman/layer/rest_workspace_layer.py | 16 ++++++++-- src/layman/layer/rest_workspace_layers.py | 13 ++++++-- .../publications/wrong_input/__init__.py | 31 +++++++++++++++++++ 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8e190260..e01da9334 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ make timgen-build - [#703](https://github.com/LayerManager/layman/issues/703) Fill column `external_table_uri` in `publications` table in prime DB schema for all publications. Value is set to `null` for all existing publications. ### Changes - [#703](https://github.com/LayerManager/layman/issues/703) Endpoints [POST Workspace Layers](doc/rest.md#post-workspace-layers) and [PATCH Workspace Layer](doc/rest.md#patch-workspace-layer) support new body parameter *db_connection*. +- [#703](https://github.com/LayerManager/layman/issues/703) Endpoint [PATCH Workspace Layer](doc/rest.md#patch-workspace-layer) supports parameter `crs` only if `file` is provided. - [#772](https://github.com/LayerManager/layman/issues/772) Speed up endpoints [GET Workspace Layer Thumbnail](doc/rest.md#get-workspace-layer-thumbnail), [GET Workspace Layer Style](doc/rest.md#get-workspace-layer-style), [GET Workspace Map Thumbnail](doc/rest.md#get-workspace-map-thumbnail) and [GET Workspace Map File](doc/rest.md#get-workspace-map-file). - [#755](https://github.com/LayerManager/layman/issues/755) Downgrade Node.js of Timgen from v18 to v16 diff --git a/doc/rest.md b/doc/rest.md index 1aacf2aa3..da69883c5 100644 --- a/doc/rest.md +++ b/doc/rest.md @@ -163,6 +163,7 @@ Body parameters: - *description* - by default it is empty string - *crs*, string, e.g. `EPSG:3857`, supported EPSG codes are defined by [LAYMAN_INPUT_SRS_LIST](./env-settings.md#LAYMAN_INPUT_SRS_LIST) + - supported only for layers with file source - CRS of the file - by default it is read/guessed from input file - *style*, style file @@ -345,7 +346,7 @@ Body parameters: - *title* - *description* - *crs*, string, e.g. `EPSG:3857`, supported EPSG codes are defined by [LAYMAN_INPUT_SRS_LIST](./env-settings.md#LAYMAN_INPUT_SRS_LIST) - - Taken into account only if `file` is provided. + - Supported only if `file` is provided. - *style*, style file - SLD or QML style file (recognized by the root element of XML: `StyledLayerDescriptor` or `qgis`) - QML style for raster data file is not supported diff --git a/src/layman/layer/rest_workspace_layer.py b/src/layman/layer/rest_workspace_layer.py index df4aafa80..74dfac447 100644 --- a/src/layman/layer/rest_workspace_layer.py +++ b/src/layman/layer/rest_workspace_layer.py @@ -81,16 +81,26 @@ def patch(workspace, layername): 'db_connection': external_table_uri_str, }}) - external_table_uri = util.parse_and_validate_external_table_uri_str(external_table_uri_str) if external_table_uri_str else None if input_files or not info.get('_is_external_table') else info.get('_table_uri') - # CRS crs_id = None - if len(input_files.raw_paths) > 0 and len(request.form.get('crs', '')) > 0: + if len(request.form.get('crs', '')) > 0: crs_id = request.form['crs'] if crs_id not in settings.INPUT_SRS_LIST: raise LaymanError(2, {'parameter': 'crs', 'supported_values': settings.INPUT_SRS_LIST}) check_crs = crs_id is None + if crs_id and not input_files: + raise LaymanError(48, { + 'parameters': ['crs', 'file'], + 'message': 'Parameter `crs` needs also parameter `file`.', + 'expected': 'Input files in `file` parameter or empty `crs` parameter.', + 'found': { + 'crs': crs_id, + 'file': request.form.getlist('file'), + }}) + + external_table_uri = util.parse_and_validate_external_table_uri_str(external_table_uri_str) if external_table_uri_str else None if input_files or not info.get('_is_external_table') else info.get('_table_uri') + # TITLE if len(request.form.get('title', '')) > 0: kwargs['title'] = request.form['title'] diff --git a/src/layman/layer/rest_workspace_layers.py b/src/layman/layer/rest_workspace_layers.py index 19f0f28e0..dd52d16a4 100644 --- a/src/layman/layer/rest_workspace_layers.py +++ b/src/layman/layer/rest_workspace_layers.py @@ -72,8 +72,6 @@ def post(workspace): 'db_connection': external_table_uri_str, }}) - external_table_uri = util.parse_and_validate_external_table_uri_str(external_table_uri_str) if external_table_uri_str else None - # NAME unsafe_layername = request.form.get('name', '') if len(unsafe_layername) == 0: @@ -90,8 +88,19 @@ def post(workspace): crs_id = request.form['crs'] if crs_id not in settings.INPUT_SRS_LIST: raise LaymanError(2, {'parameter': 'crs', 'supported_values': settings.INPUT_SRS_LIST}) + if crs_id and not input_files: + raise LaymanError(48, { + 'parameters': ['crs', 'file'], + 'message': 'Parameter `crs` needs also parameter `file`.', + 'expected': 'Input files in `file` parameter or empty `crs` parameter.', + 'found': { + 'crs': crs_id, + 'file': request.form.getlist('file'), + }}) check_crs = crs_id is None + external_table_uri = util.parse_and_validate_external_table_uri_str(external_table_uri_str) if external_table_uri_str else None + # Timeseries regex time_regex = request.form.get('time_regex') or None slugified_time_regex = input_file.slugify_timeseries_filename_pattern(time_regex) if time_regex else None diff --git a/tests/dynamic_data/publications/wrong_input/__init__.py b/tests/dynamic_data/publications/wrong_input/__init__.py index b25fd84d8..ed7528a4d 100644 --- a/tests/dynamic_data/publications/wrong_input/__init__.py +++ b/tests/dynamic_data/publications/wrong_input/__init__.py @@ -1428,6 +1428,37 @@ }, }, }, + 'crs_and_db_connect': { + KEY_PUBLICATION_TYPE: process_client.LAYER_TYPE, + KEY_ACTION_PARAMS: { + 'crs': 'EPSG:4326', + 'db_connection': 'postgresql://username:password@host:port/dbname?table=table_name&geo_column=geo_column_name', + 'compress': False, + 'with_chunks': False, + 'skip_asserts': True, + }, + consts.KEY_EXCEPTION: LaymanError, + KEY_EXPECTED_EXCEPTION: { + KEY_DEFAULT: {'http_code': 400, + 'sync': True, + 'code': 48, + 'message': 'Wrong combination of parameters', + 'detail': { + 'parameters': ['crs', 'file'], + 'message': 'Parameter `crs` needs also parameter `file`.', + 'expected': 'Input files in `file` parameter or empty `crs` parameter.', + 'found': { + 'crs': 'EPSG:4326', + 'file': [], + }}, + }, + }, + KEY_PATCHES: { + 'full': { + KEY_PATCH_POST: {}, + }, + }, + }, } VALIDATION_PATCH_ACTION = { From 2502c0b26c365781fc5aeb71837513c3ac7ee999 Mon Sep 17 00:00:00 2001 From: index-git Date: Mon, 13 Feb 2023 14:38:34 +0100 Subject: [PATCH 3/4] Create external table for test in CRS 32635 --- src/layman/layer/util_test.py | 17 +++++++++-------- test_tools/external_db.py | 9 +++++---- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/layman/layer/util_test.py b/src/layman/layer/util_test.py index 55faae520..47b0132cd 100644 --- a/src/layman/layer/util_test.py +++ b/src/layman/layer/util_test.py @@ -14,19 +14,20 @@ @pytest.fixture(scope="module") def ensure_tables(): tables = [ - ('schema_name', 'table_name', 'geo_wkb_column', ['my_id'], []), - ('schema_name', 'two_column_primary_key', 'geo_wkb_column', ['partial_id_1', 'partial_id_2'], []), - ('schema_name', 'no_primary_key', 'geo_wkb_column', [], []), - ('schema_name', 'table_with_unsafe_column_name', 'geo_wkb_column', ['my_id'], ['name-with-dashes']), - ('schema_name', 'table_without_geo_column', None, ['my_id'], []), + ('schema_name', 'table_name', 'geo_wkb_column', ['my_id'], [], 4326), + ('schema_name', 'two_column_primary_key', 'geo_wkb_column', ['partial_id_1', 'partial_id_2'], [], 4326), + ('schema_name', 'no_primary_key', 'geo_wkb_column', [], [], 4326), + ('schema_name', 'table_with_unsafe_column_name', 'geo_wkb_column', ['my_id'], ['name-with-dashes'], 4326), + ('schema_name', 'table_without_geo_column', None, ['my_id'], [], 4326), + ('schema_name', 'table_name_32635', 'geo_wkb_column', ['my_id'], [], 32635), ] - for schema, table, geo_column, primary_key_columns, other_columns in tables: + for schema, table, geo_column, primary_key_columns, other_columns, srid in tables: external_db.ensure_table(schema, table, geo_column, primary_key_columns=primary_key_columns, - other_columns=other_columns) + other_columns=other_columns, srid=srid) yield - for schema, table, _, _, _ in tables: + for schema, table, _, _, _, _ in tables: external_db.drop_table(schema, table) diff --git a/test_tools/external_db.py b/test_tools/external_db.py index d99215fb2..f4819c4b8 100644 --- a/test_tools/external_db.py +++ b/test_tools/external_db.py @@ -33,7 +33,7 @@ def ensure_schema(schema): db_util.run_statement(statement, conn_cur=conn_cur) -def ensure_table(schema, name, geo_column, *, primary_key_columns=None, other_columns=None): +def ensure_table(schema, name, geo_column, *, primary_key_columns=None, other_columns=None, srid=4326): primary_key_columns = ['id'] if primary_key_columns is None else primary_key_columns other_columns = other_columns or [] @@ -48,8 +48,9 @@ def ensure_table(schema, name, geo_column, *, primary_key_columns=None, other_co column=sql.Identifier(col) )) if geo_column: - columns.append(sql.SQL('{geo_column} geometry(Geometry, 4326)').format( - geo_column=sql.Identifier(geo_column) + columns.append(sql.SQL('{geo_column} geometry(Geometry, {srid})').format( + geo_column=sql.Identifier(geo_column), + srid=sql.Literal(srid), )) if primary_key_columns: columns.append(sql.SQL('PRIMARY KEY ({columns})').format( @@ -61,7 +62,7 @@ def ensure_table(schema, name, geo_column, *, primary_key_columns=None, other_co columns=sql.SQL(',').join(columns), ) conn_cur = db_util.create_connection_cursor(URI_STR) - db_util.run_statement(statement, conn_cur=conn_cur) + db_util.run_statement(statement, conn_cur=conn_cur, log_query=True) def import_table(input_file_path, *, table=None, schema='public', geo_column=settings.OGR_DEFAULT_GEOMETRY_COLUMN, From 36b3696991a25f52128bbe560173e964383a44f8 Mon Sep 17 00:00:00 2001 From: index-git Date: Mon, 13 Feb 2023 14:44:17 +0100 Subject: [PATCH 4/4] Assert external table is in supported CRS --- src/db/util.py | 6 +++--- src/layman/layer/db/__init__.py | 4 ++-- src/layman/layer/util.py | 11 ++++++++++- src/layman/layer/util_test.py | 12 ++++++++++++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/db/util.py b/src/db/util.py index b7dd8743b..1f974c9d5 100644 --- a/src/db/util.py +++ b/src/db/util.py @@ -88,14 +88,14 @@ def get_srid(crs): return srid -def get_crs(srid): +def get_crs(srid, conn_cur=None, *, use_internal_srid=True): crs = next(( crs_code for crs_code, crs_item_def in crs_def.CRSDefinitions.items() if crs_item_def.srid == srid - ), None) + ), None) if use_internal_srid else None if not crs: sql = 'select auth_name, auth_srid from spatial_ref_sys where srid = %s;' - auth_name, auth_srid = run_query(sql, (srid, ))[0] + auth_name, auth_srid = run_query(sql, (srid, ), conn_cur=conn_cur)[0] if auth_name or auth_srid: crs = f'{auth_name}:{auth_srid}' return crs diff --git a/src/layman/layer/db/__init__.py b/src/layman/layer/db/__init__.py index 4284798bf..9c4de132c 100644 --- a/src/layman/layer/db/__init__.py +++ b/src/layman/layer/db/__init__.py @@ -544,10 +544,10 @@ def get_bbox(schema, table_name, conn_cur=None, column=settings.OGR_DEFAULT_GEOM return result -def get_crs(schema, table_name, conn_cur=None, column=settings.OGR_DEFAULT_GEOMETRY_COLUMN): +def get_crs(schema, table_name, conn_cur=None, column=settings.OGR_DEFAULT_GEOMETRY_COLUMN, *, use_internal_srid=True): query = 'select Find_SRID(%s, %s, %s);' srid = db_util.run_query(query, (schema, table_name, column), conn_cur=conn_cur)[0][0] - crs = db_util.get_crs(srid) + crs = db_util.get_crs(srid, conn_cur, use_internal_srid=use_internal_srid) return crs diff --git a/src/layman/layer/util.py b/src/layman/layer/util.py index 38c4eb3bc..8fb2ad9a4 100644 --- a/src/layman/layer/util.py +++ b/src/layman/layer/util.py @@ -12,7 +12,7 @@ from layman.common import redis as redis_util, tasks as tasks_util, metadata as metadata_common from layman.common.util import PUBLICATION_NAME_PATTERN, PUBLICATION_MAX_LENGTH, clear_publication_info as common_clear_publication_info from . import get_layer_sources, LAYER_TYPE, get_layer_type_def, get_layer_info_keys -from .db import get_all_table_column_names +from .db import get_all_table_column_names, get_crs LAYERNAME_PATTERN = PUBLICATION_NAME_PATTERN LAYERNAME_MAX_LENGTH = PUBLICATION_MAX_LENGTH @@ -374,6 +374,15 @@ def parse_and_validate_external_table_uri_str(external_table_uri_str): } }) + crs = get_crs(schema, table, conn_cur=conn_cur, column=geo_column, use_internal_srid=False) + if crs not in settings.INPUT_SRS_LIST: + raise LaymanError(2, { + 'parameter': 'db_connection', + 'message': 'Unsupported CRS of table data.', + 'supported_values': settings.INPUT_SRS_LIST, + 'found': crs, + }) + # https://stackoverflow.com/a/20537829 query = ''' SELECT diff --git a/src/layman/layer/util_test.py b/src/layman/layer/util_test.py index 47b0132cd..a40a9e716 100644 --- a/src/layman/layer/util_test.py +++ b/src/layman/layer/util_test.py @@ -422,6 +422,18 @@ def test_parse_external_table_uri_str(external_table_uri_str, exp_result): }, }, }, id='table_without_geo_column'), + pytest.param( + 'postgresql://docker:docker@postgresql:5432/external_test_db?schema=schema_name&table=table_name_32635', + { + 'http_code': 400, + 'code': 2, + 'detail': { + 'parameter': 'db_connection', + 'message': 'Unsupported CRS of table data.', + 'found': 'EPSG:32635', + 'supported_values': settings.INPUT_SRS_LIST, + }, + }, id='table_in_crs_32635'), ]) def test_validate_external_table_uri_str(external_table_uri_str, exp_error): with pytest.raises(LaymanError) as exc_info: