Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions endpoint_route_handler/models/ir_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@ def _endpoint_routing_rules(self):
e_registry = self._endpoint_route_registry()
for endpoint_rule in e_registry.get_rules():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no warning or error if 2 rules yield the same urls and one will silently be shadowed by the other. I think we already had this issue with handlers only.
But as that part of the code is being changed, maybe a good time to fix ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed it's worth fixing, very important indeed, but it's pre-existing and not specific to generators:
for handlers the only guard is unique(endpoint_hash), which doesn't even hold across restarts
because the hash comes from Python's randomized hash().

This PR doesn't change the behavior for existing rules, and the only consumer of generators so far
(shopfloor) yields URLs under a unique per-app prefix, so it can't collide by construction.

I'll address it in a follow-up: detect conflicting rules (same URL, overlapping methods) when the
routing map is built, log an error w/ both owners and skip the later one, plus make endpoint_hash
stable so the DB constraint actually works.

Thanks for raising the point! ❤️

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't it worth a TODO or FIXME in the code, now that we are touching that part ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm opening a PR right after this gets merged. It's something I was looking at already 😉

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTR #178

_logger.debug("LOADING %s", endpoint_rule)
endpoint = endpoint_rule.endpoint
for url in endpoint_rule.routing["routes"]:
yield (url, endpoint)
if not endpoint_rule.is_generator:
yield from endpoint_rule.iter_routing_rules(self.env)
continue
# A broken generator must not break the whole routing map.
# Collect its rules first, so that nothing is yielded on failure.
try:
rules = list(endpoint_rule.iter_routing_rules(self.env))
except Exception:
_logger.exception("Routes generation failed for %s", endpoint_rule)
continue
yield from rules

@tools.ormcache("key", "cls._endpoint_route_last_version()", cache="routing")
def routing_map(cls, key=None):
Expand Down
3 changes: 3 additions & 0 deletions endpoint_route_handler/readme/DESCRIPTION.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ Technical module that provides a base handler for adding and removing
controller routes on the fly.

Can be used as a mixin or as a tool.

Rules can also provide a generator to yield many routes
computed from the code instead of a single handler (see Usage).
49 changes: 49 additions & 0 deletions endpoint_route_handler/readme/USAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,52 @@ existing one is updated, the ir.http.routing_map (which holds all Odoo
controllers) will be updated.

You can see a real life example on shopfloor.app model.

## Generated routes

A rule can provide a `generator` instead of a `handler`.
The generator is called when the routing map is built
and yields any number of `(url, endpoint)` pairs.
This way, a single row in the `endpoint_route` table
can serve many routes computed from the code
(e.g. all the methods exposed by a set of services)
and new routes do not require any update of the table.

The preferred way is to point to a model method,
which can be extended via standard inheritance:

def _prepare_endpoint_rules(self, options=None):
options = {
"generator": {
"model": self._name,
"res_id": self.id,
"method_name": "_generate_routes",
}
}
return super()._prepare_endpoint_rules(options=options)

def _generate_routes(self, rule):
for name in ("foo", "bar"):
route = f"{rule.route}/{name}"
endpoint = make_endpoint(
MyController()._handle,
{"routes": [route], "methods": ["POST"], "auth": "user"},
pargs=(self.id, name),
)
yield route, endpoint

The method is called with superuser rights
as the routing map can be built by any user.
A python class can be used too, via `klass_dotted_path`,
like for handlers.

Use `utils.make_endpoint` to build endpoints:
it fills the routing keys that Odoo requires.

A generator must be fast, deterministic and must not write to the database.
If it fails, the error is logged and its routes are skipped:
the rest of the routing map is not affected.

The routes of the rule are refreshed when the rule changes
(the routing map version is bumped)
or when the code changes (as for any controller: restart the server).
99 changes: 82 additions & 17 deletions endpoint_route_handler/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).

import functools
import importlib
import json
import logging

Expand All @@ -17,6 +16,7 @@
from odoo.addons.base.models.ir_model import query_insert

from .exceptions import EndpointHandlerNotFound
from .utils import import_method

_logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -238,6 +238,9 @@ def update_rules(self, rules, init=False):
return res

def drop_rules(self, keys):
"""Delete rules by key. Nothing to do w/o keys (`IN ()` is invalid SQL)."""
if not keys:
return True
self.cr.execute("DELETE FROM endpoint_route WHERE key IN %s", (tuple(keys),))
return True

Expand Down Expand Up @@ -314,10 +317,19 @@ def options(self):
def options(self, value):
"""Validate options.

See `_get_handler` for more info.
Options must contain either a `handler` or a `generator` key.

See `_get_handler` and `_get_generator` for more info.
"""
assert "klass_dotted_path" in value["handler"]
assert "method_name" in value["handler"]
handler = value.get("handler")
generator = value.get("generator")
assert bool(handler) != bool(generator), "Provide `handler` or `generator`"
if handler:
assert "klass_dotted_path" in handler
assert "method_name" in handler
else:
assert "method_name" in generator
assert "klass_dotted_path" in generator or "model" in generator
self.opts = value

