diff --git a/app/__init__.py b/app/__init__.py index 9131f885..5336495c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -32,15 +32,7 @@ @app.before_request def log_route(): g.setdefault('request_started', time.time()) - route_logger.info('%s %s', request.method, request.path) - - -@app.after_request -def wrap_in_callback_if_present(response): - if "callback" in request.args: - response.headers['Content-Type'] = 'application/javascript' - response.data = f'{request.args.get("callback")}({response.get_data(as_text=True)})' - return response + route_logger.debug('%s %s', request.method, request.path) # Add CORS Headers to all request diff --git a/app/helpers/validation/__init__.py b/app/helpers/validation/__init__.py index 5a7d85b3..6e716e15 100644 --- a/app/helpers/validation/__init__.py +++ b/app/helpers/validation/__init__.py @@ -2,7 +2,10 @@ from shapely.geometry import Polygon +from flask import abort + from app.helpers.helpers import float_raise_nan +from app.settings import VALID_SRID bboxes = { 2056: @@ -38,3 +41,13 @@ def srs_guesser(geom): sr = epsg break return sr + + +def validate_sr(sr): + if sr not in VALID_SRID: + abort( + 400, + "Please provide a valid number for the spatial reference system model: " + f"{', '.join(map(str, VALID_SRID))}" + ) + return sr diff --git a/app/helpers/validation/height.py b/app/helpers/validation/height.py index bb7c4a93..9d6f7a37 100644 --- a/app/helpers/validation/height.py +++ b/app/helpers/validation/height.py @@ -21,12 +21,3 @@ def validate_lon_lat(lon, lat): abort(400, "Please provide numerical values for the parameter 'northing'/'lat'") return lon, lat - - -def validate_sr(sr): - if sr not in (21781, 2056): - abort( - 400, - "Please provide a valid number for the spatial reference system model 21781 or 2056" - ) - return sr diff --git a/app/helpers/validation/profile.py b/app/helpers/validation/profile.py index 1d713f0c..9aabcc12 100644 --- a/app/helpers/validation/profile.py +++ b/app/helpers/validation/profile.py @@ -10,10 +10,13 @@ from app.helpers.profile_helpers import PROFILE_DEFAULT_AMOUNT_POINTS from app.helpers.profile_helpers import PROFILE_MAX_AMOUNT_POINTS from app.helpers.validation import srs_guesser +from app.helpers.validation import validate_sr logger = logging.getLogger(__name__) max_content_length = 32 * 1024 * 1024 # 32MB +PROFILE_VALID_GEOMETRY_TYPES = ['LineString', 'Point'] + def read_linestring(): # param geom, list of coordinates defining the line on which we want a profile @@ -30,22 +33,25 @@ def read_linestring(): if not linestring: abort(400, "No 'geom' given, cannot create a profile without coordinates") + try: geom = geojson.loads(linestring, object_hook=geojson.GeoJSON.to_instance) except ValueError as e: - logger.exception(e) - abort(400, "Error loading geometry in JSON string") + logger.error('Invalid "geom" parameter, it is not geojson: %s', e) + abort(400, "Invalid geom parameter, must be a GEOJSON") + + if geom.get('type') not in PROFILE_VALID_GEOMETRY_TYPES: + abort(400, f"geom parameter must be a {'/'.join(PROFILE_VALID_GEOMETRY_TYPES)} GEOJSON") + try: geom_to_shape = shape(geom) - # pylint: disable=broad-except - except Exception as e: - logger.exception(e) - abort(400, "Error converting JSON to Shape") - try: - geom_to_shape.is_valid - # pylint: disable=broad-except - except Exception: - abort(400, "Invalid Linestring syntax") + except ValueError as e: + logger.error("Failed to transformed GEOJSON to shape: %s", e) + abort(400, "Error converting GEOJSON to Shape") + + if not geom_to_shape.is_valid: + abort(400, f"Invalid {geom['type']}") + if len(geom_to_shape.coords) > PROFILE_MAX_AMOUNT_POINTS: abort( 413, @@ -102,18 +108,14 @@ def read_spatial_reference(linestring): abort(400, "No 'sr' given and cannot be guessed from 'geom'") spatial_reference = sr - if spatial_reference not in (21781, 2056): - abort( - 400, - "Please provide a valid number for the spatial reference system model 21781 or 2056" - ) + validate_sr(spatial_reference) return spatial_reference def read_offset(): # param offset, used for smoothing. define how many coordinates should be included # in the window used for smoothing. If value is zero smoothing is disabled. - offset = 3 + offset = 0 if 'offset' in request.args: offset = request.args.get('offset') if offset.isdigit(): diff --git a/app/routes.py b/app/routes.py index 70325e68..eede2007 100644 --- a/app/routes.py +++ b/app/routes.py @@ -1,4 +1,8 @@ +import csv +import json import logging +from distutils.util import strtobool +from io import StringIO from shapely.geometry import Point from werkzeug.exceptions import HTTPException @@ -18,8 +22,8 @@ from app.helpers.route import prefix_route from app.helpers.validation import bboxes from app.helpers.validation import srs_guesser +from app.helpers.validation import validate_sr from app.helpers.validation.height import validate_lon_lat -from app.helpers.validation.height import validate_sr # add route prefix from app.statistics.statistics import load_json from app.statistics.statistics import prepare_data @@ -71,20 +75,45 @@ def height_route(): alt = get_height(sr, lon, lat, georaster_utils) if alt is None: abort(400, f'Requested coordinate ({lon},{lat}) out of bounds in sr {sr}') - return {'height': str(alt)} + data = {'height': str(alt)} + if "callback" in request.args: + data = f'{request.args.get("callback")}({json.dumps(data, separators=(",", ":"))})' + response = make_response(data, 200, {'Content-Type': 'application/javascript'}) + else: + response = make_response(data) + return response @app.route('/profile.json', methods=['GET', 'POST']) def profile_json_route(): - return __get_profile_from_helper(True) + profile, status_code = _get_profile(True) + if "callback" in request.args: + data = f'{request.args.get("callback")}({json.dumps(profile, separators=(",", ":"))})' + response = make_response(data, {'Content-Type': 'application/javascript'}) + else: + response = make_response(jsonify(profile)) + return response, status_code @app.route('/profile.csv', methods=['GET', 'POST']) def profile_csv_route(): - return __get_profile_from_helper(False) + if "callback" in request.args: + abort(400, 'callback parameter not supported') + profile, status_code = _get_profile(False) + csv.register_dialect( + 'semi-colon', delimiter=';', quoting=csv.QUOTE_ALL, quotechar='"', lineterminator='\r\n' + ) + buffer = StringIO() + writer = csv.writer(buffer, dialect='semi-colon') + # write header + writer.writerow(profile['headers']) + writer.writerows(profile['rows']) + buffer.seek(0) + + return buffer.read(), status_code, {'Content-Type': 'text/csv'} -def __get_profile_from_helper(output_to_json=True): +def _get_profile(output_to_json): linestring = profile_arg_validation.read_linestring() nb_points = profile_arg_validation.read_number_points() is_custom_nb_points = profile_arg_validation.read_is_custom_nb_points() @@ -94,7 +123,7 @@ def __get_profile_from_helper(output_to_json=True): # param only_requested_points, which is flag that when set to True will make # the profile with only the given points in geom (no filling points) if 'only_requested_points' in request.args: - only_requested_points = bool(request.args.get('only_requested_points')) + only_requested_points = strtobool(request.args.get('only_requested_points')) else: only_requested_points = False @@ -102,12 +131,12 @@ def __get_profile_from_helper(output_to_json=True): # there's not two points closer than what the resolution is) or if points are placed without # care for that. if 'smart_filling' in request.args: - smart_filling = bool(request.args.get('smart_filling')) + smart_filling = strtobool(request.args.get('smart_filling')) else: smart_filling = False if 'distinct_points' in request.args: - keep_points = bool(request.args.get('distinct_points')) + keep_points = strtobool(request.args.get('distinct_points')) else: keep_points = False @@ -122,18 +151,17 @@ def __get_profile_from_helper(output_to_json=True): output_to_json=output_to_json, georaster_utils=georaster_utils ) - if output_to_json: - response = jsonify(result) - else: - response = str(result) + # If profile calculation resulted in a lower number of point than requested (because there's no # need to add points closer to each other than the min resolution of 2m), we return HTTP 203 to - # notify that nb_points couldn't be match. + # notify that nb_points couldn't be match. Smartfilling can result in more points as expected. status_code = 200 - if is_custom_nb_points and len(result) < nb_points: + if output_to_json and is_custom_nb_points and len(result) != nb_points: status_code = 203 - content_type = 'application/json' if output_to_json else 'text/csv' - return response, status_code, {'ContentType': content_type, 'Content-Type': content_type} + elif not output_to_json and is_custom_nb_points and len(result['rows']) != nb_points: + status_code = 203 + + return result, status_code # if in debug, we add the route to the statistics page, otherwise it is not visible diff --git a/app/settings.py b/app/settings.py index 328eb2bb..6b300150 100644 --- a/app/settings.py +++ b/app/settings.py @@ -14,3 +14,5 @@ PRELOAD_RASTER_FILES = strtobool(os.getenv('PRELOAD_RASTER_FILES', 'False')) TRAP_HTTP_EXCEPTIONS = True + +VALID_SRID = [21781, 2056] diff --git a/logging-cfg-local.yml b/logging-cfg-local.yml index 846e4699..d1eb2db9 100644 --- a/logging-cfg-local.yml +++ b/logging-cfg-local.yml @@ -38,7 +38,6 @@ filters: - method - headers - remote_addr - - json formatters: standard: @@ -56,7 +55,6 @@ formatters: - flask_request_path - flask_request_method - flask_request_headers - - flask_request_json - flask_request_remote_addr remove_empty: True fmt: @@ -65,13 +63,11 @@ formatters: logger: name module: module function: funcName - process: process - thread: thread + worker_id: "%(process)d/%(thread)x" request: path: flask_request_path method: flask_request_method headers: flask_request_headers - data: flask_request_json remote: flask_request_remote_addr exc_info: exc_info message: message diff --git a/tests/unit_tests/test_height.py b/tests/unit_tests/test_height.py index 8bb2053f..39f20670 100644 --- a/tests/unit_tests/test_height.py +++ b/tests/unit_tests/test_height.py @@ -263,7 +263,7 @@ def test_height_lv03_with_callback_valid(self, mock_georaster_utils): } ) self.assertEqual(resp.content_type, 'application/javascript') - self.assertTrue('cb_({' in resp.get_data(as_text=True)) + self.assertEqual('cb_({"height":"568.2"})', resp.get_data(as_text=True)) @patch('app.routes.georaster_utils') def test_height_lv03_miss_northing(self, mock_georaster_utils): diff --git a/tests/unit_tests/test_profile.py b/tests/unit_tests/test_profile.py index 2cdbb43d..6a26197f 100644 --- a/tests/unit_tests/test_profile.py +++ b/tests/unit_tests/test_profile.py @@ -28,71 +28,63 @@ logger = logging.getLogger(__name__) -class TestProfile(unittest.TestCase): - # pylint: disable=too-many-public-methods +class TestProfileBase(unittest.TestCase): def setUp(self) -> None: service_alti.app.config['TESTING'] = True self.test_instance = service_alti.app.test_client() self.headers = DEFAULT_HEADERS - def __check_response(self, response, expected_status=200): + def check_response(self, response, expected_status=200): self.assertIsNotNone(response) self.assertEqual(response.status_code, expected_status, msg=response.get_data(as_text=True)) + def assert_response_contains(self, response, content): + self.assertTrue( + content in response.get_data(as_text=True), + msg=f"Response doesn't contain '{content}' : '{response.get_data(as_text=True)}'" + ) + + +class TestProfileJson(TestProfileBase): + # pylint: disable=too-many-public-methods + + def verify_point_is_present(self, response, point, msg="point not present"): + self.assertEqual(response.content_type, "application/json") + if len(point) != 2: + self.fail("Point must be a [x,y] point") + present = False + for profile_point in response.json: + if point[0] == profile_point['easting'] and point[1] == profile_point['northing']: + present = True + if not present: + self.fail(msg) + + def prepare_mock_and_test_json_profile(self, mock_georaster_utils, params, expected_status): + prepare_mock(mock_georaster_utils) + return self.get_json_profile(params=params, expected_status=expected_status) + def prepare_mock_and_test_post(self, mock_georaster_utils, body, expected_status): prepare_mock(mock_georaster_utils) response = self.post_with_body(body) - self.__check_response(response, expected_status) + self.check_response(response, expected_status) return response - def prepare_mock_and_test_csv_profile(self, mock_georaster_utils, params, expected_status): - prepare_mock(mock_georaster_utils) - response = self.get_csv_with_params(params) - self.__check_response(response, expected_status) - return response + def post_with_body(self, body): + return self.test_instance.post(ENDPOINT_FOR_JSON_PROFILE, data=body, headers=self.headers) - # pylint: disable=inconsistent-return-statements def get_json_profile(self, params, expected_status=200): # pylint: disable=broad-except try: response = self.test_instance.get( ENDPOINT_FOR_JSON_PROFILE, query_string=params, headers=self.headers ) - self.__check_response(response, expected_status) + self.check_response(response, expected_status) return response except Exception as e: logger.exception(e) self.fail(f'Call to test_instance failed: {e}') - - def get_csv_with_params(self, params): - return self.test_instance.get( - ENDPOINT_FOR_CSV_PROFILE, query_string=params, headers=self.headers - ) - - def post_with_body(self, body): - return self.test_instance.post(ENDPOINT_FOR_JSON_PROFILE, data=body, headers=self.headers) - - def prepare_mock_and_test_json_profile(self, mock_georaster_utils, params, expected_status): - prepare_mock(mock_georaster_utils) - return self.get_json_profile(params=params, expected_status=expected_status) - - def verify_point_is_present(self, response, point, msg="point not present"): - self.assertEqual(response.content_type, "application/json") - if len(point) != 2: - self.fail("Point must be a [x,y] point") - present = False - for profile_point in response.json: - if point[0] == profile_point['easting'] and point[1] == profile_point['northing']: - present = True - if not present: - self.fail(msg) - - def assert_response_contains(self, response, content): - self.assertTrue( - content in response.get_data(as_text=True), - msg=f"Response doesn't contain '{content}' : '{response.get_data(as_text=True)}'" - ) + return None @patch('app.routes.georaster_utils') def test_do_not_fail_when_no_origin(self, mock_georaster_utils): @@ -117,7 +109,7 @@ def test_profile_invalid_sr_json_valid(self, mock_georaster_utils): self.assert_response_contains( resp, "Please provide a valid number for the spatial reference " - "system model 21781 or 2056" + "system model: 21781, 2056" ) @patch('app.routes.georaster_utils') @@ -179,7 +171,7 @@ def test_profile_lv03_layers_none(self, mock_georaster_utils): params={'geom': '{"type":"LineString","coordinates":[[0,0],[0,0],[0,0]]}'}, expected_status=400 ) - self.assert_response_contains(resp, "No 'sr' given and cannot be guessed from 'geom'") + self.assert_response_contains(resp, "Invalid LineString") def test_profile_lv03_layers_none2(self): resp = self.get_json_profile( @@ -220,7 +212,7 @@ def test_profile_lv03_json_wrong_geom(self, mock_georaster_utils): resp = self.prepare_mock_and_test_json_profile( mock_georaster_utils=mock_georaster_utils, params={'geom': 'toto'}, expected_status=400 ) - self.assert_response_contains(resp, 'Error loading geometry in JSON string') + self.assert_response_contains(resp, 'Invalid geom parameter, must be a GEOJSON') @patch('app.routes.georaster_utils') def test_profile_lv03_json_wrong_shape(self, mock_georaster_utils): @@ -229,10 +221,21 @@ def test_profile_lv03_json_wrong_shape(self, mock_georaster_utils): params={'geom': LINESTRING_WRONG_SHAPE}, expected_status=400 ) - self.assert_response_contains(resp, 'Error converting JSON to Shape') + self.assert_response_contains(resp, 'geom parameter must be a LineString/Point GEOJSON') @patch('app.routes.georaster_utils') - def test_profile_lv03_json_nb_points(self, mock_georaster_utils): + def test_profile_lv03_json_small_line(self, mock_georaster_utils): + + resp = self.prepare_mock_and_test_json_profile( + mock_georaster_utils=mock_georaster_utils, + params={'geom': LINESTRING_SMALL_LINE_LV03}, + expected_status=200 + ) + self.assertEqual(resp.content_type, 'application/json') + self.assertLessEqual(len(resp.json), PROFILE_DEFAULT_AMOUNT_POINTS) + + @patch('app.routes.georaster_utils') + def test_profile_lv03_json_nb_points_smart_filling(self, mock_georaster_utils): # as 150 is too much for this profile (distance between points will be smaller than 2m # resolution of the altitude model), the service will return 203 and a smaller amount of # points @@ -244,29 +247,33 @@ def test_profile_lv03_json_nb_points(self, mock_georaster_utils): expected_status=203 ) self.assertEqual(resp.content_type, 'application/json') - self.assertNotEqual(len(resp.json), 150) + self.assertLessEqual(len(resp.json), 150) @patch('app.routes.georaster_utils') - def test_profile_lv03_json_simplify_linestring(self, mock_georaster_utils): + def test_profile_lv03_json_fewer_nb_points_as_input_point(self, mock_georaster_utils): + input_points = 4 resp = self.prepare_mock_and_test_json_profile( mock_georaster_utils=mock_georaster_utils, params={ - 'geom': create_json(4, 21781), 'nb_points': '2' + 'geom': create_json(input_points, 21781), 'nb_points': '2' }, - expected_status=200 + expected_status=203 ) self.assertEqual(resp.content_type, 'application/json') + self.assertEqual(len(resp.json), input_points) @patch('app.routes.georaster_utils') - def test_profile_lv03_json_nb_points_smart_filling(self, mock_georaster_utils): + def test_profile_lv03_json_only_input_point(self, mock_georaster_utils): + input_points = 4 resp = self.prepare_mock_and_test_json_profile( mock_georaster_utils=mock_georaster_utils, params={ - 'geom': LINESTRING_SMALL_LINE_LV03, 'smart_filling': True, 'nbPoints': '150' + 'geom': create_json(input_points, 21781), 'only_requested_points': True }, - expected_status=203 + expected_status=200 ) self.assertEqual(resp.content_type, 'application/json') + self.assertEqual(len(resp.json), input_points) @patch('app.routes.georaster_utils') def test_profile_lv03_json_nb_points_wrong(self, mock_georaster_utils): @@ -306,38 +313,6 @@ def test_profile_lv03_json_default_nb_points(self, mock_georaster_utils): self.assertGreaterEqual(len(resp.json), PROFILE_DEFAULT_AMOUNT_POINTS) self.assertGreaterEqual(PROFILE_MAX_AMOUNT_POINTS, len(resp.json)) - @patch('app.routes.georaster_utils') - def test_profile_lv03_csv_valid(self, mock_georaster_utils): - resp = self.prepare_mock_and_test_csv_profile( - mock_georaster_utils=mock_georaster_utils, - params={'geom': create_json(4, 21781)}, - expected_status=200 - ) - self.assertEqual(resp.content_type, 'text/csv') - - @patch('app.routes.georaster_utils') - def test_profile_lv03_cvs_wrong_geom(self, mock_georaster_utils): - resp = self.prepare_mock_and_test_csv_profile( - mock_georaster_utils=mock_georaster_utils, params={'geom': 'toto'}, expected_status=400 - ) - self.assert_response_contains(resp, 'Error loading geometry in JSON string') - - @patch('app.routes.georaster_utils') - def test_profile_lv03_csv_misspelled_shape(self, mock_georaster_utils): - resp = self.prepare_mock_and_test_csv_profile( - mock_georaster_utils=mock_georaster_utils, - params={'geom': LINESTRING_MISSPELLED_SHAPE}, - expected_status=400 - ) - self.assert_response_contains(resp, 'Error loading geometry in JSON string') - - resp = self.prepare_mock_and_test_csv_profile( - mock_georaster_utils=mock_georaster_utils, - params={'geom': LINESTRING_WRONG_SHAPE}, - expected_status=400 - ) - self.assert_response_contains(resp, 'Error converting JSON to Shape') - @patch('app.routes.georaster_utils') def test_profile_lv03_json_invalid_linestring(self, mock_georaster_utils): resp = self.prepare_mock_and_test_json_profile( @@ -345,7 +320,7 @@ def test_profile_lv03_json_invalid_linestring(self, mock_georaster_utils): params={'geom': '{"type":"LineString","coordinates":[[550050,206550]]}'}, expected_status=400 ) - self.assert_response_contains(resp, 'Error converting JSON to Shape') + self.assert_response_contains(resp, 'Error converting GEOJSON to Shape') @patch('app.routes.georaster_utils') def test_profile_lv03_json_offset(self, mock_georaster_utils): @@ -455,3 +430,60 @@ def test_profile_all_old_elevation_models_are_returned(self, mock_georaster_util self.fail("All old elevation_models must be returned in alt for compatibility issue") if altitudes['DTM25'] != comb_value: self.fail("All values from all models should be taken from the new COMB layer") + + +class TestProfileCsv(TestProfileBase): + + def prepare_mock_and_test_csv_profile(self, mock_georaster_utils, params, expected_status): + prepare_mock(mock_georaster_utils) + response = self.get_csv_with_params(params) + self.check_response(response, expected_status) + return response + + def get_csv_with_params(self, params): + return self.test_instance.get( + ENDPOINT_FOR_CSV_PROFILE, query_string=params, headers=self.headers + ) + + @patch('app.routes.georaster_utils') + def test_profile_lv03_csv_valid(self, mock_georaster_utils): + resp = self.prepare_mock_and_test_csv_profile( + mock_georaster_utils=mock_georaster_utils, + params={'geom': create_json(4, 21781)}, + expected_status=200 + ) + self.assertEqual(resp.content_type, 'text/csv') + + @patch('app.routes.georaster_utils') + def test_profile_lv03_cvs_wrong_geom(self, mock_georaster_utils): + resp = self.prepare_mock_and_test_csv_profile( + mock_georaster_utils=mock_georaster_utils, params={'geom': 'toto'}, expected_status=400 + ) + self.assert_response_contains(resp, 'Invalid geom parameter, must be a GEOJSON') + + @patch('app.routes.georaster_utils') + def test_profile_lv03_csv_misspelled_shape(self, mock_georaster_utils): + resp = self.prepare_mock_and_test_csv_profile( + mock_georaster_utils=mock_georaster_utils, + params={'geom': LINESTRING_MISSPELLED_SHAPE}, + expected_status=400 + ) + self.assert_response_contains(resp, 'Invalid geom parameter, must be a GEOJSON') + + resp = self.prepare_mock_and_test_csv_profile( + mock_georaster_utils=mock_georaster_utils, + params={'geom': LINESTRING_WRONG_SHAPE}, + expected_status=400 + ) + self.assert_response_contains(resp, 'geom parameter must be a LineString/Point GEOJSON') + + @patch('app.routes.georaster_utils') + def test_profile_lv03_csv_callback(self, mock_georaster_utils): + resp = self.prepare_mock_and_test_csv_profile( + mock_georaster_utils=mock_georaster_utils, + params={ + 'geom': create_json(4, 21781), 'callback': '_cb' + }, + expected_status=400 + ) + self.assert_response_contains(resp, 'callback parameter not supported')