Fix concurrency race condition on picking state

When the 2 last moves of a stock.picking are assigned at the same
time by 2 jobs (different products both available), none of the
transaction will see that it is the last move to be assigned. As a
result, the picking will stay in state "confirmed" even if all its
moves are assigned.

Lock the stock.picking records when we call auto_assign.
If we have many pickings touched for the same product, the lock
can be quite large, so we may have to find a better option.

I could not write a test to exercise this, because we can't have 2
transactions being aware of it, even if we create the picking in demo
data as tests can be run during install.

stock_move_auto_assign/models/product_product.py

@@ -1,10 +1,17 @@
 # Copyright 2020 Camptocamp SA
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
+import logging
+
+import psycopg2
+
 from odoo import models
 
+from odoo.addons.queue_job.exception import RetryableJobError
 from odoo.addons.queue_job.job import job
 
+_logger = logging.getLogger(__name__)
+
 
 class ProductProduct(models.Model):
     _inherit = "product.product"
@@ -25,9 +32,10 @@ class ProductProduct(models.Model):
             # out anyway.
         ]
 
+    # TODO remove decorator
     @job(default_channel="root.stock_auto_assign")
     def moves_auto_assign(self, locations):
-        """Try to reserve moves based on product and locations
+        """Job trying to reserve moves based on product and locations
 
         When a product has been added to a location, it searches all*
         the moves with a source equal or above this location and try
@@ -38,4 +46,23 @@ class ProductProduct(models.Model):
         """
         self.ensure_one()
         moves = self.env["stock.move"].search(self._moves_auto_assign_domain(locations))
+        pickings = moves.picking_id
+        if not pickings:
+            return
+        try:
+            self.env.cr.execute(
+                "SELECT id FROM stock_picking WHERE id IN %s FOR UPDATE NOWAIT",
+                (tuple(pickings.ids),),
+            )
+        except psycopg2.OperationalError as err:
+            if err.pgcode == "55P03":  # could not obtain the lock
+                _logger.debug(
+                    "Another job is already auto-assigning moves and acquired a"
+                    " lock on one or some of stock.picking%s, retry later.",
+                    tuple(pickings.ids),
+                )
+                raise RetryableJobError(
+                    "Could not obtain lock on transfers, will retry.", ignore_retry=True
+                )
+            raise
         moves._action_assign()

stock_move_auto_assign/tests/common.py

@@ -0,0 +1,56 @@
+# Copyright 2020-2021 Camptocamp SA
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+
+from odoo.tests import SavepointCase
+
+
+class StockMoveAutoAssignCase(SavepointCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))
+
+        cls.wh = cls.env.ref("stock.warehouse0")
+        cls.out_type = cls.wh.out_type_id
+        cls.in_type = cls.wh.in_type_id
+        cls.int_type = cls.wh.int_type_id
+
+        cls.customer_loc = cls.env.ref("stock.stock_location_customers")
+        cls.supplier_loc = cls.env.ref("stock.stock_location_suppliers")
+        cls.shelf1_loc = cls.env.ref("stock.stock_location_components")
+        cls.shelf2_loc = cls.env.ref("stock.stock_location_14")
+
+        cls.product = cls.env["product.product"].create(
+            {"name": "Product", "type": "product"}
+        )
+
+    def _create_move(
+        self,
+        product,
+        picking_type,
+        qty=1.0,
+        state="confirmed",
+        procure_method="make_to_stock",
+        move_dest=None,
+    ):
+        source = picking_type.default_location_src_id or self.supplier_loc
+        dest = picking_type.default_location_dest_id or self.customer_loc
+        move_vals = {
+            "name": product.name,
+            "product_id": product.id,
+            "product_uom_qty": qty,
+            "product_uom": product.uom_id.id,
+            "picking_type_id": picking_type.id,
+            "location_id": source.id,
+            "location_dest_id": dest.id,
+            "state": state,
+            "procure_method": procure_method,
+        }
+        if move_dest:
+            move_vals["move_dest_ids"] = [(4, move_dest.id, False)]
+        return self.env["stock.move"].create(move_vals)
+
+    def _update_qty_in_location(self, location, product, quantity):
+        location.env["stock.quant"]._update_available_quantity(
+            product, location, quantity
+        )

stock_move_auto_assign/tests/test_auto_assign.py

@@ -1,65 +1,18 @@
 # Copyright 2020 Camptocamp SA
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
-from odoo.tests import SavepointCase
-
 from odoo.addons.queue_job.job import identity_exact
 from odoo.addons.queue_job.tests.common import mock_with_delay
 
+from .common import StockMoveAutoAssignCase
 
-class TestStockMoveAutoAssign(SavepointCase):
-    @classmethod
-    def setUpClass(cls):
-        super().setUpClass()
-        cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))
-
-        cls.wh = cls.env.ref("stock.warehouse0")
-        cls.out_type = cls.wh.out_type_id
-        cls.in_type = cls.wh.in_type_id
-        cls.int_type = cls.wh.int_type_id
-
-        cls.customer_loc = cls.env.ref("stock.stock_location_customers")
-        cls.supplier_loc = cls.env.ref("stock.stock_location_suppliers")
-        cls.shelf1_loc = cls.env.ref("stock.stock_location_components")
-        cls.shelf2_loc = cls.env.ref("stock.stock_location_14")
-
-        cls.product = cls.env["product.product"].create(
-            {"name": "Product", "type": "product"}
-        )
-
-    def _create_move(
-        self,
-        product,
-        picking_type,
-        qty=1.0,
-        state="confirmed",
-        procure_method="make_to_stock",
-        move_dest=None,
-    ):
-        source = picking_type.default_location_src_id or self.supplier_loc
-        dest = picking_type.default_location_dest_id or self.customer_loc
-        move_vals = {
-            "name": product.name,
-            "product_id": product.id,
-            "product_uom_qty": qty,
-            "product_uom": product.uom_id.id,
-            "picking_type_id": picking_type.id,
-            "location_id": source.id,
-            "location_dest_id": dest.id,
-            "state": state,
-            "procure_method": procure_method,
-        }
-        if move_dest:
-            move_vals["move_dest_ids"] = [(4, move_dest.id, False)]
-        return self.env["stock.move"].create(move_vals)
-
-    def _update_qty_in_location(self, location, product, quantity):
-        self.env["stock.quant"]._update_available_quantity(product, location, quantity)
 
+class TestStockMoveAutoAssign(StockMoveAutoAssignCase):
     def test_job_assign_confirmed_move(self):
         """Test job method, assign moves matching product and location"""
         move1 = self._create_move(self.product, self.out_type)
         move2 = self._create_move(self.product, self.out_type)
+        (move1 | move2)._assign_picking()
         # put stock in Stock/Shelf 1, the move has a source location in Stock
         self._update_qty_in_location(self.shelf1_loc, self.product, 100)
         self.product.moves_auto_assign(self.shelf1_loc)