From 44cca3dce5e4159659c3e1a2b18f6dc498fef6f4 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Fri, 30 Oct 2020 17:59:33 +0100 Subject: [PATCH 01/10] Deprecate job decorators * @job and @related_action are still supported but emit warnings and will be removed at some point (code to remove has a TODO with :job-no-decorator:) * the options previously set by the decorators are now stored on "queue.job.function" records, which can be explicitly created as XML records (and then editable!) * any method can be used with "with_delay()", if no "queue.job.function" exists, default values are used (root channel, default related action, ...) --- queue_job/__manifest__.py | 1 + queue_job/data/queue_job_function_data.xml | 5 + queue_job/job.py | 79 ++++++-- queue_job/models/base.py | 1 + queue_job/models/queue_job.py | 177 +++++++++++++++++- queue_job/views/queue_job_views.xml | 6 +- test_queue_job/__manifest__.py | 6 +- .../data/queue_job_channel_data.xml | 10 + .../data/queue_job_function_data.xml | 65 +++++++ test_queue_job/models/test_models.py | 21 +-- test_queue_job/tests/test_job_channels.py | 64 ++----- test_queue_job/tests/test_related_actions.py | 41 +--- 12 files changed, 358 insertions(+), 118 deletions(-) create mode 100644 queue_job/data/queue_job_function_data.xml create mode 100644 test_queue_job/data/queue_job_channel_data.xml create mode 100644 test_queue_job/data/queue_job_function_data.xml diff --git a/queue_job/__manifest__.py b/queue_job/__manifest__.py index bd5c989509..d901703c34 100644 --- a/queue_job/__manifest__.py +++ b/queue_job/__manifest__.py @@ -15,6 +15,7 @@ "security/ir.model.access.csv", "views/queue_job_views.xml", "data/queue_data.xml", + "data/queue_job_function_data.xml", ], "installable": True, "development_status": "Mature", diff --git a/queue_job/data/queue_job_function_data.xml b/queue_job/data/queue_job_function_data.xml new file mode 100644 index 0000000000..7168dc1629 --- /dev/null +++ b/queue_job/data/queue_job_function_data.xml @@ -0,0 +1,5 @@ + + + ._test_job]]> + + diff --git a/queue_job/job.py b/queue_job/job.py index a7b7912981..24015a0757 100644 --- a/queue_job/job.py +++ b/queue_job/job.py @@ -1,4 +1,4 @@ -# Copyright 2013-2016 Camptocamp +# Copyright 2013-2020 Camptocamp # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) import functools @@ -34,6 +34,10 @@ _logger = logging.getLogger(__name__) +def job_function_name(model, method): + return "<{}>.{}".format(model._name, method.__name__) + + class DelayableRecordset(object): """Allow to delay a method for a recordset @@ -78,12 +82,6 @@ def __getattr__(self, name): ) ) recordset_method = getattr(self.recordset, name) - if not getattr(recordset_method, "delayable", None): - raise AttributeError( - "method %s on %s is not allowed to be delayed, " - "it should be decorated with odoo.addons.queue_job.job.job" - % (name, self.recordset) - ) def delay(*args, **kwargs): return Job.enqueue( @@ -453,6 +451,10 @@ def __init__( self.job_model = self.env["queue.job"] self.job_model_name = "queue.job" + self.job_config = self.env["queue.job.function"].job_config( + job_function_name(self.recordset, func) + ) + self.state = PENDING self.retry = 0 @@ -672,7 +674,10 @@ def __repr__(self): return "" % (self.uuid, self.priority) def _get_retry_seconds(self, seconds=None): - retry_pattern = self.func.retry_pattern + retry_pattern = self.job_config.retry_pattern + if not retry_pattern: + # TODO deprecated by :job-no-decorator: + retry_pattern = getattr(self.func, "retry_pattern", None) if not seconds and retry_pattern: # ordered from higher to lower count of retries patt = sorted(retry_pattern.items(), key=lambda t: t[0]) @@ -701,12 +706,18 @@ def postpone(self, result=None, seconds=None): def related_action(self): record = self.db_record() - if hasattr(self.func, "related_action"): + if not self.job_config.related_action_enable: + return None + + funcname = self.job_config.related_action_func_name + if not funcname and hasattr(self.func, "related_action"): + # TODO deprecated by :job-no-decorator: funcname = self.func.related_action # decorator is set but empty: disable the default one if not funcname: return None - else: + + if not funcname: funcname = record._default_related_action if not isinstance(funcname, str): raise ValueError( @@ -714,7 +725,10 @@ def related_action(self): "method on queue.job as string" ) action = getattr(record, funcname) - action_kwargs = getattr(self.func, "kwargs", {}) + action_kwargs = self.job_config.related_action_kwargs + if not action_kwargs: + # TODO deprecated by :job-no-decorator: + action_kwargs = getattr(self.func, "kwargs", {}) return action(**action_kwargs) @@ -724,6 +738,7 @@ def _is_model_method(func): ) +# TODO deprecated by :job-no-decorator: def job(func=None, default_channel="root", retry_pattern=None): """Decorator for job methods. @@ -810,6 +825,25 @@ def retryable_example(): job, default_channel=default_channel, retry_pattern=retry_pattern ) + xml_fields = [ + ' ._test_job]]>\n' + ] + if default_channel: + xml_fields.append(' ') + if retry_pattern: + xml_fields.append(' {retry_pattern}') + + xml_record = ( + '\n' + "\n".join(xml_fields) + "\n" + ).format(**{"method": func.__name__, "retry_pattern": retry_pattern}) + # TODO same for related action + _logger.warning( + "@job is deprecated and no longer needed, if you need custom options, " + "use an XML record:\n%s", + xml_record, + ) + def delay_from_model(*args, **kwargs): raise AttributeError( "method.delay() can no longer be used, the general form is " @@ -832,6 +866,7 @@ def delay_from_model(*args, **kwargs): return func +# TODO deprecated by :job-no-decorator: def related_action(action=None, **kwargs): """Attach a *Related Action* to a job (decorator) @@ -894,6 +929,28 @@ def export_product(self): """ def decorate(func): + related_action_dict = { + "func_name": action, + } + if kwargs: + related_action_dict["kwargs"] = kwargs + + xml_fields = ( + ' ._test_job]]>\n' + ' {related_action}' + ) + + xml_record = ( + '\n' + xml_fields + "\n" + ).format(**{"method": func.__name__, "related_action": action}) + _logger.warning( + "@related_action is deprecated and no longer needed," + " add these options in a 'queue.job.function'" + " XML record:\n%s", + xml_record, + ) + func.related_action = action func.kwargs = kwargs return func diff --git a/queue_job/models/base.py b/queue_job/models/base.py index a7edf42a20..6ed9de9c60 100644 --- a/queue_job/models/base.py +++ b/queue_job/models/base.py @@ -21,6 +21,7 @@ class Base(models.AbstractModel): _inherit = "base" + # TODO deprecated by :job-no-decorator: def _register_hook(self): """Register marked jobs""" super(Base, self)._register_hook() diff --git a/queue_job/models/queue_job.py b/queue_job/models/queue_job.py index 1cff206a04..76a2b7779d 100644 --- a/queue_job/models/queue_job.py +++ b/queue_job/models/queue_job.py @@ -1,20 +1,22 @@ # Copyright 2013-2016 Camptocamp SA # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) +import ast import logging +from collections import namedtuple from datetime import datetime, timedelta -from odoo import _, api, exceptions, fields, models +from odoo import _, api, exceptions, fields, models, tools from odoo.osv import expression from ..fields import JobSerialized -from ..job import DONE, PENDING, STATES, Job, job +from ..job import DONE, PENDING, STATES, Job, job_function_name -_logger = logging.getLogger(__name__) +# TODO deprecated by :job-no-decorator: +channel_func_name = job_function_name -def channel_func_name(model, method): - return "<{}>.{}".format(model._name, method.__name__) +_logger = logging.getLogger(__name__) class QueueJob(models.Model): @@ -101,14 +103,18 @@ def _inverse_channel(self): @api.depends("job_function_id.channel_id") def _compute_channel(self): for record in self: - record.channel = record.override_channel or record.job_function_id.channel + channel = ( + record.override_channel or record.job_function_id.channel or "root" + ) + if record.channel != channel: + record.channel = channel @api.depends("model_name", "method_name", "job_function_id.channel_id") def _compute_job_function(self): for record in self: model = self.env[record.model_name] method = getattr(model, record.method_name) - channel_method_name = channel_func_name(model, method) + channel_method_name = job_function_name(model, method) func_model = self.env["queue.job.function"] function = func_model.search([("name", "=", channel_method_name)], limit=1) record.channel_method_name = channel_method_name @@ -305,7 +311,6 @@ def related_action_open_record(self): ) return action - @job def _test_job(self): _logger.info("Running test job.") @@ -411,9 +416,25 @@ class JobFunction(models.Model): _description = "Job Functions" _log_access = False + JobConfig = namedtuple( + "JobConfig", + "channel " + "retry_pattern " + "related_action_enable " + "related_action_func_name " + "related_action_kwargs ", + ) + def _default_channel(self): return self.env.ref("queue_job.channel_root") + # TODO if 2 modules create an entry for the same method, do what: + # * forbid? bad idea, prevent installing module + # * hack create method to merge them, does it work regarding xmlids + # and uninstallation of modules? + # * keep both records and let the user delete (or add "active" field) + # one of them, otherwise, take the first one + name = fields.Char(index=True) channel_id = fields.Many2one( comodel_name="queue.job.channel", @@ -422,7 +443,43 @@ def _default_channel(self): default=lambda r: r._default_channel(), ) channel = fields.Char(related="channel_id.complete_name", store=True, readonly=True) + retry_pattern = JobSerialized(string="Retry Pattern (serialized)", base_type=dict) + edit_retry_pattern = fields.Text( + string="Retry Pattern", + compute="_compute_edit_retry_pattern", + inverse="_inverse_edit_retry_pattern", + ) + related_action = JobSerialized(string="Related Action (serialized)", base_type=dict) + edit_related_action = fields.Text( + string="Related Action", + compute="_compute_edit_related_action", + inverse="_inverse_edit_related_action", + ) + @api.depends("retry_pattern") + def _compute_edit_retry_pattern(self): + for record in self: + retry_pattern = record._parse_retry_pattern() + record.edit_retry_pattern = str(retry_pattern) + + def _inverse_edit_retry_pattern(self): + try: + self.retry_pattern = ast.literal_eval(self.edit_retry_pattern or "{}") + except (ValueError, TypeError): + raise exceptions.UserError(self._retry_pattern_format_error_message()) + + @api.depends("related_action") + def _compute_edit_related_action(self): + for record in self: + record.edit_related_action = str(record.related_action) + + def _inverse_edit_related_action(self): + try: + self.related_action = ast.literal_eval(self.edit_related_action or "{}") + except (ValueError, TypeError): + raise exceptions.UserError(self._related_action_format_error_message()) + + # TODO deprecated by :job-no-decorator: def _find_or_create_channel(self, channel_path): channel_model = self.env["queue.job.channel"] parts = channel_path.split(".") @@ -444,8 +501,110 @@ def _find_or_create_channel(self, channel_path): ) return channel + def job_default_config(self): + return self.JobConfig( + channel="root", + retry_pattern={}, + related_action_enable=True, + related_action_func_name=None, + related_action_kwargs={}, + ) + + def _parse_retry_pattern(self): + try: + # as json can't have integers as keys and the field is stored + # as json, convert back to int + retry_pattern = { + int(try_count): postpone_seconds + for try_count, postpone_seconds in self.retry_pattern.items() + } + except ValueError: + _logger.error( + "Invalid retry pattern for job function %s," + " keys could not be parsed as integers, fallback" + " to the default retry pattern.", + self.name, + ) + retry_pattern = {} + return retry_pattern + + @tools.ormcache("name") + def job_config(self, name): + config = self.search([("name", "=", name)], limit=1) + if not config: + return self.job_default_config() + retry_pattern = config._parse_retry_pattern() + return self.JobConfig( + channel=config.channel, + retry_pattern=retry_pattern, + related_action_enable=config.related_action.get("enable", True), + related_action_func_name=config.related_action.get("func_name"), + related_action_kwargs=config.related_action.get("kwargs"), + ) + + def _retry_pattern_format_error_message(self): + return _( + "Unexpected format of Retry Pattern for {}.\n" + "Example of valid format:\n" + "{{1: 300, 5: 600, 10: 1200, 15: 3000}}" + ).format(self.name) + + @api.constrains("retry_pattern") + def _check_retry_pattern(self): + for record in self: + retry_pattern = record.retry_pattern + if not retry_pattern: + continue + + all_values = list(retry_pattern) + list(retry_pattern.values()) + for value in all_values: + try: + int(value) + except ValueError: + raise exceptions.UserError( + record._retry_pattern_format_error_message() + ) + + def _related_action_format_error_message(self): + return _( + "Unexpected format of Related Action for {}.\n" + "Example of valid format:\n" + '{{"enable": True, "func_name": "related_action_foo",' + ' "kwargs" {{"limit": 10}}}}' + ).format(self.name) + + @api.constrains("related_action") + def _check_related_action(self): + valid_keys = ("enable", "func_name", "kwargs") + for record in self: + related_action = record.related_action + if not related_action: + continue + + if any(key not in valid_keys for key in related_action): + raise exceptions.UserError( + record._related_action_format_error_message() + ) + + @api.model_create_multi + def create(self, vals_list): + records = super().create(vals_list) + self.clear_caches() + return records + + def write(self, values): + res = super().write(values) + self.clear_caches() + return res + + def unlink(self): + res = super().unlink() + self.clear_caches() + return res + + # TODO deprecated by :job-no-decorator: def _register_job(self, model, job_method): - func_name = channel_func_name(model, job_method) + func_name = job_function_name(model, job_method) if not self.search_count([("name", "=", func_name)]): channel = self._find_or_create_channel(job_method.default_channel) self.create({"name": func_name, "channel_id": channel.id}) diff --git a/queue_job/views/queue_job_views.xml b/queue_job/views/queue_job_views.xml index 8a48ae950c..791e560a84 100644 --- a/queue_job/views/queue_job_views.xml +++ b/queue_job/views/queue_job_views.xml @@ -289,10 +289,12 @@ queue.job.function.form queue.job.function -
+ + +
@@ -301,7 +303,7 @@ queue.job.function.tree queue.job.function - + diff --git a/test_queue_job/__manifest__.py b/test_queue_job/__manifest__.py index dfeb22ccc0..fbdc6128d9 100644 --- a/test_queue_job/__manifest__.py +++ b/test_queue_job/__manifest__.py @@ -9,6 +9,10 @@ "category": "Generic Modules", "depends": ["queue_job"], "website": "https://github.com/OCA/queue", - "data": ["security/ir.model.access.csv"], + "data": [ + "data/queue_job_channel_data.xml", + "data/queue_job_function_data.xml", + "security/ir.model.access.csv", + ], "installable": True, } diff --git a/test_queue_job/data/queue_job_channel_data.xml b/test_queue_job/data/queue_job_channel_data.xml new file mode 100644 index 0000000000..2b442117eb --- /dev/null +++ b/test_queue_job/data/queue_job_channel_data.xml @@ -0,0 +1,10 @@ + + + sub + + + + subsub + + + diff --git a/test_queue_job/data/queue_job_function_data.xml b/test_queue_job/data/queue_job_function_data.xml new file mode 100644 index 0000000000..a75a862adb --- /dev/null +++ b/test_queue_job/data/queue_job_function_data.xml @@ -0,0 +1,65 @@ + + + .testing_method]]> + + + + .job_with_retry_pattern]]> + + + + .job_with_retry_pattern__no_zero]]> + + + + .job_sub_channel]]> + + + + .testing_related_action__return_none]]> + + + + .testing_related_action__kwargs]]> + + + + .testing_related_action__store]]> + + + + .job_a]]> + + diff --git a/test_queue_job/models/test_models.py b/test_queue_job/models/test_models.py index 0478318caa..36fdb1c6f9 100644 --- a/test_queue_job/models/test_models.py +++ b/test_queue_job/models/test_models.py @@ -4,7 +4,6 @@ from odoo import fields, models from odoo.addons.queue_job.exception import RetryableJobError -from odoo.addons.queue_job.job import job, related_action class QueueJob(models.Model): @@ -34,8 +33,6 @@ class TestQueueJob(models.Model): name = fields.Char() - @job - @related_action(action="testing_related_method") def testing_method(self, *args, **kwargs): """ Method used for tests @@ -47,23 +44,18 @@ def testing_method(self, *args, **kwargs): return self.env.context return args, kwargs - @job def no_description(self): return - @job(retry_pattern={1: 60, 2: 180, 3: 10, 5: 300}) def job_with_retry_pattern(self): return - @job(retry_pattern={3: 180}) def job_with_retry_pattern__no_zero(self): return - @job def mapped(self, func): return super(TestQueueJob, self).mapped(func) - @job def job_alter_mutable(self, mutable_arg, mutable_kwarg=None): mutable_arg.append(2) mutable_kwarg["b"] = 2 @@ -75,18 +67,16 @@ class TestQueueChannel(models.Model): _name = "test.queue.channel" _description = "Test model for queue.channel" - @job def job_a(self): return - @job def job_b(self): return - @job(default_channel="root.sub.subsub") def job_sub_channel(self): return + # TODO deprecated by :job-no-decorator: @property def dummy_property(self): """ Return foo @@ -103,23 +93,14 @@ class TestRelatedAction(models.Model): _name = "test.related.action" _description = "Test model for related actions" - @job def testing_related_action__no(self): return - @job - @related_action() # default action returns None def testing_related_action__return_none(self): return - @job - @related_action(action="testing_related_method", b=4) def testing_related_action__kwargs(self): return - @job - @related_action( - action="testing_related__url", url="https://en.wikipedia.org/wiki/{subject}" - ) def testing_related_action__store(self): return diff --git a/test_queue_job/tests/test_job_channels.py b/test_queue_job/tests/test_job_channels.py index 3dbd2d36ad..f5b8c7e7ae 100644 --- a/test_queue_job/tests/test_job_channels.py +++ b/test_queue_job/tests/test_job_channels.py @@ -4,7 +4,7 @@ import odoo.tests.common as common from odoo import exceptions -from odoo.addons.queue_job.job import Job, job +from odoo.addons.queue_job.job import Job, job, job_function_name class TestJobChannels(common.TransactionCase): @@ -35,71 +35,46 @@ def test_channel_root(self): with self.assertRaises(exceptions.Warning): self.root_channel.name = "leaf" - def test_register_jobs(self): - self.env["queue.job.function"].search([]).unlink() - self.env["queue.job.channel"].search([("name", "!=", "root")]).unlink() - - method_a = self.env["test.queue.channel"].job_a - self.env["queue.job.function"]._register_job( - self.env["test.queue.channel"], method_a - ) - method_b = self.env["test.queue.channel"].job_b - self.env["queue.job.function"]._register_job( - self.env["test.queue.channel"], method_b - ) - - path_a = ".job_a" - path_b = ".job_b" - self.assertTrue(self.function_model.search([("name", "=", path_a)])) - self.assertTrue(self.function_model.search([("name", "=", path_b)])) - def test_channel_on_job(self): - self.env["queue.job.function"].search([]).unlink() - self.env["queue.job.channel"].search([("name", "!=", "root")]).unlink() - method = self.env["test.queue.channel"].job_a - self.env["queue.job.function"]._register_job( - self.env["test.queue.channel"], method - ) - path_a = "<{}>.{}".format(method.__self__.__class__._name, method.__name__) + path_a = job_function_name(self.env["test.queue.channel"], method) job_func = self.function_model.search([("name", "=", path_a)]) + self.assertEquals(job_func.channel, "root") test_job = Job(method) test_job.store() - stored = self.env["queue.job"].search([("uuid", "=", test_job.uuid)]) + stored = test_job.db_record() self.assertEquals(stored.channel, "root") job_read = Job.load(self.env, test_job.uuid) self.assertEquals(job_read.channel, "root") - channel = self.channel_model.create( - {"name": "sub", "parent_id": self.root_channel.id} - ) - job_func.channel_id = channel + sub_channel = self.env.ref("test_queue_job.channel_sub") + job_func.channel_id = sub_channel test_job = Job(method) test_job.store() - stored = self.env["queue.job"].search([("uuid", "=", test_job.uuid)]) + stored = test_job.db_record() self.assertEquals(stored.channel, "root.sub") # it's also possible to override the channel - test_job = Job(method, channel="root.sub.sub.sub") + test_job = Job(method, channel="root.sub") test_job.store() - stored = self.env["queue.job"].search([("uuid", "=", test_job.uuid)]) + stored = test_job.db_record() self.assertEquals(stored.channel, test_job.channel) - def test_default_channel(self): - self.env["queue.job.function"].search([]).unlink() - self.env["queue.job.channel"].search([("name", "!=", "root")]).unlink() + def test_default_channel_no_xml(self): + """Channel on job is root if there is no queue.job.function record""" + test_job = Job(self.env["res.users"].browse) + test_job.store() + stored = test_job.db_record() + self.assertEquals(stored.channel, "root") + def test_set_channel_from_record(self): method = self.env["test.queue.channel"].job_sub_channel - self.env["queue.job.function"]._register_job( - self.env["test.queue.channel"], method - ) - self.assertEquals(method.default_channel, "root.sub.subsub") - - path_a = "<{}>.{}".format(method.__self__.__class__._name, method.__name__) - job_func = self.function_model.search([("name", "=", path_a)]) + func_name = job_function_name(self.env["test.queue.channel"], method) + job_func = self.function_model.search([("name", "=", func_name)]) + self.assertEqual(job_func.channel, "root.sub.subsub") channel = job_func.channel_id self.assertEquals(channel.name, "subsub") @@ -107,6 +82,7 @@ def test_default_channel(self): self.assertEquals(channel.parent_id.parent_id.name, "root") self.assertEquals(job_func.channel, "root.sub.subsub") + # TODO deprecated by :job-no-decorator: def test_job_decorator(self): """ Test the job decorator """ default_channel = "channel" diff --git a/test_queue_job/tests/test_related_actions.py b/test_queue_job/tests/test_related_actions.py index 31fa771519..b6433048e6 100644 --- a/test_queue_job/tests/test_related_actions.py +++ b/test_queue_job/tests/test_related_actions.py @@ -23,15 +23,9 @@ def test_attributes(self): self.assertEqual(act_kwargs, {"b": 4}) def test_decorator_empty(self): - """ Job with decorator without value disable the default action - - The function is:: - - @job - @related_action() # default action returns None - def testing_related_action__return_none(self): - return + """Job with decorator without value disable the default action + The ``related_action`` configuration is: ``{"enable": False}`` """ # default action returns None job_ = self.record.with_delay().testing_related_action__return_none() @@ -50,12 +44,7 @@ def test_default_no_record(self): When called on no record. - The function is:: - - @job - def testing_related_action__no(self): - return - + The ``related_action`` configuration is: ``{}`` """ job_ = self.model.with_delay().testing_related_action__no() expected = None @@ -75,12 +64,7 @@ def test_default_one_record(self): When called on one record. - The function is:: - - @job - def testing_related_action__no(self): - return - + The ``related_action`` configuration is: ``{}`` """ job_ = self.record.with_delay().testing_related_action__no() expected = { @@ -97,12 +81,7 @@ def test_default_several_record(self): When called on several record. - The function is:: - - @job - def testing_related_action__no(self): - return - + The ``related_action`` configuration is: ``{}`` """ job_ = self.records.with_delay().testing_related_action__no() expected = { @@ -119,12 +98,12 @@ def test_decorator(self): The function is:: - @job - @related_action(action='testing_related__url', - url='https://en.wikipedia.org/wiki/{subject}') - def testing_related_action__store(self): - return + The ``related_action`` configuration is:: + { + "func_name": "testing_related__url", + "kwargs": {"url": "https://en.wikipedia.org/wiki/{subject}"} + } """ job_ = self.record.with_delay().testing_related_action__store("Discworld") expected = { From 27208af45919c751f52173f8a4a013a69b7948f5 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 10:40:56 +0100 Subject: [PATCH 02/10] Rebind job functions/channels with same name during install As previously, the job functions and channels were created automatically by "_register_job" on methods decorated by `@job`, databases have records without xmlids. To prevent duplicates (or unique constraint error) in case of channels, the create method automatically merges the new record with the existing record. This behavior happens only when installing/upgrading a module, records created manually behave normally. If 2 modules adds the same channel or job function, they'll be merged together with 2 xmlids, on uninstallation of one of the module, the record will be kept thanks to the second xmlid. --- queue_job/models/queue_job.py | 51 +++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/queue_job/models/queue_job.py b/queue_job/models/queue_job.py index 76a2b7779d..2891f535b5 100644 --- a/queue_job/models/queue_job.py +++ b/queue_job/models/queue_job.py @@ -388,6 +388,31 @@ def parent_required(self): if record.name != "root" and not record.parent_id: raise exceptions.ValidationError(_("Parent channel required.")) + @api.model_create_multi + def create(self, vals_list): + records = self.browse() + if self.env.context.get("install_mode"): + # installing a module that creates a channel: rebinds the channel + # to an existing one (likely we already had the channel created by + # the @job decorator previously) + new_vals_list = [] + for vals in vals_list: + name = vals.get("name") + parent_id = vals.get("parent_id") + if name and parent_id: + existing = self.search( + [("name", "=", name), ("parent_id", "=", parent_id)] + ) + if existing: + if not existing.get_metadata()[0].get("noupdate"): + existing.write(vals) + records |= existing + continue + new_vals_list.append(vals) + vals_list = new_vals_list + records |= super().create(vals_list) + return records + def write(self, values): for channel in self: if ( @@ -428,13 +453,6 @@ class JobFunction(models.Model): def _default_channel(self): return self.env.ref("queue_job.channel_root") - # TODO if 2 modules create an entry for the same method, do what: - # * forbid? bad idea, prevent installing module - # * hack create method to merge them, does it work regarding xmlids - # and uninstallation of modules? - # * keep both records and let the user delete (or add "active" field) - # one of them, otherwise, take the first one - name = fields.Char(index=True) channel_id = fields.Many2one( comodel_name="queue.job.channel", @@ -588,7 +606,24 @@ def _check_related_action(self): @api.model_create_multi def create(self, vals_list): - records = super().create(vals_list) + records = self.browse() + if self.env.context.get("install_mode"): + # installing a module that creates a job function: rebinds the record + # to an existing one (likely we already had the job function created by + # the @job decorator previously) + new_vals_list = [] + for vals in vals_list: + name = vals.get("name") + if name: + existing = self.search([("name", "=", name)], limit=1) + if existing: + if not existing.get_metadata()[0].get("noupdate"): + existing.write(vals) + records |= existing + continue + new_vals_list.append(vals) + vals_list = new_vals_list + records |= super().create(vals_list) self.clear_caches() return records From 59e3eafd3cc27f289d645cd909ffb108448d163e Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 13:22:08 +0100 Subject: [PATCH 03/10] Update documentation for deprecation of decorators --- queue_job/job.py | 9 ++- queue_job/models/base.py | 19 ++++-- queue_job/models/queue_job.py | 11 ++- queue_job/readme/DESCRIPTION.rst | 6 +- queue_job/readme/HISTORY.rst | 2 + queue_job/readme/USAGE.rst | 112 +++++++++++++++++++++++++++++++ 6 files changed, 146 insertions(+), 13 deletions(-) diff --git a/queue_job/job.py b/queue_job/job.py index 24015a0757..69fed0b1b3 100644 --- a/queue_job/job.py +++ b/queue_job/job.py @@ -46,9 +46,6 @@ class DelayableRecordset(object): delayable = DelayableRecordset(recordset, priority=20) delayable.method(args, kwargs) - ``method`` must be a method of the recordset's Model, decorated with - :func:`~odoo.addons.queue_job.job.job`. - The method call will be processed asynchronously in the job queue, with the passed arguments. @@ -742,6 +739,9 @@ def _is_model_method(func): def job(func=None, default_channel="root", retry_pattern=None): """Decorator for job methods. + Deprecated. Use ``queue.job.function`` XML records (details in + ``readme/USAGE.rst``). + It enables the possibility to use a Model's method as a job function. Optional argument: @@ -870,6 +870,9 @@ def delay_from_model(*args, **kwargs): def related_action(action=None, **kwargs): """Attach a *Related Action* to a job (decorator) + Deprecated. Use ``queue.job.function`` XML records (details in + ``readme/USAGE.rst``). + A *Related Action* will appear as a button on the Odoo view. The button will execute the action, usually it will open the form view of the record related to the job. diff --git a/queue_job/models/base.py b/queue_job/models/base.py index 6ed9de9c60..3bb4d78361 100644 --- a/queue_job/models/base.py +++ b/queue_job/models/base.py @@ -46,15 +46,22 @@ def with_delay( ): """ Return a ``DelayableRecordset`` - The returned instance allow to enqueue any method of the recordset's - Model which is decorated by :func:`~odoo.addons.queue_job.job.job`. + The returned instance allows to enqueue any method of the recordset's + Model. Usage:: self.env['res.users'].with_delay().write({'name': 'test'}) - In the line above, in so far ``write`` is allowed to be delayed with - ``@job``, the write will be executed in an asynchronous job. + ``with_delay()`` accepts job properties which specify how the job will + be executed. + + Usage with job properties:: + + delayable = env['a.model'].with_delay(priority=30, eta=60*60*5) + delayable.export_one_thing(the_thing_to_export) + # => the job will be executed with a low priority and not before a + # delay of 5 hours from now :param priority: Priority of the job, 0 being the higher priority. Default is 10. @@ -70,7 +77,9 @@ def with_delay( defined on the function :param identity_key: key uniquely identifying the job, if specified and a job with the same key has not yet been run, - the new job will not be added. + the new job will not be added. It is either a + string, either a function that takes the job as + argument (see :py:func:`..job.identity_exact`). :return: instance of a DelayableRecordset :rtype: :class:`odoo.addons.queue_job.job.DelayableRecordset` diff --git a/queue_job/models/queue_job.py b/queue_job/models/queue_job.py index 2891f535b5..a68fb1bb15 100644 --- a/queue_job/models/queue_job.py +++ b/queue_job/models/queue_job.py @@ -12,7 +12,7 @@ from ..fields import JobSerialized from ..job import DONE, PENDING, STATES, Job, job_function_name -# TODO deprecated by :job-no-decorator: +# TODO alias for backward compatibility deprecated by :job-no-decorator: channel_func_name = job_function_name @@ -466,12 +466,21 @@ def _default_channel(self): string="Retry Pattern", compute="_compute_edit_retry_pattern", inverse="_inverse_edit_retry_pattern", + help="Pattern expressing from the count of retries on retryable errors," + " the number of of seconds to postpone the next execution.\n" + "Example: {1: 10, 5: 20, 10: 30, 15: 300}.\n" + "See the module description for details.", ) related_action = JobSerialized(string="Related Action (serialized)", base_type=dict) edit_related_action = fields.Text( string="Related Action", compute="_compute_edit_related_action", inverse="_inverse_edit_related_action", + help="The action when the button *Related Action* is used on a job. " + "The default action is to open the view of the record related " + "to the job. Configured as a dictionary with optional keys: " + "enable, func_name, kwargs.\n" + "See the module description for details.", ) @api.depends("retry_pattern") diff --git a/queue_job/readme/DESCRIPTION.rst b/queue_job/readme/DESCRIPTION.rst index e196c8fc66..263f86385d 100644 --- a/queue_job/readme/DESCRIPTION.rst +++ b/queue_job/readme/DESCRIPTION.rst @@ -9,12 +9,10 @@ Example: .. code-block:: python from odoo import models, fields, api - from odoo.addons.queue_job.job import job class MyModel(models.Model): _name = 'my.model' - @job def my_method(self, a, k=None): _logger.info('executed with a: %s and k: %s', a, k) @@ -26,8 +24,8 @@ Example: self.env['my.model'].with_delay().my_method('a', k=2) -In the snippet of code above, when we call ``button_do_stuff``, a job capturing -the method and arguments will be postponed. It will be executed as soon as the +In the snippet of code above, when we call ``button_do_stuff``, a job **capturing +the method and arguments** will be postponed. It will be executed as soon as the Jobrunner has a free bucket, which can be instantaneous if no other job is running. diff --git a/queue_job/readme/HISTORY.rst b/queue_job/readme/HISTORY.rst index 43fbbd3b23..072e87fd31 100644 --- a/queue_job/readme/HISTORY.rst +++ b/queue_job/readme/HISTORY.rst @@ -13,6 +13,8 @@ Next * [ADD] Run jobrunner as a worker process instead of a thread in the main process (when running with --workers > 0) +* [REF] ``@job`` and ``@related_action`` deprecated, any method can be delayed, + and configured using ``queue.job.function`` records 13.0.1.2.0 (2020-03-10) diff --git a/queue_job/readme/USAGE.rst b/queue_job/readme/USAGE.rst index 5335862e94..ee3988549f 100644 --- a/queue_job/readme/USAGE.rst +++ b/queue_job/readme/USAGE.rst @@ -5,6 +5,118 @@ To use this module, you need to: Developers ~~~~~~~~~~ +**Configure default options for jobs** + +In earlier versions, jobs could be configured using the ``@job`` decorator. +This is now obsolete, they can be configured using optional ``queue.job.function`` +and ``queue.job.channel`` XML records. + +Example of channel: + +.. code-block:: XML + + + sale + + + +Example of job function: + +.. code-block:: XML + + + .action_done]]> + + + + + +The general form for the ``name`` is: ``.method``. + +The channel, related action and retry pattern options are optional, they are +documented below. + +When writing modules, if 2+ modules add a job function or channel with the same +name (and parent for channels), they'll be merged in the same record, even if +they have different xmlids. On uninstall, the merged record is deleted when all +the modules using it are uninstalled. + + +**Job function: channel** + +The channel where the job will be delayed. The default channel is ``root``. + +**Job function: related action** + +The *Related Action* appears as a button on the Job's view. +The button will execute the defined action. + +The default one is to open the view of the record related to the job (form view +when there is a single record, list view for several records). +In many cases, the default related action is enough and doesn't need +customization, but it can be customized by providing a dictionary on the job +function: + +.. code-block:: python + + { + "enable": False, + "func_name": "related_action_partner", + "kwargs": {"name": "Partner"}, + } + +* ``enable``: when ``False``, the button has no effect (default: ``True``) +* ``func_name``: name of the method on ``queue.job`` that returns an action +* ``kwargs``: extra arguments to pass to the related action method + +Example of related action code: + +.. code-block:: python + + class QueueJob(models.Model): + _inherit = 'queue.job' + + def related_action_partner(self, name): + self.ensure_one() + model = self.model_name + partner = self.env[model].browse(self.record_ids) + action = { + 'name': name, + 'type': 'ir.actions.act_window', + 'res_model': model, + 'view_type': 'form', + 'view_mode': 'form', + 'res_id': partner.id, + } + return action + + +**Job function: retry pattern** + +When a job fails with a retryable error type, it is automatically +retried later. By default, the retry is always 10 minutes later. + +A retry pattern can be configured on the job function. What a pattern represents +is "from X tries, postpone to Y seconds". It is expressed as a dictionary where +keys are tries and values are seconds to postpone as integers: + + +.. code-block:: python + + { + 1: 10, + 5: 20, + 10: 30, + 15: 300, + } + +Based on this configuration, we can tell that: + +* 5 first retries are postponed 10 seconds later +* retries 5 to 10 postponed 20 seconds later +* retries 10 to 15 postponed 30 seconds later +* all subsequent retries postponed 5 minutes later + **Bypass jobs on running Odoo** When you are developing (ie: connector modules) you might want From 97bfbf4b3c556f20bbcb66353b0e7839e5cf4f5e Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 14:53:59 +0100 Subject: [PATCH 04/10] Add safety to prevent crafting queue.job records As we can delay a job on any method, and the queue.job model is accessible from RPC (as any model), prevent to: * create a queue.job using RPC * write on protected fields (e.g. method name) using RPC Admittedly, the risk is low since users need have Queue Job Manager access to create/write on jobs, but it would allow these users to call internal methods. The check is done using a context key that must be equal to a sentinel object, which is impossible to pass through RPC. --- queue_job/job.py | 9 +++- queue_job/models/queue_job.py | 49 ++++++++++++++++--- queue_job/tests/__init__.py | 1 + .../tests/test_queue_job_protected_write.py | 25 ++++++++++ 4 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 queue_job/tests/test_queue_job_protected_write.py diff --git a/queue_job/job.py b/queue_job/job.py index 69fed0b1b3..afe97c2931 100644 --- a/queue_job/job.py +++ b/queue_job/job.py @@ -553,9 +553,14 @@ def store(self): if self.identity_key: vals["identity_key"] = self.identity_key + job_model = self.env["queue.job"] + # The sentinel is used to prevent edition sensitive fields (such as + # method_name) from RPC methods. + edit_sentinel = job_model.EDIT_SENTINEL + db_record = self.db_record() if db_record: - db_record.write(vals) + db_record.with_context(_job_edit_sentinel=edit_sentinel).write(vals) else: date_created = self.date_created # The following values must never be modified after the @@ -577,7 +582,7 @@ def store(self): if self.channel: vals.update({"channel": self.channel}) - self.env[self.job_model_name].sudo().create(vals) + job_model.with_context(_job_edit_sentinel=edit_sentinel).sudo().create(vals) def db_record(self): return self.db_record_from_uuid(self.env, self.uuid) diff --git a/queue_job/models/queue_job.py b/queue_job/models/queue_job.py index a68fb1bb15..4a8fad5598 100644 --- a/queue_job/models/queue_job.py +++ b/queue_job/models/queue_job.py @@ -32,6 +32,22 @@ class QueueJob(models.Model): _removal_interval = 30 # days _default_related_action = "related_action_open_record" + # This must be passed in a context key "_job_edit_sentinel" to write on + # protected fields. It protects against crafting "queue.job" records from + # RPC (e.g. on internal methods). When ``with_delay`` is used, the sentinel + # is set. + EDIT_SENTINEL = object() + _protected_fields = ( + "uuid", + "name", + "date_created", + "model_name", + "method_name", + "record_ids", + "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) company_id = fields.Many2one( @@ -130,6 +146,33 @@ def _compute_func_string(self): all_args = ", ".join(args + kwargs) record.func_string = "{}.{}({})".format(model, record.method_name, all_args) + @api.model_create_multi + def create(self, vals_list): + if self.env.context.get("_job_edit_sentinel") is not self.EDIT_SENTINEL: + # Prevent to create a queue.job record "raw" from RPC. + # ``with_delay()`` must be used. + raise exceptions.AccessError( + _("Queue jobs must created by calling 'with_delay()'.") + ) + return super().create(vals_list) + + def write(self, vals): + if self.env.context.get("_job_edit_sentinel") is not self.EDIT_SENTINEL: + write_on_protected_fields = [ + fieldname for fieldname in vals if fieldname in self._protected_fields + ] + if write_on_protected_fields: + raise exceptions.AccessError( + _("Not allowed to change field(s): {}").format( + write_on_protected_fields + ) + ) + + if vals.get("state") == "failed": + self._message_post_on_failure() + + return super().write(vals) + def open_related_action(self): """Open the related action associated to the job""" self.ensure_one() @@ -175,12 +218,6 @@ def _message_post_on_failure(self): if msg: record.message_post(body=msg, subtype="queue_job.mt_job_failed") - def write(self, vals): - res = super(QueueJob, self).write(vals) - if vals.get("state") == "failed": - self._message_post_on_failure() - return res - def _subscribe_users_domain(self): """Subscribe all users having the 'Queue Job Manager' group""" group = self.env.ref("queue_job.group_queue_job_manager") diff --git a/queue_job/tests/__init__.py b/queue_job/tests/__init__.py index 75f3a5536c..5eb2dcb6f0 100644 --- a/queue_job/tests/__init__.py +++ b/queue_job/tests/__init__.py @@ -2,3 +2,4 @@ from . import test_runner_runner from . import test_json_field from . import test_model_job_channel +from . import test_queue_job_protected_write diff --git a/queue_job/tests/test_queue_job_protected_write.py b/queue_job/tests/test_queue_job_protected_write.py new file mode 100644 index 0000000000..cf8380bcec --- /dev/null +++ b/queue_job/tests/test_queue_job_protected_write.py @@ -0,0 +1,25 @@ +# copyright 2020 Camptocamp +# license lgpl-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from odoo import exceptions +from odoo.tests import common + + +class TestJobWriteProtected(common.SavepointCase): + def test_create_error(self): + with self.assertRaises(exceptions.AccessError): + self.env["queue.job"].create( + {"uuid": "test", "model_name": "res.partner", "method_name": "write"} + ) + + def test_write_protected_field_error(self): + job_ = self.env["res.partner"].with_delay().create({"name": "test"}) + db_job = job_.db_record() + with self.assertRaises(exceptions.AccessError): + db_job.method_name = "unlink" + + def test_write_allow_no_protected_field_error(self): + job_ = self.env["res.partner"].with_delay().create({"name": "test"}) + db_job = job_.db_record() + db_job.priority = 30 + self.assertEqual(db_job.priority, 30) From 27687b2ed9009edfd7f4baeb4dce0ebcdf025901 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 21:07:20 +0100 Subject: [PATCH 05/10] Replace job function name by model + method fields A many2one to select the model + a char for the method name is more convenient and error-proof than having to type the expected format correctly. --- queue_job/__manifest__.py | 2 +- queue_job/data/queue_job_function_data.xml | 3 +- queue_job/job.py | 15 +++-- .../migrations/13.0.2.0.0/post-migration.py | 23 ++++++++ queue_job/models/queue_job.py | 54 ++++++++++++++---- queue_job/readme/USAGE.rst | 3 +- queue_job/tests/__init__.py | 1 + queue_job/tests/test_model_job_function.py | 56 +++++++++++++++++++ queue_job/views/queue_job_views.xml | 2 + .../data/queue_job_function_data.xml | 36 ++++++------ test_queue_job/tests/test_job_channels.py | 11 ++-- 11 files changed, 163 insertions(+), 43 deletions(-) create mode 100644 queue_job/migrations/13.0.2.0.0/post-migration.py create mode 100644 queue_job/tests/test_model_job_function.py diff --git a/queue_job/__manifest__.py b/queue_job/__manifest__.py index d901703c34..05d87c8b03 100644 --- a/queue_job/__manifest__.py +++ b/queue_job/__manifest__.py @@ -3,7 +3,7 @@ { "name": "Job Queue", - "version": "13.0.1.7.0", + "version": "13.0.2.0.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/data/queue_job_function_data.xml b/queue_job/data/queue_job_function_data.xml index 7168dc1629..0105dbc508 100644 --- a/queue_job/data/queue_job_function_data.xml +++ b/queue_job/data/queue_job_function_data.xml @@ -1,5 +1,6 @@ - ._test_job]]> + + _test_job diff --git a/queue_job/job.py b/queue_job/job.py index afe97c2931..718a5b2098 100644 --- a/queue_job/job.py +++ b/queue_job/job.py @@ -34,10 +34,6 @@ _logger = logging.getLogger(__name__) -def job_function_name(model, method): - return "<{}>.{}".format(model._name, method.__name__) - - class DelayableRecordset(object): """Allow to delay a method for a recordset @@ -449,7 +445,9 @@ def __init__( self.job_model_name = "queue.job" self.job_config = self.env["queue.job.function"].job_config( - job_function_name(self.recordset, func) + self.env["queue.job.function"].job_function_name( + self.model_name, self.method_name + ) ) self.state = PENDING @@ -831,7 +829,8 @@ def retryable_example(): ) xml_fields = [ - ' ._test_job]]>\n' + ' \n' + ' _test_job\n' ] if default_channel: xml_fields.append(' ') @@ -842,7 +841,6 @@ def retryable_example(): '\n' + "\n".join(xml_fields) + "\n" ).format(**{"method": func.__name__, "retry_pattern": retry_pattern}) - # TODO same for related action _logger.warning( "@job is deprecated and no longer needed, if you need custom options, " "use an XML record:\n%s", @@ -944,7 +942,8 @@ def decorate(func): related_action_dict["kwargs"] = kwargs xml_fields = ( - ' ._test_job]]>\n' + ' \n' + ' _test_job\n' ' {related_action}' ) diff --git a/queue_job/migrations/13.0.2.0.0/post-migration.py b/queue_job/migrations/13.0.2.0.0/post-migration.py new file mode 100644 index 0000000000..a399e53e36 --- /dev/null +++ b/queue_job/migrations/13.0.2.0.0/post-migration.py @@ -0,0 +1,23 @@ +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +import logging + +from odoo import SUPERUSER_ID, api, exceptions + +_logger = logging.getLogger(__name__) + + +def migrate(cr, version): + with api.Environment.manage(): + env = api.Environment(cr, SUPERUSER_ID, {}) + for job_func in env["queue.job.function"].search([]): + try: + # trigger inverse field to set model_id and method + job_func.name = job_func.name + except exceptions.UserError: + # ignore invalid entries not to block migration + _logger.error( + "could not migrate job function '%s' (id: %s), invalid name", + job_func.name, + job_func.id, + ) diff --git a/queue_job/models/queue_job.py b/queue_job/models/queue_job.py index 4a8fad5598..ed60e49d63 100644 --- a/queue_job/models/queue_job.py +++ b/queue_job/models/queue_job.py @@ -3,6 +3,7 @@ import ast import logging +import re from collections import namedtuple from datetime import datetime, timedelta @@ -10,13 +11,12 @@ from odoo.osv import expression from ..fields import JobSerialized -from ..job import DONE, PENDING, STATES, Job, job_function_name +from ..job import DONE, PENDING, STATES, Job -# TODO alias for backward compatibility deprecated by :job-no-decorator: -channel_func_name = job_function_name +_logger = logging.getLogger(__name__) -_logger = logging.getLogger(__name__) +regex_job_function_name = re.compile(r"^<([0-9a-z_\.]+)>\.([0-9a-zA-Z_]+)$") class QueueJob(models.Model): @@ -128,10 +128,10 @@ def _compute_channel(self): @api.depends("model_name", "method_name", "job_function_id.channel_id") def _compute_job_function(self): for record in self: - model = self.env[record.model_name] - method = getattr(model, record.method_name) - channel_method_name = job_function_name(model, method) func_model = self.env["queue.job.function"] + channel_method_name = func_model.job_function_name( + record.model_name, record.method_name + ) function = func_model.search([("name", "=", channel_method_name)], limit=1) record.channel_method_name = channel_method_name record.job_function_id = function @@ -310,7 +310,7 @@ def _get_stuck_jobs_domain(self, queue_dl, started_dl): def _get_stuck_jobs_to_requeue(self, enqueued_delta, started_delta): job_model = self.env["queue.job"] stuck_jobs = job_model.search( - self._get_stuck_jobs_domain(enqueued_delta, started_delta,) + self._get_stuck_jobs_domain(enqueued_delta, started_delta) ) return stuck_jobs @@ -490,7 +490,17 @@ class JobFunction(models.Model): def _default_channel(self): return self.env.ref("queue_job.channel_root") - name = fields.Char(index=True) + name = fields.Char( + compute="_compute_name", inverse="_inverse_name", index=True, store=True, + ) + + # model and method should be required, but the required flag doesn't + # let a chance to _inverse_name to be executed + model_id = fields.Many2one( + comodel_name="ir.model", string="Model", ondelete="cascade" + ) + method = fields.Char() + channel_id = fields.Many2one( comodel_name="queue.job.channel", string="Channel", @@ -520,6 +530,26 @@ def _default_channel(self): "See the module description for details.", ) + @api.depends("model_id.model", "method") + def _compute_name(self): + for record in self: + if not (record.model_id and record.method): + record.name = "" + continue + record.name = self.job_function_name(record.model_id.model, record.method) + + def _inverse_name(self): + groups = regex_job_function_name.match(self.name) + if not groups: + raise exceptions.UserError(_("Invalid job function: {}").format(self.name)) + model_name = groups[1] + method = groups[2] + model = self.env["ir.model"].search([("model", "=", model_name)], limit=1) + if not model: + raise exceptions.UserError(_("Model {} not found").format(model_name)) + self.model_id = model.id + self.method = method + @api.depends("retry_pattern") def _compute_edit_retry_pattern(self): for record in self: @@ -543,6 +573,10 @@ def _inverse_edit_related_action(self): except (ValueError, TypeError): raise exceptions.UserError(self._related_action_format_error_message()) + @staticmethod + def job_function_name(model_name, method_name): + return "<{}>.{}".format(model_name, method_name) + # TODO deprecated by :job-no-decorator: def _find_or_create_channel(self, channel_path): channel_model = self.env["queue.job.channel"] @@ -685,7 +719,7 @@ def unlink(self): # TODO deprecated by :job-no-decorator: def _register_job(self, model, job_method): - func_name = job_function_name(model, job_method) + func_name = self.job_function_name(model._name, job_method.__name__) if not self.search_count([("name", "=", func_name)]): channel = self._find_or_create_channel(job_method.default_channel) self.create({"name": func_name, "channel_id": channel.id}) diff --git a/queue_job/readme/USAGE.rst b/queue_job/readme/USAGE.rst index ee3988549f..08de8fef9b 100644 --- a/queue_job/readme/USAGE.rst +++ b/queue_job/readme/USAGE.rst @@ -25,7 +25,8 @@ Example of job function: .. code-block:: XML - .action_done]]> + + action_done diff --git a/queue_job/tests/__init__.py b/queue_job/tests/__init__.py index 5eb2dcb6f0..10138c469e 100644 --- a/queue_job/tests/__init__.py +++ b/queue_job/tests/__init__.py @@ -2,4 +2,5 @@ from . import test_runner_runner from . import test_json_field from . import test_model_job_channel +from . import test_model_job_function from . import test_queue_job_protected_write diff --git a/queue_job/tests/test_model_job_function.py b/queue_job/tests/test_model_job_function.py new file mode 100644 index 0000000000..c9bdea56e8 --- /dev/null +++ b/queue_job/tests/test_model_job_function.py @@ -0,0 +1,56 @@ +# copyright 2020 Camptocamp +# license lgpl-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from odoo import exceptions +from odoo.tests import common + + +class TestJobFunction(common.SavepointCase): + def test_function_name_compute(self): + function = self.env["queue.job.function"].create( + {"model_id": self.env.ref("base.model_res_users").id, "method": "read"} + ) + self.assertEqual(function.name, ".read") + + def test_function_name_inverse(self): + function = self.env["queue.job.function"].create({"name": ".read"}) + self.assertEqual(function.model_id.model, "res.users") + self.assertEqual(function.method, "read") + + def test_function_name_inverse_invalid_regex(self): + with self.assertRaises(exceptions.UserError): + self.env["queue.job.function"].create({"name": ".read"} + ) + + def test_function_job_config(self): + channel = self.env["queue.job.channel"].create( + {"name": "foo", "parent_id": self.env.ref("queue_job.channel_root").id} + ) + self.env["queue.job.function"].create( + { + "model_id": self.env.ref("base.model_res_users").id, + "method": "read", + "channel_id": channel.id, + "edit_retry_pattern": "{1: 2, 3: 4}", + "edit_related_action": ( + '{"enable": True,' + ' "func_name": "related_action_foo",' + ' "kwargs": {"b": 1}}' + ), + } + ) + self.assertEqual( + self.env["queue.job.function"].job_config(".read"), + self.env["queue.job.function"].JobConfig( + channel="root.foo", + retry_pattern={1: 2, 3: 4}, + related_action_enable=True, + related_action_func_name="related_action_foo", + related_action_kwargs={"b": 1}, + ), + ) diff --git a/queue_job/views/queue_job_views.xml b/queue_job/views/queue_job_views.xml index 791e560a84..7bdf028bb8 100644 --- a/queue_job/views/queue_job_views.xml +++ b/queue_job/views/queue_job_views.xml @@ -292,6 +292,8 @@
+ + diff --git a/test_queue_job/data/queue_job_function_data.xml b/test_queue_job/data/queue_job_function_data.xml index a75a862adb..8338045141 100644 --- a/test_queue_job/data/queue_job_function_data.xml +++ b/test_queue_job/data/queue_job_function_data.xml @@ -1,47 +1,51 @@ - .testing_method]]> + + testing_method - .job_with_retry_pattern]]> + + job_with_retry_pattern - .job_with_retry_pattern__no_zero]]> + + job_with_retry_pattern__no_zero - .job_sub_channel]]> + + job_sub_channel + + + job_a + - .testing_related_action__return_none]]> + + testing_related_action__return_none - .testing_related_action__kwargs]]> + + testing_related_action__kwargs - .testing_related_action__store]]> + + testing_related_action__store - - .job_a]]> - diff --git a/test_queue_job/tests/test_job_channels.py b/test_queue_job/tests/test_job_channels.py index f5b8c7e7ae..01173e0dd9 100644 --- a/test_queue_job/tests/test_job_channels.py +++ b/test_queue_job/tests/test_job_channels.py @@ -4,7 +4,7 @@ import odoo.tests.common as common from odoo import exceptions -from odoo.addons.queue_job.job import Job, job, job_function_name +from odoo.addons.queue_job.job import Job, job class TestJobChannels(common.TransactionCase): @@ -37,7 +37,9 @@ def test_channel_root(self): def test_channel_on_job(self): method = self.env["test.queue.channel"].job_a - path_a = job_function_name(self.env["test.queue.channel"], method) + path_a = self.env["queue.job.function"].job_function_name( + "test.queue.channel", "job_a" + ) job_func = self.function_model.search([("name", "=", path_a)]) self.assertEquals(job_func.channel, "root") @@ -71,8 +73,9 @@ def test_default_channel_no_xml(self): self.assertEquals(stored.channel, "root") def test_set_channel_from_record(self): - method = self.env["test.queue.channel"].job_sub_channel - func_name = job_function_name(self.env["test.queue.channel"], method) + func_name = self.env["queue.job.function"].job_function_name( + "test.queue.channel", "job_sub_channel" + ) job_func = self.function_model.search([("name", "=", func_name)]) self.assertEqual(job_func.channel, "root.sub.subsub") From 4cd3adae667f003329c86367cd0f299077db3d13 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 13:45:30 +0100 Subject: [PATCH 06/10] queue_job_cron: remove `@job` decorator --- queue_job_cron/data/data.xml | 5 +++++ queue_job_cron/models/ir_cron.py | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/queue_job_cron/data/data.xml b/queue_job_cron/data/data.xml index f59e15cb12..0a56d9df32 100644 --- a/queue_job_cron/data/data.xml +++ b/queue_job_cron/data/data.xml @@ -4,4 +4,9 @@ ir_cron + + + _compute_run_as_queue_job + + diff --git a/queue_job_cron/models/ir_cron.py b/queue_job_cron/models/ir_cron.py index c867bec38e..3c95ec341a 100644 --- a/queue_job_cron/models/ir_cron.py +++ b/queue_job_cron/models/ir_cron.py @@ -4,8 +4,6 @@ from odoo import api, fields, models -from odoo.addons.queue_job.job import job - _logger = logging.getLogger(__name__) @@ -30,7 +28,6 @@ def _compute_run_as_queue_job(self): else: cron.channel_id = False - @job(default_channel="root.ir_cron") def _run_job_as_queue_job(self, server_action): return server_action.run() From 8432350a3f96b1f100865d5e19bdc711d6a0b573 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 13:46:07 +0100 Subject: [PATCH 07/10] base_import_async: remove `@job` decorator --- base_import_async/__manifest__.py | 2 +- .../data/queue_job_function_data.xml | 21 +++++++++++++++++++ .../models/base_import_import.py | 5 ----- 3 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 base_import_async/data/queue_job_function_data.xml diff --git a/base_import_async/__manifest__.py b/base_import_async/__manifest__.py index 6ee3c839ae..12beaba8b3 100644 --- a/base_import_async/__manifest__.py +++ b/base_import_async/__manifest__.py @@ -11,7 +11,7 @@ "website": "https://github.com/OCA/queue", "category": "Generic Modules", "depends": ["base_import", "queue_job"], - "data": ["views/base_import_async.xml"], + "data": ["data/queue_job_function_data.xml", "views/base_import_async.xml"], "qweb": ["static/src/xml/import.xml"], "installable": True, "development_status": "Stable", diff --git a/base_import_async/data/queue_job_function_data.xml b/base_import_async/data/queue_job_function_data.xml new file mode 100644 index 0000000000..22cc8dbab0 --- /dev/null +++ b/base_import_async/data/queue_job_function_data.xml @@ -0,0 +1,21 @@ + + + + _split_file + + + + + _import_one_chunk + + + diff --git a/base_import_async/models/base_import_import.py b/base_import_async/models/base_import_import.py index 184470bf85..4db93532bf 100644 --- a/base_import_async/models/base_import_import.py +++ b/base_import_async/models/base_import_import.py @@ -13,7 +13,6 @@ from odoo.models import fix_import_export_id_paths from odoo.addons.queue_job.exception import FailedJobError -from odoo.addons.queue_job.job import job, related_action # options defined in base_import/import.js OPT_HAS_HEADER = "headers" @@ -124,8 +123,6 @@ def _extract_chunks(model_obj, fields, data, chunk_size): if row_from < len(data): yield row_from, len(data) - 1 - @job - @related_action("_related_action_attachment") def _split_file( self, model_name, @@ -172,8 +169,6 @@ def _split_file( self._link_attachment_to_job(delayed_job, attachment) priority += 1 - @job - @related_action("_related_action_attachment") def _import_one_chunk(self, model_name, attachment, options): model_obj = self.env[model_name] fields, data = self._read_csv_attachment(attachment, options) From b371414c72c53b86e88003b69a12bb2e0f7cd1cb Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 15:06:39 +0100 Subject: [PATCH 08/10] Add missing pylint check "development-status-allowed" --- .pylintrc-mandatory | 1 + 1 file changed, 1 insertion(+) diff --git a/.pylintrc-mandatory b/.pylintrc-mandatory index 7635cbb179..55893fe8b6 100644 --- a/.pylintrc-mandatory +++ b/.pylintrc-mandatory @@ -21,6 +21,7 @@ enable=anomalous-backslash-in-string, class-camelcase, dangerous-default-value, dangerous-view-replace-wo-priority, + development-status-allowed, duplicate-id-csv, duplicate-key, duplicate-xml-fields, From 509af7558cf5c662e15e7c91f76c3b9059a82eec Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 2 Nov 2020 15:19:44 +0100 Subject: [PATCH 09/10] Fix invalid development_status (accepted value is Production/Stable) --- base_import_async/__manifest__.py | 2 +- test_base_import_async/__manifest__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/base_import_async/__manifest__.py b/base_import_async/__manifest__.py index 12beaba8b3..7ea248c9f6 100644 --- a/base_import_async/__manifest__.py +++ b/base_import_async/__manifest__.py @@ -14,5 +14,5 @@ "data": ["data/queue_job_function_data.xml", "views/base_import_async.xml"], "qweb": ["static/src/xml/import.xml"], "installable": True, - "development_status": "Stable", + "development_status": "Production/Stable", } diff --git a/test_base_import_async/__manifest__.py b/test_base_import_async/__manifest__.py index 31c8dd637e..69a3712c31 100644 --- a/test_base_import_async/__manifest__.py +++ b/test_base_import_async/__manifest__.py @@ -16,5 +16,5 @@ "depends": ["base_import_async", "account"], "data": [], "installable": True, - "development_status": "Stable", + "development_status": "Production/Stable", } From 007922f8e4bc1445b50d65fd5a8866e9ee19690e Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Mon, 9 Nov 2020 14:42:01 +0100 Subject: [PATCH 10/10] Add missing sudo() when reading queue.job.function Users shouldn't need access to queue.job.function. Read config as sudo(). --- queue_job/job.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/queue_job/job.py b/queue_job/job.py index 718a5b2098..af8293564e 100644 --- a/queue_job/job.py +++ b/queue_job/job.py @@ -444,9 +444,13 @@ def __init__( self.job_model = self.env["queue.job"] self.job_model_name = "queue.job" - self.job_config = self.env["queue.job.function"].job_config( - self.env["queue.job.function"].job_function_name( - self.model_name, self.method_name + self.job_config = ( + self.env["queue.job.function"] + .sudo() + .job_config( + self.env["queue.job.function"].job_function_name( + self.model_name, self.method_name + ) ) )