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 d77f4db883d33171d152570c461050f1438cbb4b..09cf7db802959d0a8ebb76b10fb614459fef737c 100644 (file)
--- 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)
 #
 #
 # 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.
 """
 # 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]
 
 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:
 
 class FormatDispatcher:

-    """ A factory class for result formatters.
+    """ Helper class to conveniently create formatting functions in
+        a module using decorators.

     """
 
     def __init__(self) -> None:
     """
 
     def __init__(self) -> None:


@@ -63,7 +35,22 @@ class FormatDispatcher:
         return decorator
 
 
         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 (file)
index 0000000..c83562d
--- /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 9114d45feed4541cc4a6b25b4d6c6ed51f9c66c2..cb2b15a71725b33e5b3c5aac89f0232b89961281 100644 (file)
--- 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 collections import OrderedDict
 import json
 

-from nominatim.result_formatter.base import FormatDispatcher
+from nominatim.api.result_formatting import FormatDispatcher

 from nominatim.api import StatusResult
 
 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}"
 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'
 
 
     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
 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 050665f896c72b980831100b5da992efc6f5ad9d..cc65f5f6e357f2fed10f08067a130c21395957f1 100644 (file)
--- 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
 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
 
 # Do not repeat documentation of subcommand classes.
 # pylint: disable=C0111


@@ -276,7 +276,7 @@ class APIStatus:
     """
 
     def add_args(self, parser: argparse.ArgumentParser) -> None:
     """
 
     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')
         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()
 
     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
         return 0

diff --git a/nominatim/result_formatter/__init__.py b/nominatim/result_formatter/__init__.py
deleted file mode 100644 (file)
index e69de29..0000000

diff --git a/nominatim/server/falcon/server.py b/nominatim/server/falcon/server.py
index c56034b53127e57c920a157b0bb95f965d6fa120..62b5677023f3dd73bacb56333f7915b2835405ce 100644 (file)
--- 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 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,
 
 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)
 
     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:
 
 
     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)
             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: " +
             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,
 
 
     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()`.
         """
         """ 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)
 
 
         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 797157af0844fdf5ec2c0e82d8408384d1bd488f..8dd29b542639dcdd0ca077755ac015947e3c81b1 100644 (file)
--- 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 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')
 
 
 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.
     """
     """ 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'))
     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
         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)
 
     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: " +
         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
 
 
     return None
 


@@ -76,9 +75,6 @@ def get_application(project_dir: Path,
     app = sanic.Sanic("NominatimInstance")
 
     app.ctx.api = NominatimAPIAsync(project_dir, environ)
     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)
 
 
     app.blueprint(api)
 

diff --git a/nominatim/server/starlette/server.py b/nominatim/server/starlette/server.py
index e6dbbc782a71439a29200a6802f3a23a8b6baeec..60a78a475d8a81fa2cbde9073f853a7001fea605 100644 (file)
--- 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
 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'
 }
 
 
 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)
 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: " +
         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.format = fmt

-    request.state.formatter = fmtter

 
 
 def format_response(request: Request, result: Any) -> Response:
 
 
 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
     """
     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'))
 
 
                     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 (file)
index 0000000..9547291
--- /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 (file)
index fc5a367..0000000
--- 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, )