From 1e9cbfc1d987da2260dfa9a74eeaeb10d6d4f927 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Thu, 29 Oct 2020 15:49:24 +0100 Subject: [PATCH 1/2] Refactor user_id,model_name,record_ids fields on queue.job These fields contain the current model and record ids and user id the job works on. Storing them as separate fields does not allow to take benefits on the "JobSerialized" field, which keeps the original user, and later the 'su' flag. We could store the context there as well (or a cleaned one). Use computed fields to keep the model_name, record_ids, user_id fields for backward compatibility. The model_name should stay anyway for the searches on the UI, and the user_id for the same reason + possibility to edit the user. --- queue_job/__manifest__.py | 2 +- queue_job/fields.py | 22 +++++++-- queue_job/i18n/queue_job.pot | 4 +- queue_job/job.py | 24 +++++----- .../migrations/13.0.3.2.0/pre-migration.py | 28 +++++++++++ queue_job/models/queue_job.py | 48 +++++++++++++++---- queue_job/readme/USAGE.rst | 2 +- queue_job/tests/test_json_field.py | 28 ++++++++++- test_queue_job/tests/test_job.py | 10 ++-- 9 files changed, 132 insertions(+), 36 deletions(-) create mode 100644 queue_job/migrations/13.0.3.2.0/pre-migration.py diff --git a/queue_job/__manifest__.py b/queue_job/__manifest__.py index 879b454a1a..537db47cd0 100644 --- a/queue_job/__manifest__.py +++ b/queue_job/__manifest__.py @@ -3,7 +3,7 @@ { "name": "Job Queue", - "version": "13.0.3.1.0", + "version": "13.0.3.2.0", "author": "Camptocamp,ACSONE SA/NV,Odoo Community Association (OCA)", "website": "https://github.com/OCA/queue/queue_job", "license": "LGPL-3", diff --git a/queue_job/fields.py b/queue_job/fields.py index 4f675f9d25..b76888fb99 100644 --- a/queue_job/fields.py +++ b/queue_job/fields.py @@ -27,18 +27,29 @@ class JobSerialized(fields.Field): _slots = {"_base_type": type} - _default_json_mapping = {dict: "{}", list: "[]", tuple: "[]"} + # these are the default values when we convert an empty value + _default_json_mapping = { + dict: "{}", + list: "[]", + tuple: "[]", + models.BaseModel: lambda env: json.dumps( + {"_type": "odoo_recordset", "model": "base", "ids": [], "uid": env.uid} + ), + } def __init__(self, string=fields.Default, base_type=fields.Default, **kwargs): super().__init__(string=string, _base_type=base_type, **kwargs) def _setup_attrs(self, model, name): super()._setup_attrs(model, name) - if not self._base_type_default_json(): + if self._base_type not in self._default_json_mapping: raise ValueError("%s is not a supported base type" % (self._base_type)) - def _base_type_default_json(self): - return self._default_json_mapping.get(self._base_type) + def _base_type_default_json(self, env): + default_json = self._default_json_mapping.get(self._base_type) + if not isinstance(default_json, str): + default_json = default_json(env) + return default_json def convert_to_column(self, value, record, values=None, validate=True): return self.convert_to_cache(value, record, validate=validate) @@ -51,7 +62,7 @@ def convert_to_cache(self, value, record, validate=True): return value or None def convert_to_record(self, value, record): - default = self._base_type_default_json() + default = self._base_type_default_json(record.env) return json.loads(value or default, cls=JobDecoder, env=record.env) @@ -97,6 +108,7 @@ def object_hook(self, obj): model = self.env[obj["model"]] if obj.get("uid"): model = model.with_user(obj["uid"]) + return model.browse(obj["ids"]) elif type_ == "datetime_isoformat": return dateutil.parser.parse(obj["value"]) diff --git a/queue_job/i18n/queue_job.pot b/queue_job/i18n/queue_job.pot index 4fa978db09..eb60bd6d64 100644 --- a/queue_job/i18n/queue_job.pot +++ b/queue_job/i18n/queue_job.pot @@ -544,8 +544,8 @@ msgid "Queue jobs must created by calling 'with_delay()'." msgstr "" #. module: queue_job -#: model:ir.model.fields,field_description:queue_job.field_queue_job__record_ids -msgid "Record" +#: model:ir.model.fields,field_description:queue_job.field_queue_job__records +msgid "Record(s)" msgstr "" #. module: queue_job diff --git a/queue_job/job.py b/queue_job/job.py index 457cb1c77c..568808c002 100644 --- a/queue_job/job.py +++ b/queue_job/job.py @@ -257,15 +257,12 @@ def load(cls, env, job_uuid): @classmethod def _load_from_db_record(cls, job_db_record): stored = job_db_record - env = job_db_record.env args = stored.args kwargs = stored.kwargs method_name = stored.method_name - model = env[stored.model_name] - - recordset = model.browse(stored.record_ids) + recordset = stored.records method = getattr(recordset, method_name) eta = None @@ -299,8 +296,6 @@ def _load_from_db_record(cls, job_db_record): job_.state = stored.state job_.result = stored.result if stored.result else None job_.exc_info = stored.exc_info if stored.exc_info else None - job_.user_id = stored.user_id.id if stored.user_id else None - job_.model_name = stored.model_name if stored.model_name else None job_.retry = stored.retry job_.max_retries = stored.max_retries if stored.company_id: @@ -438,7 +433,6 @@ def __init__( recordset = func.__self__ env = recordset.env - self.model_name = recordset._name self.method_name = func.__name__ self.recordset = recordset @@ -492,7 +486,6 @@ def __init__( self.result = None self.exc_info = None - self.user_id = env.uid if "company_id" in env.context: company_id = env.context["company_id"] else: @@ -537,7 +530,6 @@ def store(self): "retry": self.retry, "max_retries": self.max_retries, "exc_info": self.exc_info, - "user_id": self.user_id or self.env.uid, "company_id": self.company_id, "result": str(self.result) if self.result else False, "date_enqueued": False, @@ -576,9 +568,8 @@ def store(self): "uuid": self.uuid, "name": self.description, "date_created": date_created, - "model_name": self.model_name, "method_name": self.method_name, - "record_ids": self.recordset.ids, + "records": self.recordset, "args": self.args, "kwargs": self.kwargs, } @@ -596,7 +587,6 @@ def db_record(self): @property def func(self): recordset = self.recordset.with_context(job_uuid=self.uuid) - recordset = recordset.with_user(self.user_id) return getattr(recordset, self.method_name) @property @@ -633,6 +623,14 @@ def uuid(self): self._uuid = str(uuid.uuid4()) return self._uuid + @property + def model_name(self): + return self.recordset._name + + @property + def user_id(self): + return self.recordset.env.uid + @property def eta(self): return self._eta @@ -909,7 +907,7 @@ class QueueJob(models.Model): def related_action_partner(self): self.ensure_one() model = self.model_name - partner = self.env[model].browse(self.record_ids) + partner = self.records # possibly get the real ID if partner_id is a binding ID action = { 'name': _("Partner"), diff --git a/queue_job/migrations/13.0.3.2.0/pre-migration.py b/queue_job/migrations/13.0.3.2.0/pre-migration.py new file mode 100644 index 0000000000..897846fa83 --- /dev/null +++ b/queue_job/migrations/13.0.3.2.0/pre-migration.py @@ -0,0 +1,28 @@ +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +import logging + +from odoo.tools.sql import column_exists + +_logger = logging.getLogger(__name__) + + +def migrate(cr, version): + if not column_exists(cr, "queue_job", "records"): + cr.execute( + """ + ALTER TABLE queue_job + ADD COLUMN records text; + """ + ) + cr.execute( + """ + UPDATE queue_job + SET records = '{"_type": "odoo_recordset"' + || ', "model": "' || model_name || '"' + || ', "uid": ' || user_id + || ', "ids": ' || record_ids + || '}' + WHERE records IS NULL; + """ + ) diff --git a/queue_job/models/queue_job.py b/queue_job/models/queue_job.py index 25d5caace5..1da0eaf86d 100644 --- a/queue_job/models/queue_job.py +++ b/queue_job/models/queue_job.py @@ -43,21 +43,34 @@ class QueueJob(models.Model): "date_created", "model_name", "method_name", - "record_ids", + "records", "args", "kwargs", ) uuid = fields.Char(string="UUID", readonly=True, index=True, required=True) - user_id = fields.Many2one(comodel_name="res.users", string="User ID", required=True) + user_id = fields.Many2one( + comodel_name="res.users", + string="User ID", + compute="_compute_user_id", + inverse="_inverse_user_id", + store=True, + ) company_id = fields.Many2one( comodel_name="res.company", string="Company", index=True ) name = fields.Char(string="Description", readonly=True) - model_name = fields.Char(string="Model", readonly=True) + model_name = fields.Char( + string="Model", compute="_compute_model_name", store=True, readonly=True + ) method_name = fields.Char(readonly=True) - record_ids = JobSerialized(readonly=True, base_type=list) + # record_ids field is only for backward compatibility (e.g. used in related + # actions), can be removed (replaced by "records") in 14.0 + record_ids = JobSerialized(compute="_compute_record_ids", base_type=list) + records = JobSerialized( + string="Record(s)", readonly=True, base_type=models.BaseModel, + ) args = JobSerialized(readonly=True, base_type=tuple) kwargs = JobSerialized(readonly=True, base_type=dict) func_string = fields.Char( @@ -113,6 +126,25 @@ def init(self): "'enqueued') AND identity_key IS NOT NULL;" ) + @api.depends("records") + def _compute_user_id(self): + for record in self: + record.user_id = record.records.env.uid + + def _inverse_user_id(self): + for record in self.with_context(_job_edit_sentinel=self.EDIT_SENTINEL): + record.records = record.records.with_user(record.user_id.id) + + @api.depends("records") + def _compute_model_name(self): + for record in self: + record.model_name = record.records._name + + @api.depends("records") + def _compute_record_ids(self): + for record in self: + record.record_ids = record.records.ids + def _inverse_channel(self): for record in self: record.override_channel = record.channel @@ -137,11 +169,10 @@ def _compute_job_function(self): record.channel_method_name = channel_method_name record.job_function_id = function - @api.depends("model_name", "method_name", "record_ids", "args", "kwargs") + @api.depends("model_name", "method_name", "records", "args", "kwargs") def _compute_func_string(self): for record in self: - record_ids = record.record_ids - model = repr(self.env[record.model_name].browse(record_ids)) + model = repr(record.records) args = [repr(arg) for arg in record.args] kwargs = ["{}={!r}".format(key, val) for key, val in record.kwargs.items()] all_args = ", ".join(args + kwargs) @@ -327,8 +358,7 @@ def related_action_open_record(self): """ self.ensure_one() - model_name = self.model_name - records = self.env[model_name].browse(self.record_ids).exists() + records = self.records.exists() if not records: return None action = { diff --git a/queue_job/readme/USAGE.rst b/queue_job/readme/USAGE.rst index 08de8fef9b..6c472eccf9 100644 --- a/queue_job/readme/USAGE.rst +++ b/queue_job/readme/USAGE.rst @@ -80,7 +80,7 @@ Example of related action code: def related_action_partner(self, name): self.ensure_one() model = self.model_name - partner = self.env[model].browse(self.record_ids) + partner = self.records action = { 'name': name, 'type': 'ir.actions.act_window', diff --git a/queue_job/tests/test_json_field.py b/queue_job/tests/test_json_field.py index 20fec32140..228efbe71b 100644 --- a/queue_job/tests/test_json_field.py +++ b/queue_job/tests/test_json_field.py @@ -15,6 +15,19 @@ class TestJson(common.TransactionCase): def test_encoder_recordset(self): + demo_user = self.env.ref("base.user_demo") + partner = self.env(user=demo_user).ref("base.main_partner") + value = partner + value_json = json.dumps(value, cls=JobEncoder) + expected = { + "uid": demo_user.id, + "_type": "odoo_recordset", + "model": "res.partner", + "ids": [partner.id], + } + self.assertEqual(json.loads(value_json), expected) + + def test_encoder_recordset_list(self): demo_user = self.env.ref("base.user_demo") partner = self.env(user=demo_user).ref("base.main_partner") value = ["a", 1, partner] @@ -32,6 +45,19 @@ def test_encoder_recordset(self): self.assertEqual(json.loads(value_json), expected) def test_decoder_recordset(self): + demo_user = self.env.ref("base.user_demo") + partner = self.env(user=demo_user).ref("base.main_partner") + value_json = ( + '{"_type": "odoo_recordset",' + '"model": "res.partner",' + '"ids": [%s],"uid": %s}' % (partner.id, demo_user.id) + ) + expected = partner + value = json.loads(value_json, cls=JobDecoder, env=self.env) + self.assertEqual(value, expected) + self.assertEqual(demo_user, expected.env.user) + + def test_decoder_recordset_list(self): demo_user = self.env.ref("base.user_demo") partner = self.env(user=demo_user).ref("base.main_partner") value_json = ( @@ -45,7 +71,7 @@ def test_decoder_recordset(self): self.assertEqual(value, expected) self.assertEqual(demo_user, expected[2].env.user) - def test_decoder_recordset_without_user(self): + def test_decoder_recordset_list_without_user(self): value_json = ( '["a", 1, {"_type": "odoo_recordset",' '"model": "res.users", "ids": [1]}]' ) diff --git a/test_queue_job/tests/test_job.py b/test_queue_job/tests/test_job.py index e55636362a..0d434df843 100644 --- a/test_queue_job/tests/test_job.py +++ b/test_queue_job/tests/test_job.py @@ -193,7 +193,6 @@ def test_read(self): eta=eta, description="My description", ) - test_job.user_id = 1 test_job.worker_pid = 99999 # normally set on "set_start" test_job.company_id = self.env.ref("base.main_company").id test_job.store() @@ -252,7 +251,6 @@ def test_unicode(self): priority=15, description=u"My dé^Wdescription", ) - test_job.user_id = 1 test_job.store() job_read = Job.load(self.env, test_job.uuid) self.assertEqual(test_job.args, job_read.args) @@ -270,7 +268,6 @@ def test_accented_bytestring(self): priority=15, description="My dé^Wdescription", ) - test_job.user_id = 1 test_job.store() job_read = Job.load(self.env, test_job.uuid) self.assertEqual(job_read.args, ("öô¿‽", "ñě")) @@ -300,7 +297,6 @@ def test_job_identity_key_str(self): description="Test I am the first one", identity_key=id_key, ) - test_job_1.user_id = 1 test_job_1.store() job1 = Job.load(self.env, test_job_1.uuid) self.assertEqual(job1.identity_key, id_key) @@ -507,6 +503,12 @@ def test_override_channel(self): test_job = delayable.testing_method(return_context=True) self.assertEqual("root.sub.sub", test_job.channel) + def test_job_change_user_id(self): + demo_user = self.env.ref("base.user_demo") + stored = self._create_job() + stored.user_id = demo_user + self.assertEqual(stored.records.env.uid, demo_user.id) + class TestJobStorageMultiCompany(common.TransactionCase): """ Test storage of jobs """ From cacfd14e6fc047e0253a768ee52d0fcf3677e619 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Fri, 13 Nov 2020 13:39:07 +0100 Subject: [PATCH 2/2] Store 'su' flag in records stored in JobSerialized fields Which means that the record the job is working on, or any record argument of the job keeps the 'su' flag. --- queue_job/fields.py | 5 ++--- queue_job/tests/test_json_field.py | 4 ++++ test_queue_job/tests/test_job.py | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/queue_job/fields.py b/queue_job/fields.py index b76888fb99..2658e01c73 100644 --- a/queue_job/fields.py +++ b/queue_job/fields.py @@ -76,6 +76,7 @@ def default(self, obj): "model": obj._name, "ids": obj.ids, "uid": obj.env.uid, + "su": obj.env.su, } elif isinstance(obj, datetime): return {"_type": "datetime_isoformat", "value": obj.isoformat()} @@ -105,9 +106,7 @@ def object_hook(self, obj): return obj type_ = obj["_type"] if type_ == "odoo_recordset": - model = self.env[obj["model"]] - if obj.get("uid"): - model = model.with_user(obj["uid"]) + model = self.env(user=obj.get("uid"), su=obj.get("su"))[obj["model"]] return model.browse(obj["ids"]) elif type_ == "datetime_isoformat": diff --git a/queue_job/tests/test_json_field.py b/queue_job/tests/test_json_field.py index 228efbe71b..3028bc0d02 100644 --- a/queue_job/tests/test_json_field.py +++ b/queue_job/tests/test_json_field.py @@ -24,6 +24,7 @@ def test_encoder_recordset(self): "_type": "odoo_recordset", "model": "res.partner", "ids": [partner.id], + "su": False, } self.assertEqual(json.loads(value_json), expected) @@ -40,6 +41,7 @@ def test_encoder_recordset_list(self): "_type": "odoo_recordset", "model": "res.partner", "ids": [partner.id], + "su": False, }, ] self.assertEqual(json.loads(value_json), expected) @@ -50,6 +52,7 @@ def test_decoder_recordset(self): value_json = ( '{"_type": "odoo_recordset",' '"model": "res.partner",' + '"su": false,' '"ids": [%s],"uid": %s}' % (partner.id, demo_user.id) ) expected = partner @@ -64,6 +67,7 @@ def test_decoder_recordset_list(self): '["a", 1, ' '{"_type": "odoo_recordset",' '"model": "res.partner",' + '"su": false,' '"ids": [%s],"uid": %s}]' % (partner.id, demo_user.id) ) expected = ["a", 1, partner] diff --git a/test_queue_job/tests/test_job.py b/test_queue_job/tests/test_job.py index 0d434df843..f9b679cc27 100644 --- a/test_queue_job/tests/test_job.py +++ b/test_queue_job/tests/test_job.py @@ -428,6 +428,26 @@ def test_job_with_mutable_arguments(self): self.assertEquals(([1],), job_instance.args) self.assertEquals({"mutable_kwarg": {"a": 1}}, job_instance.kwargs) + def test_store_env_su_no_sudo(self): + demo_user = self.env.ref("base.user_demo") + self.env = self.env(user=demo_user) + delayable = self.env["test.queue.job"].with_delay() + test_job = delayable.testing_method() + stored = test_job.db_record() + job_instance = Job.load(self.env, stored.uuid) + self.assertFalse(job_instance.recordset.env.su) + self.assertTrue(job_instance.user_id, demo_user) + + def test_store_env_su_sudo(self): + demo_user = self.env.ref("base.user_demo") + self.env = self.env(user=demo_user) + delayable = self.env["test.queue.job"].sudo().with_delay() + test_job = delayable.testing_method() + stored = test_job.db_record() + job_instance = Job.load(self.env, stored.uuid) + self.assertTrue(job_instance.recordset.env.su) + self.assertTrue(job_instance.user_id, demo_user) + class TestJobModel(JobCommonCase): def test_job_change_state(self):