diff --git a/http2py/api_pkg_maker.py b/http2py/api_pkg_maker.py index 0beec38..55293a2 100644 --- a/http2py/api_pkg_maker.py +++ b/http2py/api_pkg_maker.py @@ -84,8 +84,9 @@ def {func_name}(*args, **kwargs): return api.routes[{func_name!r}](*args, **kwargs) """ -# The first http2py release with ``HttpClient.routes``, which generated packages call. -MIN_HTTP2PY_VERSION = "0.1.37" +# The first http2py release whose route names are what generated packages call: +# ``HttpClient.routes``, with names unique among routes (``get_a_b_2``) and ``get_root``. +MIN_HTTP2PY_VERSION = "0.1.40" SETUP_PY = """from setuptools import setup diff --git a/http2py/client.py b/http2py/client.py index 87402f7..80000e8 100644 --- a/http2py/client.py +++ b/http2py/client.py @@ -13,6 +13,7 @@ from http2py.authentication import mk_auth, DFLT_CONFIG_FILENAME from http2py.py2request import ( DFLT_BINARY_DUMPS, + _route_name, mk_method_spec_from_openapi_method_spec, mk_request_function, ) @@ -112,6 +113,19 @@ def __init__( if self.refresh_inputs is None: raise ValueError("No refresh inputs provided when instantiating HttpClient") self.routes = {} + # Names derived from the spec must avoid every explicit name, and a suffixed + # name must not take the natural name of a route that comes later. + self._explicit_route_names, self._natural_route_names = set(), set() + for pathname, path_spec in openapi_spec["paths"].items(): + for http_method, spec in path_spec.items(): + if not isinstance(spec, dict): + continue + if spec.get("x-method_name"): + self._explicit_route_names.add(spec["x-method_name"]) + else: + self._natural_route_names.add( + _route_name(spec, http_method, pathname, self.base_url + pathname) + ) for pathname, path_spec in openapi_spec["paths"].items(): url_template = self.base_url + pathname for http_method, openapi_method_spec in path_spec.items(): @@ -160,6 +174,10 @@ def register_method( content_type=content_type, pathname=pathname, ) + if not openapi_method_spec.get("x-method_name"): + method_spec["method_name"] = self._unique_derived_name( + method_spec["method_name"], url_template + ) func = mk_request_function( method_spec, dispatch=self.handle_request, @@ -191,6 +209,30 @@ def register_method( stacklevel=3, ) + def _unique_derived_name(self, name, url_template): + """``name``, a name derived from the spec (operationId, or method and path), + made unique among the routes so far and the explicit ``x-method_name`` ones: the + first route keeps it, later ones get ``_2``, ``_3``, ... in spec order (so + a route added before another with the same name renames the other; give + routes an ``x-method_name`` to pin their names). Explicit names are never + changed. A client that skips ``__init__`` knows no explicit names yet.""" + taken = set(getattr(self, "routes", ())) | getattr( + self, "_explicit_route_names", set() + ) + if name not in taken: + return name + taken |= getattr(self, "_natural_route_names", set()) + n = 2 + while f"{name}_{n}" in taken: + n += 1 + unique = f"{name}_{n}" + warnings.warn( + f"Another route is already named {name!r}: {url_template!r} is named " + f"{unique!r} (give each route its own x-method_name to choose).", + stacklevel=4, + ) + return unique + def _can_be_route_attribute(self, name): """Whether a route called ``name`` may be set as an attribute of the client: the name is public, and is not an attribute the client works with already diff --git a/http2py/py2request.py b/http2py/py2request.py index eb4c47e..e2e7923 100644 --- a/http2py/py2request.py +++ b/http2py/py2request.py @@ -673,7 +673,8 @@ def _route_name(openapi_method_spec, method, pathname, url_template): name = _identifier(openapi_method_spec.get("operationId") or "") if pathname is None: pathname = urlparse(url_template).path - return name or default_path_to_func_name(method, pathname) + # the root path would be named after the bare method (``get``) + return name or default_path_to_func_name(method, pathname.strip("/") or "root") def mk_method_spec_from_openapi_method_spec( diff --git a/tests/test_route_namespace.py b/tests/test_route_namespace.py index c460ad0..941d5db 100644 --- a/tests/test_route_namespace.py +++ b/tests/test_route_namespace.py @@ -174,3 +174,99 @@ def test_a_second_route_of_the_same_name_replaces_the_first_with_a_warning(): name = other["get"]["x-method_name"] assert client.routes[name].method_spec["url_template"].endswith("/plain2") assert getattr(client, name) is client.routes[name] + + +def _spec_with_routes(*routes): + """A spec whose routes are ``(path, operationId or None, x-method_name or None)``.""" + spec = copy.deepcopy(SPEC) + template = spec["paths"]["/plain"]["get"] + spec["paths"] = {} + for path, operation_id, method_name in routes: + op = copy.deepcopy(template) + op.pop("x-method_name", None) + if operation_id: + op["operationId"] = operation_id + if method_name: + op["x-method_name"] = method_name + spec["paths"][path] = {"get": op} + return spec + + +def test_derived_names_that_collide_are_suffixed_in_spec_order(): + # i2mint/http2py#28 + spec = _spec_with_routes( + ("/a-b", None, None), + ("/a_b", None, None), + ("/a/b", None, None), + ("/u/{id}", None, None), + ("/u/id", None, None), + ("/x", "list-things", None), + ("/y", "list_things", None), + ("/z", "get_zz", None), + ("/zz", None, None), + ) + with pytest.warns(UserWarning, match="get_a_b_2"): + client = _client(spec) + assert list(client.routes) == [ + "get_a_b", + "get_a_b_2", + "get_a_b_3", + "get_u_id", + "get_u_id_2", + "list_things", + "list_things_2", + "get_zz", + "get_zz_2", + ] + for name, route in client.routes.items(): + assert route.__name__ == name and getattr(client, name) is route + + +def test_explicit_names_are_never_suffixed_and_keep_their_name_over_derived(): + spec = _spec_with_routes( + ("/a-b", None, None), # derived get_a_b, but an explicit one claims it + ("/other", None, "get_a_b"), + ) + client = _client(spec) + assert list(client.routes) == ["get_a_b_2", "get_a_b"] + assert client.routes["get_a_b"].method_spec["url_template"].endswith("/other") + + +def test_two_explicit_names_that_collide_still_warn_and_the_last_wins(): + spec = _spec_with_routes(("/p", None, "same"), ("/q", None, "same")) + with pytest.warns(UserWarning, match="Two routes are named 'same'"): + client = _client(spec) + assert list(client.routes) == ["same"] + + +def test_api_pkg_files_keeps_every_colliding_route(): + spec = _spec_with_routes(("/a-b", None, None), ("/a_b", None, None)) + with pytest.warns(UserWarning): + files = api_pkg_files(spec, pkg_name="genpkg_d") + funcs = files["genpkg_d/funcs.py"] + assert "def get_a_b(" in funcs and "def get_a_b_2(" in funcs + + +def test_root_path_is_not_named_after_the_bare_method(): + client = _client(_spec_with_routes(("/", None, None))) + assert list(client.routes) == ["get_root"] + + +def test_a_suffix_does_not_take_the_natural_name_of_a_later_route(): + client = _client( + _spec_with_routes(("/a-b", None, None), ("/a_b", None, None), ("/a_b_2", None, None)) + ) + assert list(client.routes) == ["get_a_b", "get_a_b_3", "get_a_b_2"] + assert client.routes["get_a_b_2"].method_spec["url_template"].endswith("/a_b_2") + + +def test_root_path_and_a_root_route_are_told_apart(): + client = _client(_spec_with_routes(("/root", None, None), ("/", None, None))) + assert sorted(client.routes) == ["get_root", "get_root_2"] + + +def test_openapi_to_py_keeps_the_bare_root_name(): + from http2py.py2request import openapi_to_py + + spec = _spec_with_routes(("/", None, None), ("/root", None, None)) + assert sorted(openapi_to_py(spec)) == ["get", "get_root"]