♻️ servicelib tools: json serialization and fixes responses (ITISFoundation#2582)

* servicelib utils to get httpexc for status-code

* Adds richer encoder to client session

* pytest-simcore: separated fixture to reduce dependencies

* servicelib: adds tests

* servicelib: fixes uuid encoding and remove ujson reqs

* fixes pytest-mocks

* adds pydantic encode to json serialization

* fixes type annotation

* modifies get_http_error

* renames api_version_prefix -> VTAG

* removes repeated fixture and replaces by a pytest_plugins reference

* servicelib adds faker in tests

* removed unused ujson requirement

* adds cache to get_http_error

* uses servicelib tools in handlers

* unused meta module

* removes assert

* removes meta

* @GitHK review: using f-strings

* @GitHK review: dict check & get

* @colinRawlings review: improved tests
and collected all http_errors upon import instead

* fixes R1716: Simplify chained comparison between the operands (chained-comparison)

Author: pcrespov

.pylintrc

@@ -3,7 +3,7 @@
 # A comma-separated list of package or module names from where C extensions may
 # be loaded. Extensions are loading into the active Python interpreter and may
 # run arbitrary code
-extension-pkg-whitelist=pydantic,orjson
+extension-pkg-whitelist=pydantic,orjson,ujson
 
 # Add files or directories to the blacklist. They should be base names, not
 # paths.

packages/pytest-simcore/src/pytest_simcore/aioresponses_mocker.py

@@ -0,0 +1,23 @@
+import pytest
+from aioresponses import aioresponses as AioResponsesMock
+
+PASSTHROUGH_REQUESTS_PREFIXES = ["http://127.0.0.1", "ws://"]
+
+
+@pytest.fixture
+def aioresponses_mocker() -> AioResponsesMock:
+    """Generick aioresponses mock
+
+    SEE https://github.com/pnuckowski/aioresponses
+
+    Usage
+
+        async def test_this(aioresponses_mocker):
+            aioresponses_mocker.get("https://foo.io")
+
+            async with aiohttp.ClientSession() as session:
+                async with session.get("https://foo.io") as response:
+                    assert response.status == 200
+    """
+    with AioResponsesMock(passthrough=PASSTHROUGH_REQUESTS_PREFIXES) as mock:
+        yield mock

packages/pytest-simcore/src/pytest_simcore/services_api_mocks_for_aiohttp_clients.py

@@ -7,11 +7,16 @@
 from typing import Any, Dict, List
 
 import pytest
+from aiohttp import web
 from aioresponses import aioresponses as AioResponsesMock
 from aioresponses.core import CallbackResult
 from models_library.projects_state import RunningState
 from yarl import URL
 
+pytest_plugins = [
+    "pytest_simcore.aioresponses_mocker",
+]
+
 # WARNING: any request done through the client will go through aioresponses. It is
 # unfortunate but that means any valid request (like calling the test server) prefix must be set as passthrough.
 # Other than that it seems to behave nicely
@@ -56,25 +61,6 @@
 }
 
 
-@pytest.fixture
-def aioresponses_mocker() -> AioResponsesMock:
-    """Generick aioresponses mock
-
-    SEE https://github.com/pnuckowski/aioresponses
-
-    Usage
-
-        async def test_this(aioresponses_mocker):
-            aioresponses_mocker.get("https://foo.io")
-
-            async with aiohttp.ClientSession() as session:
-                async with session.get("https://foo.aio") as response:
-                    assert response.status == 200
-    """
-    with AioResponsesMock(passthrough=PASSTHROUGH_REQUESTS_PREFIXES) as mock:
-        yield mock
-
-
 def creation_cb(url, **kwargs) -> CallbackResult:
 
     assert "json" in kwargs, f"missing body in call to {url}"