@classmethod
Expand Down Expand Up @@ -347,9 +359,18 @@ def to_row(self):
row[k] = json.dumps(v)
return row

@property
def is_generator(self):
"""True if the rule yields its routes via a generator."""
return bool(self.opts.get("generator"))

@property
def endpoint(self):
"""Lookup http.Endpoint to be used for the routing map."""
if self.is_generator:
raise ValueError(
f"{self} is a generator rule: use `iter_routing_rules` instead"
)
handler = self._get_handler()
pargs = self.handler_options.get("default_pargs", ())
kwargs = self.handler_options.get("default_kwargs", {})
Expand All @@ -362,6 +383,28 @@ def endpoint(self):
def handler_options(self):
return self.options.handler

@property
def generator_options(self):
"""Options of the generator (see `_get_generator`)."""
return self.options.generator

def iter_routing_rules(self, env):
"""Yield `(url, endpoint)` pairs to load in the routing map.

A handler rule yields its own endpoint for each route.
A generator rule delegates to its generator,
which can yield any number of routes.
"""
if not self.is_generator:
endpoint = self.endpoint
for url in self.routing["routes"]:
yield (url, endpoint)
return
generator = self._get_generator(env)
pargs = self.generator_options.get("default_pargs", ())
kwargs = self.generator_options.get("default_kwargs", {})
yield from generator(self, *pargs, **kwargs)

def _get_handler(self):
"""Resolve endpoint handler lookup.

Expand All @@ -378,20 +421,42 @@ def _get_handler(self):

If any of them is not found, a specific exception is raised.
"""
mod_path, klass_name = self.handler_options.klass_dotted_path.rsplit(".", 1)
try:
mod = importlib.import_module(mod_path)
except ImportError as exc:
raise EndpointHandlerNotFound(f"Module `{mod_path}` not found") from exc
try:
klass = getattr(mod, klass_name)
except AttributeError as exc:
raise EndpointHandlerNotFound(f"Class `{klass_name}` not found") from exc
method_name = self.handler_options.method_name
return import_method(
self.handler_options.klass_dotted_path, self.handler_options.method_name
)

def _get_generator(self, env):
"""Resolve routes generator lookup.

`options` must contain `generator` key to provide either:

* `model`, optional `res_id` and `method_name`:
the method of the model (or of the record) is used.
This is the preferred way as the generator
can be extended via standard Odoo inheritance.
* `klass_dotted_path` and `method_name`:
same lookup as for handlers.

The generator is called with the rule as first argument
followed by `default_pargs` and `default_kwargs`.
It must return an iterable of `(url, endpoint)`.
Use `utils.make_endpoint` to build valid endpoints.
"""
opts = self.generator_options
if not opts.get("model"):
return import_method(opts.klass_dotted_path, opts.method_name)
if opts.model not in env:
raise EndpointHandlerNotFound(f"Model `{opts.model}` not found")
target = env[opts.model].sudo()
if opts.get("res_id"):
target = target.browse(opts.res_id).exists()
if not target:
raise EndpointHandlerNotFound(
f"Record `{opts.model}({opts.res_id})` not found"
)
try:
method = getattr(klass(), method_name)
return getattr(target, opts.method_name)
except AttributeError as exc:
raise EndpointHandlerNotFound(
f"Method name `{method_name}` not found"
f"Method name `{opts.method_name}` not found"
) from exc
return method
1 change: 1 addition & 0 deletions endpoint_route_handler/tests/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from . import test_registry
from . import test_endpoint
from . import test_endpoint_controller
from . import test_generator
28 changes: 28 additions & 0 deletions endpoint_route_handler/tests/fake_controllers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from odoo import http

from ..utils import make_endpoint


class CTRLFake(http.Controller):
# Shortcut for dotted path
Expand All @@ -29,3 +31,29 @@ def _do_something1(self, foo=None):
def _do_something2(self, default_arg, foo=None):
body = f"{default_arg} -> got: {foo}"
return http.request.make_response(body)


class GeneratorFake:
"""Fake routes generator."""

_path = "odoo.addons.endpoint_route_handler.tests.fake_controllers.GeneratorFake"

def generate(self, rule, *names, methods=("GET",)):
"""Yield `<rule route>/<name>` for each name, served by `handle(name)`."""
for name in names:
route = f"{rule.route}/{name}"
routing = {
"routes": [route],
"auth": "public",
"methods": list(methods),
"csrf": False,
}
yield route, make_endpoint(self.handle, routing, pargs=(name,))

def handle(self, name, **kw):
"""Respond w/ the name the route was generated for."""
return http.request.make_response(f"Generated: {name}")

def broken(self, rule, *names):
"""Fail as a buggy generator would do."""
raise ValueError("Boom")
Loading
Loading