[16.0][FIX] queue_job: rebind args/kwargs to inner cursor in in_temporary_env

[deeniiz]

Job.in_temporary_env() rebinds self.recordset to the inner cursor via the @env.setter, but leaves self.args and self.kwargs on the outer worker cursor where _prevent_commit is patched. A cr.commit() reached through any recordset argument then raises RuntimeError: Commit is forbidden in queue jobs even with allow_commit=True. Typical case: a connector exporter taking a binding and a related record, committing inside binder.bind_export(...).

Introduced by #910.

Fix: after the env swap, walk self.args and self.kwargs recursively (BaseModel, list, tuple, dict) and rebind each recordset with value.with_env(value.env(cr=new_env.cr)) — only the cursor changes, uid/su/context are kept. Originals restored in the finally, like self._env.

Regression test in test_queue_job passes the same recordset as positional arg, list element and dict value, checks cursor identity in all three, then commits. Run through RunJobController._runjob.

[deeniiz]

@eantones @guewen, can you please review it?

[OCA-git-bot]

Hi @guewen, @sbidoul,
some modules you are maintaining are being modified, check this out!

[eantones]

Rebased on current 16.0; all checks green (codecov/patch 100%).

@guewen @sbidoul gentle ping — this completes the allow_commit option from #899/#910: currently in_temporary_env() rebinds self.recordset only, so any recordset passed as a job argument keeps the outer commit-forbidden cursor, and the job still raises RuntimeError: Commit is forbidden in queue jobs even with the option enabled. Typical hit: connector-style export_record(backend, ...) jobs committing bindings so external ids are not lost — the case discussed in #889. The fix rebinds args/kwargs exactly like self.recordset (cursor-only swap, originals restored on exit), with a regression test through RunJobController._runjob.

Happy to port this to 18.0 and 19.0 right after approval.

[sbidoul]

I'll try to find time to look into this. It will not before end of next week though.

With this and #923, I can't help thinking there is something subtly wrong in the way we manage job envs and some refactoring could make things clearer and less fragile. Not sure.

[guewen]

I was looking at this this morning, and on one side it seems to me in the current state, this is the only way forward, but I have the same worry as @sbidoul , there is something that looks wrong if we have to do that.

On the details (really), I would have added args and kwargs as "lazy" methods that rebind on the fly, something along the lines of

    @property
    def args(self):
        return _rebind_env(self._args, self.env.cr) # also _rebind_env doesn't the env, only cr

Thanks for the investigation and correction

[eantones]

Hi @guewen, you got me thinking with your "lazy" idea. I tried it and I think it's actually the better solution, so I implemented it on a separate branch: nuobit#1

args/kwargs stay in _args/_kwargs and are rebound on access (only the cr is swapped, uid/su/context are kept), so in_temporary_env doesn't need to touch them anymore. Same test, coverage stays at 100%.

I can update this PR to that version if you prefer. The 18.0/19.0 ports will follow whatever shape this one ends up with.

cc @sbidoul