From ddf636ec0632dc004d1bcd0457ad806117021dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Alix?= Date: Tue, 24 May 2022 14:32:44 +0200 Subject: [PATCH 1/5] [IMP] account_mass_reconcile_as_job: do not duplicate jobs --- account_mass_reconcile_as_job/models/mass_reconcile.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/account_mass_reconcile_as_job/models/mass_reconcile.py b/account_mass_reconcile_as_job/models/mass_reconcile.py index 3d959f00..37ac078f 100644 --- a/account_mass_reconcile_as_job/models/mass_reconcile.py +++ b/account_mass_reconcile_as_job/models/mass_reconcile.py @@ -5,6 +5,7 @@ import ast import logging from odoo import models +from odoo.addons.queue_job.job import identity_exact _logger = logging.getLogger(__name__) @@ -25,7 +26,8 @@ class AccountMassReconcile(models.Model): if as_job and self.env.context.get("mass_reconcile_as_job", True): for rec in self: - rec.with_delay().reconcile_as_job() + job_options = {"identity_key": identity_exact} + rec.with_delay(**job_options).reconcile_as_job() return True else: return super().run_reconcile() From fc4c93c19d90002617111c18ce0bba4b1b202fa9 Mon Sep 17 00:00:00 2001 From: Yannick Vaucher Date: Wed, 5 Jan 2022 09:19:58 +0100 Subject: [PATCH 2/5] account_mass_reconcile: Fix early commit by chunk --- .../models/base_advanced_reconciliation.py | 120 ++++++++++++------ 1 file changed, 82 insertions(+), 38 deletions(-) diff --git a/account_mass_reconcile/models/base_advanced_reconciliation.py b/account_mass_reconcile/models/base_advanced_reconciliation.py index 173d460b..32a161fc 100644 --- a/account_mass_reconcile/models/base_advanced_reconciliation.py +++ b/account_mass_reconcile/models/base_advanced_reconciliation.py @@ -5,7 +5,7 @@ import logging from itertools import product -from odoo import models +from odoo import api, models, registry from odoo.tools.translate import _ _logger = logging.getLogger(__name__) @@ -219,39 +219,10 @@ class MassReconcileAdvanced(models.AbstractModel): """ return False - def _rec_auto_lines_advanced(self, credit_lines, debit_lines): - """Advanced reconciliation main loop""" - # pylint: disable=invalid-commit + def _rec_group(self, reconcile_groups, lines_by_id): reconciled_ids = [] - reconcile_groups = [] - _logger.info("%d credit lines to reconcile", len(credit_lines)) - for idx, credit_line in enumerate(credit_lines, start=1): - if idx % 50 == 0: - _logger.info( - "... %d/%d credit lines inspected ...", idx, len(credit_lines) - ) - if self._skip_line(credit_line): - continue - opposite_lines = self._search_opposites(credit_line, debit_lines) - if not opposite_lines: - continue - opposite_ids = [line["id"] for line in opposite_lines] - line_ids = opposite_ids + [credit_line["id"]] - for group in reconcile_groups: - if any([lid in group for lid in opposite_ids]): - _logger.debug( - "New lines %s matched with an existing " "group %s", - line_ids, - group, - ) - group.update(line_ids) - break - else: - _logger.debug("New group of lines matched %s", line_ids) - reconcile_groups.append(set(line_ids)) - lines_by_id = {line["id"]: line for line in credit_lines + debit_lines} - _logger.info("Found %d groups to reconcile", len(reconcile_groups)) for group_count, reconcile_group_ids in enumerate(reconcile_groups, start=1): + _logger.debug( "Reconciling group %d/%d with ids %s", group_count, @@ -262,12 +233,85 @@ class MassReconcileAdvanced(models.AbstractModel): reconciled, full = self._reconcile_lines(group_lines, allow_partial=True) if reconciled and full: reconciled_ids += reconcile_group_ids + return reconciled_ids - if ( - self.env.context.get("commit_every", 0) - and group_count % self.env.context["commit_every"] == 0 - ): - self.env.cr.commit() - _logger.info("Commit the reconciliations after %d groups", group_count) + def _rec_group_by_chunk(self, reconcile_groups, lines_by_id, chunk_size): + """Commit after each chunk + + :param dict reconcile_grous: all groups to reconcile, will be splitted + by chunk + :param list lines_by_id: list of dict of move lines values, + the move lines we want to search for + :return: list of reconciled lines + """ + reconciled_ids = [] + + _logger.info("Reconciling by chunk of %d", chunk_size) + + # Copy and commit current transient model before creating a new cursor + # This is required to avoid CacheMiss when using data from `self` + # which is created during current transaction. + with api.Environment.manage(): + with registry(self.env.cr.dbname).cursor() as new_cr: + new_env = api.Environment(new_cr, self.env.uid, self.env.context) + self_env = self.with_env(new_env) + rec = self_env.create(self.copy_data()) + for i in range(0, len(reconcile_groups), chunk_size): + chunk = reconcile_groups[i : i + chunk_size] + _logger.debug("Reconcile group chunk %s", chunk) + try: + with api.Environment.manage(): + with registry(self.env.cr.dbname).cursor() as new_cr: + new_env = api.Environment( + new_cr, self.env.uid, self.env.context + ) + # Re-use the commited transient we just commited + self_env = self.with_env(new_env).browse(rec.id) + reconciled_ids += self_env._rec_group(chunk, lines_by_id) + except Exception as e: + msg = "Reconciliation failed for group chunk %s with error:\n%s" + _logger.exception(msg, chunk, e) + return reconciled_ids + + def _rec_auto_lines_advanced(self, credit_lines, debit_lines): + """Advanced reconciliation main loop""" + # pylint: disable=invalid-commit + reconciled_ids = [] + for rec in self: + commit_every = rec.account_id.company_id.reconciliation_commit_every + reconcile_groups = [] + _logger.info("%d credit lines to reconcile", len(credit_lines)) + for idx, credit_line in enumerate(credit_lines, start=1): + if idx % 50 == 0: + _logger.info( + "... %d/%d credit lines inspected ...", idx, len(credit_lines) + ) + if self._skip_line(credit_line): + continue + opposite_lines = self._search_opposites(credit_line, debit_lines) + if not opposite_lines: + continue + opposite_ids = [opp["id"] for opp in opposite_lines] + line_ids = opposite_ids + [credit_line["id"]] + for group in reconcile_groups: + if any([lid in group for lid in opposite_ids]): + _logger.debug( + "New lines %s matched with an existing " "group %s", + line_ids, + group, + ) + group.update(line_ids) + break + else: + _logger.debug("New group of lines matched %s", line_ids) + reconcile_groups.append(set(line_ids)) + lines_by_id = {line["id"]: line for line in credit_lines + debit_lines} + _logger.info("Found %d groups to reconcile", len(reconcile_groups)) + if commit_every: + reconciled_ids = self._rec_group_by_chunk( + reconcile_groups, lines_by_id, commit_every + ) + else: + reconciled_ids = self._rec_group(reconcile_groups, lines_by_id) _logger.info("Reconciliation is over") return reconciled_ids From 5b45b6029bd966283c6d084c6ccc2fca24347e69 Mon Sep 17 00:00:00 2001 From: Yannick Vaucher Date: Wed, 12 Jan 2022 09:58:13 +0100 Subject: [PATCH 3/5] Each reconciliation as a job Add an option to create one job per reconciliation. The aim is to make it more resilient to failure with large amount of data. --- .../models/__init__.py | 1 + .../models/base_reconciliation.py | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 account_mass_reconcile_as_job/models/base_reconciliation.py diff --git a/account_mass_reconcile_as_job/models/__init__.py b/account_mass_reconcile_as_job/models/__init__.py index 5fe2f827..d15affd7 100644 --- a/account_mass_reconcile_as_job/models/__init__.py +++ b/account_mass_reconcile_as_job/models/__init__.py @@ -1 +1,2 @@ from . import mass_reconcile +from . import base_reconciliation diff --git a/account_mass_reconcile_as_job/models/base_reconciliation.py b/account_mass_reconcile_as_job/models/base_reconciliation.py new file mode 100644 index 00000000..7537f18b --- /dev/null +++ b/account_mass_reconcile_as_job/models/base_reconciliation.py @@ -0,0 +1,47 @@ +# Copyright 2022 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). + +import ast +import logging + +from odoo import models + +_logger = logging.getLogger(__name__) + +try: + from odoo.addons.queue_job.job import job +except ImportError: + _logger.debug('Can not `import queue_job`.') + + +class MassReconcileBase(models.AbstractModel): + _inherit = 'mass.reconcile.base' + + # WARNING: this has limitation as it creates a job based on an object wizard + # when the transient model will be garbadge collected the job won't + # be able to run anymore. + # FIXME: move or copy wizard data configuration in a standard model or refactor + # to have it as a parameter. + + def _reconcile_lines(self, lines, allow_partial=False): + as_job = self.env['ir.config_parameter'].sudo().get_param( + 'account.mass.reconcile.lines.as.job', default=False + ) + try: + as_job = ast.literal_eval(as_job) if as_job else False + except ValueError: + as_job = False + + if as_job and self.env.context.get("reconcile_lines_as_job", True): + self.with_delay().reconcile_lines_as_job( + lines, + allow_partial=allow_partial + ) + # Report is not available with reconcile jobs + return False, False + else: + return super()._reconcile_lines(lines, allow_partial=allow_partial) + + @job(default_channel='root.mass_reconcile') + def reconcile_lines_as_job(self, lines, allow_partial=False): + self.with_context(reconcile_lines_as_job=False)._reconcile_lines(lines, allow_partial=allow_partial) From 90025cb6539a63d53499ce8f16cdb787a44cc292 Mon Sep 17 00:00:00 2001 From: Akim Juillerat Date: Mon, 4 Jul 2022 13:11:26 +0200 Subject: [PATCH 4/5] Migrate job decorator to XML function --- .../data/queue_job_function_data.xml | 11 +++++++++++ .../models/base_reconciliation.py | 14 +++++--------- .../models/mass_reconcile.py | 1 + 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/account_mass_reconcile_as_job/data/queue_job_function_data.xml b/account_mass_reconcile_as_job/data/queue_job_function_data.xml index 5e69f25b..ae34c7e2 100644 --- a/account_mass_reconcile_as_job/data/queue_job_function_data.xml +++ b/account_mass_reconcile_as_job/data/queue_job_function_data.xml @@ -10,4 +10,15 @@ /> reconcile_as_job + + + + reconcile_lines_as_job + diff --git a/account_mass_reconcile_as_job/models/base_reconciliation.py b/account_mass_reconcile_as_job/models/base_reconciliation.py index 7537f18b..9cefebd3 100644 --- a/account_mass_reconcile_as_job/models/base_reconciliation.py +++ b/account_mass_reconcile_as_job/models/base_reconciliation.py @@ -8,14 +8,9 @@ from odoo import models _logger = logging.getLogger(__name__) -try: - from odoo.addons.queue_job.job import job -except ImportError: - _logger.debug('Can not `import queue_job`.') - class MassReconcileBase(models.AbstractModel): - _inherit = 'mass.reconcile.base' + _inherit = "mass.reconcile.base" # WARNING: this has limitation as it creates a job based on an object wizard # when the transient model will be garbadge collected the job won't @@ -24,8 +19,10 @@ class MassReconcileBase(models.AbstractModel): # to have it as a parameter. def _reconcile_lines(self, lines, allow_partial=False): - as_job = self.env['ir.config_parameter'].sudo().get_param( - 'account.mass.reconcile.lines.as.job', default=False + as_job = ( + self.env["ir.config_parameter"] + .sudo() + .get_param("account.mass.reconcile.lines.as.job", default=False) ) try: as_job = ast.literal_eval(as_job) if as_job else False @@ -42,6 +39,5 @@ class MassReconcileBase(models.AbstractModel): else: return super()._reconcile_lines(lines, allow_partial=allow_partial) - @job(default_channel='root.mass_reconcile') def reconcile_lines_as_job(self, lines, allow_partial=False): self.with_context(reconcile_lines_as_job=False)._reconcile_lines(lines, allow_partial=allow_partial) diff --git a/account_mass_reconcile_as_job/models/mass_reconcile.py b/account_mass_reconcile_as_job/models/mass_reconcile.py index 37ac078f..79829619 100644 --- a/account_mass_reconcile_as_job/models/mass_reconcile.py +++ b/account_mass_reconcile_as_job/models/mass_reconcile.py @@ -5,6 +5,7 @@ import ast import logging from odoo import models + from odoo.addons.queue_job.job import identity_exact _logger = logging.getLogger(__name__) From 14862989dea8972ca581eb475d83656e9dfbdfb6 Mon Sep 17 00:00:00 2001 From: Akim Juillerat Date: Mon, 11 Jul 2022 12:29:23 +0200 Subject: [PATCH 5/5] Do not rely on wizard to reconcile lines as job --- .../models/base_reconciliation.py | 22 ++-- .../tests/__init__.py | 1 + .../tests/test_scenario_reconcile_as_job.py | 112 ++++++++++++++++++ 3 files changed, 125 insertions(+), 10 deletions(-) create mode 100644 account_mass_reconcile_as_job/tests/test_scenario_reconcile_as_job.py diff --git a/account_mass_reconcile_as_job/models/base_reconciliation.py b/account_mass_reconcile_as_job/models/base_reconciliation.py index 9cefebd3..2a8fa0c7 100644 --- a/account_mass_reconcile_as_job/models/base_reconciliation.py +++ b/account_mass_reconcile_as_job/models/base_reconciliation.py @@ -4,7 +4,7 @@ import ast import logging -from odoo import models +from odoo import api, models _logger = logging.getLogger(__name__) @@ -12,12 +12,6 @@ _logger = logging.getLogger(__name__) class MassReconcileBase(models.AbstractModel): _inherit = "mass.reconcile.base" - # WARNING: this has limitation as it creates a job based on an object wizard - # when the transient model will be garbadge collected the job won't - # be able to run anymore. - # FIXME: move or copy wizard data configuration in a standard model or refactor - # to have it as a parameter. - def _reconcile_lines(self, lines, allow_partial=False): as_job = ( self.env["ir.config_parameter"] @@ -30,14 +24,22 @@ class MassReconcileBase(models.AbstractModel): as_job = False if as_job and self.env.context.get("reconcile_lines_as_job", True): + wiz_data = self.copy_data()[0] self.with_delay().reconcile_lines_as_job( lines, - allow_partial=allow_partial + allow_partial=allow_partial, + wiz_creation_data=(self._name, wiz_data), ) # Report is not available with reconcile jobs return False, False else: return super()._reconcile_lines(lines, allow_partial=allow_partial) - def reconcile_lines_as_job(self, lines, allow_partial=False): - self.with_context(reconcile_lines_as_job=False)._reconcile_lines(lines, allow_partial=allow_partial) + @api.model + def reconcile_lines_as_job( + self, lines, allow_partial=False, wiz_creation_data=False + ): + new_wiz = self.env[wiz_creation_data[0]].create(wiz_creation_data[1]) + return new_wiz.with_context(reconcile_lines_as_job=False)._reconcile_lines( + lines, allow_partial=allow_partial + ) diff --git a/account_mass_reconcile_as_job/tests/__init__.py b/account_mass_reconcile_as_job/tests/__init__.py index 111a97bc..0ff08edc 100644 --- a/account_mass_reconcile_as_job/tests/__init__.py +++ b/account_mass_reconcile_as_job/tests/__init__.py @@ -1 +1,2 @@ from . import test_mass_reconcile_as_job +from . import test_scenario_reconcile_as_job diff --git a/account_mass_reconcile_as_job/tests/test_scenario_reconcile_as_job.py b/account_mass_reconcile_as_job/tests/test_scenario_reconcile_as_job.py new file mode 100644 index 00000000..bcd39821 --- /dev/null +++ b/account_mass_reconcile_as_job/tests/test_scenario_reconcile_as_job.py @@ -0,0 +1,112 @@ +# Copyright 2022 Camptocamp SA +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl) +from odoo.tests import tagged + +from odoo.addons.account_mass_reconcile.tests.test_scenario_reconcile import ( + TestScenarioReconcile, +) +from odoo.addons.queue_job.tests.common import trap_jobs + + +@tagged("post_install", "-at_install") +class TestScenarioReconcileAsJob(TestScenarioReconcile): + def test_scenario_reconcile_as_job(self): + self.env["ir.config_parameter"].sudo().set_param( + "account.mass.reconcile.as.job", True + ) + invoice = self.create_invoice() + self.assertEqual("posted", invoice.state) + + receivalble_account_id = invoice.partner_id.property_account_receivable_id.id + # create payment + payment = self.env["account.payment"].create( + { + "partner_type": "customer", + "payment_type": "inbound", + "partner_id": invoice.partner_id.id, + "destination_account_id": receivalble_account_id, + "amount": 50.0, + "journal_id": self.bank_journal.id, + } + ) + payment.action_post() + + # create the mass reconcile record + mass_rec = self.mass_rec_obj.create( + { + "name": "mass_reconcile_1", + "account": invoice.partner_id.property_account_receivable_id.id, + "reconcile_method": [(0, 0, {"name": "mass.reconcile.simple.partner"})], + } + ) + with trap_jobs() as trap: + # call the automatic reconcilation method + mass_rec.run_reconcile() + trap.assert_jobs_count(1) + trap.assert_enqueued_job( + self.env["account.mass.reconcile"].reconcile_as_job, + args=(), + ) + job = trap.enqueued_jobs[0] + self.assertEqual(job.state, "pending") + trap.perform_enqueued_jobs() + self.assertEqual("paid", invoice.payment_state) + + def test_scenario_reconcile_lines_as_job(self): + self.env["ir.config_parameter"].sudo().set_param( + "account.mass.reconcile.as.job", True + ) + self.env["ir.config_parameter"].sudo().set_param( + "account.mass.reconcile.lines.as.job", True + ) + invoice = self.create_invoice() + self.assertEqual("posted", invoice.state) + + receivalble_account_id = invoice.partner_id.property_account_receivable_id.id + # create payment + payment = self.env["account.payment"].create( + { + "partner_type": "customer", + "payment_type": "inbound", + "partner_id": invoice.partner_id.id, + "destination_account_id": receivalble_account_id, + "amount": 50.0, + "journal_id": self.bank_journal.id, + } + ) + payment.action_post() + + # create the mass reconcile record + mass_rec = self.mass_rec_obj.create( + { + "name": "mass_reconcile_1", + "account": invoice.partner_id.property_account_receivable_id.id, + "reconcile_method": [(0, 0, {"name": "mass.reconcile.simple.partner"})], + } + ) + with trap_jobs() as trap: + self.assertFalse(self.env["mass.reconcile.simple.partner"].search([])) + # call the automatic reconcilation method + mass_rec.run_reconcile() + trap.assert_jobs_count(1) + trap.assert_enqueued_job( + self.env["account.mass.reconcile"].reconcile_as_job, + args=(), + ) + job = trap.enqueued_jobs[0] + self.assertEqual(job.state, "pending") + self.assertFalse(self.env["mass.reconcile.simple.partner"].search([])) + trap.perform_enqueued_jobs() + trap.assert_jobs_count(2) + job_2 = trap.enqueued_jobs[1] + # Cannot use assert_enqueue_job with all the parameters + self.assertEqual(job_2.model_name, "mass.reconcile.simple.partner") + self.assertEqual(job_2.method_name, "reconcile_lines_as_job") + self.assertEqual(job_2.state, "pending") + # Delete existing wizard to make sure the job can still after after + # the wizard is garbage collected + wiz = self.env["mass.reconcile.simple.partner"].search([]) + self.assertTrue(wiz) + wiz.unlink() + trap.perform_enqueued_jobs() + self.assertEqual("paid", invoice.payment_state)