From: Sarah Hoffmann Date: Tue, 24 Jan 2023 16:20:51 +0000 (+0100) Subject: reorganize code around result formatting X-Git-Tag: v4.3.0~110^2~3 X-Git-Url: https://git.openstreetmap.org./nominatim.git/commitdiff_plain/8f4426fbc8038c159eae999e74d5a4f1fb992530 reorganize code around result formatting Code is now organized by api version. So formatting has moved to the api.v1 module. Instead of holding a separate ResultFormatter object per result format, simply move the functions to the formater collector and hand in the requested format as a parameter. Thus reorganized, the api.v1 module can export three simple functions for result formatting which in turn makes the code that uses the formatters much simpler. --- diff --git a/nominatim/result_formatter/base.py b/nominatim/api/result_formatting.py similarity index 53% rename from nominatim/result_formatter/base.py rename to nominatim/api/result_formatting.py index d77f4db8..09cf7db8 100644 --- a/nominatim/result_formatter/base.py +++ b/nominatim/api/result_formatting.py @@ -2,49 +2,21 @@ # # This file is part of Nominatim. (https://nominatim.org) # -# Copyright (C) 2022 by the Nominatim developer community. +# Copyright (C) 2023 by the Nominatim developer community. # For a full list of authors see the git log. """ -Helper classes and function for writing result formatting modules. +Helper classes and functions for formating results into API responses. """ -from typing import Type, TypeVar, Dict, Mapping, List, Callable, Generic, Any +from typing import Type, TypeVar, Dict, List, Callable, Any from collections import defaultdict T = TypeVar('T') # pylint: disable=invalid-name FormatFunc = Callable[[T], str] -class ResultFormatter(Generic[T]): - """ This class dispatches format calls to the appropriate formatting - function previously defined with the `format_func` decorator. - """ - - def __init__(self, funcs: Mapping[str, FormatFunc[T]]) -> None: - self.functions = funcs - - - def list_formats(self) -> List[str]: - """ Return a list of formats supported by this formatter. - """ - return list(self.functions.keys()) - - - def supports_format(self, fmt: str) -> bool: - """ Check if the given format is supported by this formatter. - """ - return fmt in self.functions - - - def format(self, result: T, fmt: str) -> str: - """ Convert the given result into a string using the given format. - - The format is expected to be in the list returned by - `list_formats()`. - """ - return self.functions[fmt](result) - class FormatDispatcher: - """ A factory class for result formatters. + """ Helper class to conveniently create formatting functions in + a module using decorators. """ def __init__(self) -> None: @@ -63,7 +35,22 @@ class FormatDispatcher: return decorator - def __call__(self, result_class: Type[T]) -> ResultFormatter[T]: - """ Create an instance of a format class for the given result type. + def list_formats(self, result_type: Type[Any]) -> List[str]: + """ Return a list of formats supported by this formatter. + """ + return list(self.format_functions[result_type].keys()) + + + def supports_format(self, result_type: Type[Any], fmt: str) -> bool: + """ Check if the given format is supported by this formatter. + """ + return fmt in self.format_functions[result_type] + + + def format_result(self, result: Any, fmt: str) -> str: + """ Convert the given result into a string using the given format. + + The format is expected to be in the list returned by + `list_formats()`. """ - return ResultFormatter(self.format_functions[result_class]) + return self.format_functions[type(result)][fmt](result) diff --git a/nominatim/api/v1/__init__.py b/nominatim/api/v1/__init__.py new file mode 100644 index 00000000..c83562d1 --- /dev/null +++ b/nominatim/api/v1/__init__.py @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# This file is part of Nominatim. (https://nominatim.org) +# +# Copyright (C) 2023 by the Nominatim developer community. +# For a full list of authors see the git log. +""" +Implementation of API version v1 (aka the legacy version). +""" + +import nominatim.api.v1.format as _format + +list_formats = _format.dispatch.list_formats +supports_format = _format.dispatch.supports_format +format_result = _format.dispatch.format_result diff --git a/nominatim/result_formatter/v1.py b/nominatim/api/v1/format.py similarity index 84% rename from nominatim/result_formatter/v1.py rename to nominatim/api/v1/format.py index 9114d45f..cb2b15a7 100644 --- a/nominatim/result_formatter/v1.py +++ b/nominatim/api/v1/format.py @@ -11,12 +11,12 @@ from typing import Dict, Any from collections import OrderedDict import json -from nominatim.result_formatter.base import FormatDispatcher +from nominatim.api.result_formatting import FormatDispatcher from nominatim.api import StatusResult -create = FormatDispatcher() +dispatch = FormatDispatcher() -@create.format_func(StatusResult, 'text') +@dispatch.format_func(StatusResult, 'text') def _format_status_text(result: StatusResult) -> str: if result.status: return f"ERROR: {result.message}" @@ -24,7 +24,7 @@ def _format_status_text(result: StatusResult) -> str: return 'OK' -@create.format_func(StatusResult, 'json') +@dispatch.format_func(StatusResult, 'json') def _format_status_json(result: StatusResult) -> str: out: Dict[str, Any] = OrderedDict() out['status'] = result.status diff --git a/nominatim/clicmd/api.py b/nominatim/clicmd/api.py index 050665f8..cc65f5f6 100644 --- a/nominatim/clicmd/api.py +++ b/nominatim/clicmd/api.py @@ -15,7 +15,7 @@ from nominatim.tools.exec_utils import run_api_script from nominatim.errors import UsageError from nominatim.clicmd.args import NominatimArgs from nominatim.api import NominatimAPI, StatusResult -import nominatim.result_formatter.v1 as formatting +import nominatim.api.v1 as api_output # Do not repeat documentation of subcommand classes. # pylint: disable=C0111 @@ -276,7 +276,7 @@ class APIStatus: """ def add_args(self, parser: argparse.ArgumentParser) -> None: - formats = formatting.create(StatusResult).list_formats() + formats = api_output.list_formats(StatusResult) group = parser.add_argument_group('API parameters') group.add_argument('--format', default=formats[0], choices=formats, help='Format of result') @@ -284,5 +284,5 @@ class APIStatus: def run(self, args: NominatimArgs) -> int: status = NominatimAPI(args.project_dir).status() - print(formatting.create(StatusResult).format(status, args.format)) + print(api_output.format_result(status, args.format)) return 0 diff --git a/nominatim/result_formatter/__init__.py b/nominatim/result_formatter/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/nominatim/server/falcon/server.py b/nominatim/server/falcon/server.py index c56034b5..62b56770 100644 --- a/nominatim/server/falcon/server.py +++ b/nominatim/server/falcon/server.py @@ -14,7 +14,7 @@ import falcon import falcon.asgi from nominatim.api import NominatimAPIAsync, StatusResult -import nominatim.result_formatter.v1 as formatting +import nominatim.api.v1 as api_impl CONTENT_TYPE = { 'text': falcon.MEDIA_TEXT, @@ -27,10 +27,6 @@ class NominatimV1: def __init__(self, project_dir: Path, environ: Optional[Mapping[str, str]]) -> None: self.api = NominatimAPIAsync(project_dir, environ) - self.formatters = {} - - for rtype in (StatusResult, ): - self.formatters[rtype] = formatting.create(rtype) def parse_format(self, req: falcon.asgi.Request, rtype: Type[Any], default: str) -> None: @@ -39,12 +35,11 @@ class NominatimV1: format value to assume when no parameter is present. """ req.context.format = req.get_param('format', default=default) - req.context.formatter = self.formatters[rtype] - if not req.context.formatter.supports_format(req.context.format): + if not api_impl.supports_format(rtype, req.context.format): raise falcon.HTTPBadRequest( description="Parameter 'format' must be one of: " + - ', '.join(req.context.formatter.list_formats())) + ', '.join(api_impl.list_formats(rtype))) def format_response(self, req: falcon.asgi.Request, resp: falcon.asgi.Response, @@ -52,7 +47,7 @@ class NominatimV1: """ Render response into a string according to the formatter set in `parse_format()`. """ - resp.text = req.context.formatter.format(result, req.context.format) + resp.text = api_impl.format_result(result, req.context.format) resp.content_type = CONTENT_TYPE.get(req.context.format, falcon.MEDIA_JSON) diff --git a/nominatim/server/sanic/server.py b/nominatim/server/sanic/server.py index 797157af..8dd29b54 100644 --- a/nominatim/server/sanic/server.py +++ b/nominatim/server/sanic/server.py @@ -13,7 +13,7 @@ from pathlib import Path import sanic from nominatim.api import NominatimAPIAsync, StatusResult -import nominatim.result_formatter.v1 as formatting +import nominatim.api.v1 as api_impl api = sanic.Blueprint('NominatimAPI') @@ -32,7 +32,7 @@ def api_response(request: sanic.Request, result: Any) -> sanic.HTTPResponse: """ Render a response from the query results using the configured formatter. """ - body = request.ctx.formatter.format(result, request.ctx.format) + body = api_impl.format_result(result, request.ctx.format) return sanic.response.text(body, content_type=CONTENT_TYPE.get(request.ctx.format, 'application/json')) @@ -46,12 +46,11 @@ async def extract_format(request: sanic.Request) -> Optional[sanic.HTTPResponse] is present. """ assert request.route is not None - request.ctx.formatter = request.app.ctx.formatters[request.route.ctx.result_type] request.ctx.format = request.args.get('format', request.route.ctx.default_format) - if not request.ctx.formatter.supports_format(request.ctx.format): + if not api_impl.supports_format(request.route.ctx.result_type, request.ctx.format): return usage_error("Parameter 'format' must be one of: " + - ', '.join(request.ctx.formatter.list_formats())) + ', '.join(api_impl.list_formats(request.route.ctx.result_type))) return None @@ -76,9 +75,6 @@ def get_application(project_dir: Path, app = sanic.Sanic("NominatimInstance") app.ctx.api = NominatimAPIAsync(project_dir, environ) - app.ctx.formatters = {} - for rtype in (StatusResult, ): - app.ctx.formatters[rtype] = formatting.create(rtype) app.blueprint(api) diff --git a/nominatim/server/starlette/server.py b/nominatim/server/starlette/server.py index e6dbbc78..60a78a47 100644 --- a/nominatim/server/starlette/server.py +++ b/nominatim/server/starlette/server.py @@ -17,40 +17,32 @@ from starlette.responses import Response from starlette.requests import Request from nominatim.api import NominatimAPIAsync, StatusResult -import nominatim.result_formatter.v1 as formatting +import nominatim.api.v1 as api_impl CONTENT_TYPE = { 'text': 'text/plain; charset=utf-8', 'xml': 'text/xml; charset=utf-8' } -FORMATTERS = { - StatusResult: formatting.create(StatusResult) -} - - def parse_format(request: Request, rtype: Type[Any], default: str) -> None: """ Get and check the 'format' parameter and prepare the formatter. `rtype` describes the expected return type and `default` the format value to assume when no parameter is present. """ fmt = request.query_params.get('format', default=default) - fmtter = FORMATTERS[rtype] - if not fmtter.supports_format(fmt): + if not api_impl.supports_format(rtype, fmt): raise HTTPException(400, detail="Parameter 'format' must be one of: " + - ', '.join(fmtter.list_formats())) + ', '.join(api_impl.list_formats(rtype))) request.state.format = fmt - request.state.formatter = fmtter def format_response(request: Request, result: Any) -> Response: - """ Render response into a string according to the formatter - set in `parse_format()`. + """ Render response into a string according. """ fmt = request.state.format - return Response(request.state.formatter.format(result, fmt), + return Response(api_impl.format_result(result, fmt), media_type=CONTENT_TYPE.get(fmt, 'application/json')) diff --git a/test/python/api/test_result_formatting_v1.py b/test/python/api/test_result_formatting_v1.py new file mode 100644 index 00000000..95472916 --- /dev/null +++ b/test/python/api/test_result_formatting_v1.py @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# This file is part of Nominatim. (https://nominatim.org) +# +# Copyright (C) 2023 by the Nominatim developer community. +# For a full list of authors see the git log. +""" +Tests for formatting results for the V1 API. +""" +import datetime as dt +import pytest + +import nominatim.api.v1 as api_impl +from nominatim.api import StatusResult +from nominatim.version import NOMINATIM_VERSION + +STATUS_FORMATS = {'text', 'json'} + +# StatusResult + +def test_status_format_list(): + assert set(api_impl.list_formats(StatusResult)) == STATUS_FORMATS + + +@pytest.mark.parametrize('fmt', list(STATUS_FORMATS)) +def test_status_supported(fmt): + assert api_impl.supports_format(StatusResult, fmt) + + +def test_status_unsupported(): + assert not api_impl.supports_format(StatusResult, 'gagaga') + + +def test_status_format_text(): + assert api_impl.format_result(StatusResult(0, 'message here'), 'text') == 'OK' + + +def test_status_format_text(): + assert api_impl.format_result(StatusResult(500, 'message here'), 'text') == 'ERROR: message here' + + +def test_status_format_json_minimal(): + status = StatusResult(700, 'Bad format.') + + result = api_impl.format_result(status, 'json') + + assert result == '{"status": 700, "message": "Bad format.", "software_version": "%s"}' % (NOMINATIM_VERSION, ) + + +def test_status_format_json_full(): + status = StatusResult(0, 'OK') + status.data_updated = dt.datetime(2010, 2, 7, 20, 20, 3, 0, tzinfo=dt.timezone.utc) + status.database_version = '5.6' + + result = api_impl.format_result(status, 'json') + + assert result == '{"status": 0, "message": "OK", "data_updated": "2010-02-07T20:20:03+00:00", "software_version": "%s", "database_version": "5.6"}' % (NOMINATIM_VERSION, ) diff --git a/test/python/result_formatter/test_v1.py b/test/python/result_formatter/test_v1.py deleted file mode 100644 index fc5a3671..00000000 --- a/test/python/result_formatter/test_v1.py +++ /dev/null @@ -1,63 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0-only -# -# This file is part of Nominatim. (https://nominatim.org) -# -# Copyright (C) 2023 by the Nominatim developer community. -# For a full list of authors see the git log. -""" -Tests for formatting results for the V1 API. -""" -import datetime as dt -import pytest - -import nominatim.result_formatter.v1 as format_module -from nominatim.api import StatusResult -from nominatim.version import NOMINATIM_VERSION - -STATUS_FORMATS = {'text', 'json'} - -class TestStatusResultFormat: - - - @pytest.fixture(autouse=True) - def make_formatter(self): - self.formatter = format_module.create(StatusResult) - - - def test_format_list(self): - assert set(self.formatter.list_formats()) == STATUS_FORMATS - - - @pytest.mark.parametrize('fmt', list(STATUS_FORMATS)) - def test_supported(self, fmt): - assert self.formatter.supports_format(fmt) - - - def test_unsupported(self): - assert not self.formatter.supports_format('gagaga') - - - def test_format_text(self): - assert self.formatter.format(StatusResult(0, 'message here'), 'text') == 'OK' - - - def test_format_text(self): - assert self.formatter.format(StatusResult(500, 'message here'), 'text') == 'ERROR: message here' - - - def test_format_json_minimal(self): - status = StatusResult(700, 'Bad format.') - - result = self.formatter.format(status, 'json') - - assert result == '{"status": 700, "message": "Bad format.", "software_version": "%s"}' % (NOMINATIM_VERSION, ) - - - def test_format_json_full(self): - status = StatusResult(0, 'OK') - status.data_updated = dt.datetime(2010, 2, 7, 20, 20, 3, 0, tzinfo=dt.timezone.utc) - status.database_version = '5.6' - - result = self.formatter.format(status, 'json') - - assert result == '{"status": 0, "message": "OK", "data_updated": "2010-02-07T20:20:03+00:00", "software_version": "%s", "database_version": "5.6"}' % (NOMINATIM_VERSION, )