[FIX] queue_job: prevent conflict w/ TestOverrides:test_creates#802
[FIX] queue_job: prevent conflict w/ TestOverrides:test_creates#802OCA-git-bot merged 1 commit intoOCA:18.0from
Conversation
This prevents TestOverrides.test_creates from failing in the Odoo `base` module due to sentinel protections taking effect even for local create invocations.
|
Hi @guewen, |
@simahawk I liked your idea and went ahead and implemented it. Tests are green and I will start running this change in my local development environment this week. |
|
@Kimkhoi3010 Would you be willing to review this pull request, since it mirrors your own? |
guewen
left a comment
guewen
left a comment
There was a problem hiding this comment.
TIL @api.private, nice.
Thanks!
|
/ocabot merge patch |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at 5b7cedd. Thanks a lot for contributing to OCA. ❤️ |
|
That's cool! Question: what about the |
I did consider it during implementation, but |
🤣 that makes sense 😄 My understanding is that writes always happen by calling @guewen any opinion? |
|
I wondered about that too when reviewing but then realized some fields can be modified through the API (from the UI), some mustn't, so I don't think we can do that without the sentinel? |
|
After this fix I am getting a: |
|
Hi @amh-mw, I just looked at |
|
That seems unfortunately right. My knee jerk reaction is to override |
I had the exact same reasoning |
|
After digging a bit more, it looks like the Per https://github.com/odoo/odoo/blob/18.0/addons/web/controllers/dataset.py#L32-L36 @http.route(['/web/dataset/call_kw', '/web/dataset/call_kw/<path:path>'], type='json', auth="user", readonly=_call_kw_readonly)
def call_kw(self, model, method, args, kwargs, path=None):
Model = request.env[model]
get_public_method(Model, method)
return call_kw(request.env[model], method, args, kwargs)Per https://github.com/odoo/odoo/blob/18.0/odoo/service/model.py#L29-L46 def get_public_method(model, name):
""" Get the public unbound method from a model.
When the method does not exist or is inaccessible, raise appropriate errors.
Accessible methods are public (in sense that python defined it:
not prefixed with "_") and are not decorated with `@api.private`.
"""
assert isinstance(model, BaseModel), f"{model!r} is not a BaseModel for {name}"
cls = type(model)
method = getattr(cls, name, None)
if not callable(method):
raise AttributeError(f"The method '{model._name}.{name}' does not exist") # noqa: TRY004
for mro_cls in cls.mro():
cla_method = getattr(mro_cls, name, None)
if not cla_method:
continue
if name.startswith('_') or getattr(cla_method, '_api_private', False) or name in _UNSAFE_ATTRIBUTES:
raise AccessError(f"Private methods (such as '{model._name}.{name}') cannot be called remotely.")
return method |
|
Argh, no -- I'm reading it wrong, because |
|
@amh-mw wait, actually no user should have permissions to create jobs. Queue job managers have permissions to create and delete that should be removed IMO. In older versions, the admin user could bypass the permissions so the sentinel was needed, but now that access rights are enforced even for the admin user, it seems the cleanest and simplest solution (and they should never have had these permissions anyway). Does it sound right to you? If yes, I can open a PR for this. |
6 participants
This prevents TestOverrides.test_creates from failing in the Odoo
basemodule due to the sentinel protections taking effect even for local create invocations.Fixes #727