@@ -112,8 +98,9 @@ def creation_cb(url, **kwargs) -> CallbackResult:
 
     return CallbackResult(
         status=201,
+        # NOTE: aioresponses uses json.dump which does NOT encode serialization of UUIDs
         payload={
-            "id": kwargs["json"]["project_id"],
+            "id": str(kwargs["json"]["project_id"]),
             "state": state,
             "pipeline_details": {
                 "adjacency_list": pipeline,
@@ -161,16 +148,17 @@ async def director_v2_service_mock(
     aioresponses_mocker.post(
         create_computation_pattern,
         callback=creation_cb,
+        status=web.HTTPCreated.status_code,
         repeat=True,
     )
     aioresponses_mocker.post(
         stop_computation_pattern,
-        status=204,
+        status=web.HTTPAccepted.status_code,
         repeat=True,
     )
     aioresponses_mocker.get(
         get_computation_pattern,
-        status=202,
+        status=web.HTTPAccepted.status_code,
         callback=get_computation_cb,
         repeat=True,
     )
@@ -189,7 +177,8 @@ def get_download_link_cb(url: URL, **kwargs) -> CallbackResult:
         file_id = url.path.rsplit("/files/")[1]
 
         return CallbackResult(
-            status=200, payload={"data": {"link": f"file://{file_id}"}}
+            status=web.HTTPOk.status_code,
+            payload={"data": {"link": f"file://{file_id}"}},
         )
 
     get_download_link_pattern = re.compile(
@@ -205,7 +194,7 @@ def get_download_link_cb(url: URL, **kwargs) -> CallbackResult:
     )
     aioresponses_mocker.get(
         get_locations_link_pattern,
-        status=200,
+        status=web.HTTPOk.status_code,
         payload={"data": [{"name": "simcore.s3", "id": "0"}]},
     )
     return aioresponses_mocker

packages/service-library/requirements/_base.in

@@ -4,14 +4,8 @@
 --constraint ../../../requirements/constraints.txt
 --constraint ./constraints.txt
 
-ujson
-
+aiodebug
 pydantic
+pyinstrument
 pyyaml
-
-# misc
-aiodebug
 tenacity
-
-# used for monitoring of slow callbacks
-pyinstrument

packages/service-library/requirements/_base.txt

@@ -20,5 +20,3 @@ tenacity==8.0.1
     # via -r requirements/_base.in
 typing-extensions==3.10.0.2
     # via pydantic
-ujson==4.1.0
-    # via -r requirements/_base.in

packages/service-library/requirements/_test.in

@@ -21,6 +21,7 @@ pytest-docker
 pytest-mock
 pytest-sugar
 
+faker
 
 pylint  # NOTE: The version in pylint at _text.txt is used as a reference for ci/helpers/install_pylint.bash
 coveralls

packages/service-library/requirements/_test.txt

@@ -62,6 +62,8 @@ docopt==0.6.2
     # via
     #   coveralls
     #   docker-compose
+faker==9.0.0
+    # via -r requirements/_test.in
 idna==2.10
     # via
     #   -c requirements/_aiohttp.txt
@@ -147,6 +149,8 @@ pytest-runner==5.3.1
     # via -r requirements/_test.in
 pytest-sugar==0.9.4
     # via -r requirements/_test.in
+python-dateutil==2.8.2
+    # via faker
 python-dotenv==0.19.0
     # via docker-compose
 pyyaml==5.4.1
@@ -164,16 +168,20 @@ requests==2.26.0
 six==1.16.0
     # via
     #   -c requirements/_aiohttp.txt
+    #   -c requirements/_fastapi.txt
     #   bcrypt
     #   dockerpty
     #   isodate
     #   jsonschema
     #   openapi-schema-validator
     #   openapi-spec-validator
     #   pynacl
+    #   python-dateutil
     #   websocket-client
 termcolor==1.1.0
     # via pytest-sugar
+text-unidecode==1.3
+    # via faker
 texttable==1.6.4
     # via docker-compose
 toml==0.10.2

packages/service-library/src/servicelib/aiohttp/client_session.py

@@ -8,12 +8,13 @@
 
 from aiohttp import ClientSession, ClientTimeout, web
 
-from .application_keys import APP_CLIENT_SESSION_KEY
+from ..json_serialization import json_dumps
 from ..utils import (
     get_http_client_request_aiohttp_connect_timeout,
     get_http_client_request_aiohttp_sock_connect_timeout,
     get_http_client_request_total_timeout,
 )
+from .application_keys import APP_CLIENT_SESSION_KEY
 
 log = logging.getLogger(__name__)
 
@@ -33,8 +34,12 @@ def get_client_session(app: MutableMapping[str, Any]) -> ClientSession:
             total=get_http_client_request_total_timeout(),
             connect=get_http_client_request_aiohttp_connect_timeout(),
             sock_connect=get_http_client_request_aiohttp_sock_connect_timeout(),
+        )  # type: ignore
+
+        app[APP_CLIENT_SESSION_KEY] = session = ClientSession(
+            timeout=timeout_settings,
+            json_serialize=json_dumps,
         )
-        app[APP_CLIENT_SESSION_KEY] = session = ClientSession(timeout=timeout_settings)
     return session
 
 
@@ -61,7 +66,7 @@ async def persistent_client_session(app: web.Application):
         )
 
     await session.close()
-    log.info("Session is actually closed? %s", session.closed)
+    assert session.closed  # nosec
 
 
 # FIXME: if get_client_session upon startup fails and session is NOT closed. Implement some kind of gracefull shutdonw https://docs.aiohttp.org/en/latest/client_advanced.html#graceful-shutdown

packages/service-library/src/servicelib/aiohttp/rest_responses.py

@@ -1,10 +1,12 @@
 """ Utils to check, convert and compose server responses for the RESTApi
 
 """
+import inspect
 from typing import Any, Dict, List, Mapping, Optional, Tuple, Type, Union
 
 import attr
-from aiohttp import web
+from aiohttp import web, web_exceptions
+from aiohttp.web_exceptions import HTTPError, HTTPException
 
 from .rest_codecs import json, jsonify
 from .rest_models import ErrorItemType, ErrorType, LogMessageType
@@ -89,10 +91,10 @@ def create_data_response(
 def create_error_response(
     errors: Union[List[Exception], Exception],
     reason: Optional[str] = None,
-    http_error_cls: Type[web.HTTPError] = web.HTTPInternalServerError,
+    http_error_cls: Type[HTTPError] = web.HTTPInternalServerError,
     *,
-    skip_internal_error_details: bool = False
-) -> web.HTTPError:
+    skip_internal_error_details: bool = False,
+) -> HTTPError:
     """
     - Response body conforms OAS schema model
     - Can skip internal details when 500 status e.g. to avoid transmitting server
@@ -134,3 +136,34 @@ def create_log_response(msg: str, level: str) -> web.Response:
     msg = LogMessageType(msg, level)
     response = web.json_response(data={"data": attr.asdict(msg), "error": None})
     return response
+
+
+# Inverse map from code to HTTPException classes
+def _collect_http_exceptions(exception_cls: Type[HTTPException] = HTTPException):
+    def _pred(obj) -> bool:
+        return (
+            inspect.isclass(obj)
+            and issubclass(obj, exception_cls)
+            and getattr(obj, "status_code", 0) > 0
+        )
+
+    found: List[Tuple[str, Any]] = inspect.getmembers(web_exceptions, _pred)
+    assert found  # nosec
+
+    http_statuses = {cls.status_code: cls for _, cls in found}
+    assert len(http_statuses) == len(found), "No duplicates"  # nosec
+
+    return http_statuses
+
+
+_STATUS_CODE_TO_HTTP_ERRORS: Dict[int, Type[HTTPError]] = _collect_http_exceptions(
+    HTTPError
+)
+
+
+def get_http_error(status_code: int) -> Optional[Type[HTTPError]]:
+    """Returns aiohttp error class corresponding to a 4XX or 5XX status code
+
+    NOTICE that any non-error code (i.e. 2XX, 3XX and 4XX) will return None
+    """
+    return _STATUS_CODE_TO_HTTP_ERRORS.get(status_code)

packages/service-library/src/servicelib/json_serialization.py

@@ -0,0 +1,15 @@
+import json
+from typing import Any
+
+from pydantic.json import pydantic_encoder
+
+
+def json_dumps(obj: Any, **kwargs):
+    """json.dumps with rich encoder.
+    A big applause for pydantic authors here!!!
+    """
+    return json.dumps(obj, default=pydantic_encoder, **kwargs)
+
+
+# TODO: support for orjson
+# TODO: support for ujson (fast but poor encoding, only for basic types)