From 08810dbcce3c96855c2c5bfe266ba9d7ea61fc42 Mon Sep 17 00:00:00 2001
From: Ankush Menat <ankush@frappe.io>
Date: Sun, 30 Jan 2022 16:25:42 +0530
Subject: [PATCH] fix: add unique constraint on bin at db level

---
 erpnext/stock/doctype/bin/bin.json    |  8 ++++--
 erpnext/stock/doctype/bin/bin.py      |  2 +-
 erpnext/stock/doctype/bin/test_bin.py | 35 ++++++++++++++++++++++++---
 erpnext/stock/utils.py                | 30 ++++++++++++-----------
 4 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/erpnext/stock/doctype/bin/bin.json b/erpnext/stock/doctype/bin/bin.json
index 8e79f0e555..56dc71c57e 100644
--- a/erpnext/stock/doctype/bin/bin.json
+++ b/erpnext/stock/doctype/bin/bin.json
@@ -33,6 +33,7 @@
    "oldfieldtype": "Link",
    "options": "Warehouse",
    "read_only": 1,
+   "reqd": 1,
    "search_index": 1
   },
   {
@@ -46,6 +47,7 @@
    "oldfieldtype": "Link",
    "options": "Item",
    "read_only": 1,
+   "reqd": 1,
    "search_index": 1
   },
   {
@@ -169,10 +171,11 @@
  "idx": 1,
  "in_create": 1,
  "links": [],
- "modified": "2021-03-30 23:09:39.572776",
+ "modified": "2022-01-30 17:04:54.715288",
  "modified_by": "Administrator",
  "module": "Stock",
  "name": "Bin",
+ "naming_rule": "Expression (old style)",
  "owner": "Administrator",
  "permissions": [
   {
@@ -200,5 +203,6 @@
  "quick_entry": 1,
  "search_fields": "item_code,warehouse",
  "sort_field": "modified",
- "sort_order": "ASC"
+ "sort_order": "ASC",
+ "states": []
 }
\ No newline at end of file
diff --git a/erpnext/stock/doctype/bin/bin.py b/erpnext/stock/doctype/bin/bin.py
index 0ef7ce2923..c34e9d05ce 100644
--- a/erpnext/stock/doctype/bin/bin.py
+++ b/erpnext/stock/doctype/bin/bin.py
@@ -123,7 +123,7 @@ class Bin(Document):
 		self.db_set('projected_qty', self.projected_qty)
 
 def on_doctype_update():
-	frappe.db.add_index("Bin", ["item_code", "warehouse"])
+	frappe.db.add_unique("Bin", ["item_code", "warehouse"], constraint_name="unique_item_warehouse")
 
 
 def update_stock(bin_name, args, allow_negative_stock=False, via_landed_cost_voucher=False):
diff --git a/erpnext/stock/doctype/bin/test_bin.py b/erpnext/stock/doctype/bin/test_bin.py
index 9c390d94b4..250126c6b9 100644
--- a/erpnext/stock/doctype/bin/test_bin.py
+++ b/erpnext/stock/doctype/bin/test_bin.py
@@ -1,9 +1,36 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # See license.txt
 
-import unittest
+import frappe
 
-# test_records = frappe.get_test_records('Bin')
+from erpnext.stock.doctype.item.test_item import make_item
+from erpnext.stock.utils import _create_bin
+from erpnext.tests.utils import ERPNextTestCase
 
-class TestBin(unittest.TestCase):
-	pass
+
+class TestBin(ERPNextTestCase):
+
+
+	def test_concurrent_inserts(self):
+		""" Ensure no duplicates are possible in case of concurrent inserts"""
+		item_code = "_TestConcurrentBin"
+		make_item(item_code)
+		warehouse = "_Test Warehouse - _TC"
+
+		bin1 = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse)
+		bin1.insert()
+
+		bin2 = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse)
+		with self.assertRaises(frappe.UniqueValidationError):
+			bin2.insert()
+
+		# util method should handle it
+		bin = _create_bin(item_code, warehouse)
+		self.assertEqual(bin.item_code, item_code)
+
+		frappe.db.rollback()
+
+	def test_index_exists(self):
+		indexes = frappe.db.sql("show index from tabBin where Non_unique = 0", as_dict=1)
+		if not any(index.get("Key_name") == "unique_item_warehouse" for index in indexes):
+			self.fail(f"Expected unique index on item-warehouse")
diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py
index f620c183eb..7c63c17ad0 100644
--- a/erpnext/stock/utils.py
+++ b/erpnext/stock/utils.py
@@ -176,13 +176,7 @@ def get_latest_stock_balance():
 def get_bin(item_code, warehouse):
 	bin = frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": warehouse})
 	if not bin:
-		bin_obj = frappe.get_doc({
-			"doctype": "Bin",
-			"item_code": item_code,
-			"warehouse": warehouse,
-		})
-		bin_obj.flags.ignore_permissions = 1
-		bin_obj.insert()
+		bin_obj = _create_bin(item_code, warehouse)
 	else:
 		bin_obj = frappe.get_doc('Bin', bin, for_update=True)
 	bin_obj.flags.ignore_permissions = True
@@ -192,16 +186,24 @@ def get_or_make_bin(item_code: str , warehouse: str) -> str:
 	bin_record = frappe.db.get_value('Bin', {'item_code': item_code, 'warehouse': warehouse})
 
 	if not bin_record:
-		bin_obj = frappe.get_doc({
-			"doctype": "Bin",
-			"item_code": item_code,
-			"warehouse": warehouse,
-		})
+		bin_obj = _create_bin(item_code, warehouse)
+		bin_record = bin_obj.name
+	return bin_record
+
+def _create_bin(item_code, warehouse):
+	"""Create a bin and take care of concurrent inserts."""
+
+	bin_creation_savepoint = "create_bin"
+	try:
+		frappe.db.savepoint(bin_creation_savepoint)
+		bin_obj = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse)
 		bin_obj.flags.ignore_permissions = 1
 		bin_obj.insert()
-		bin_record = bin_obj.name
+	except frappe.UniqueValidationError:
+		frappe.db.rollback(save_point=bin_creation_savepoint)  # preserve transaction in postgres
+		bin_obj = frappe.get_last_doc("Bin", {"item_code": item_code, "warehouse": warehouse})
 
-	return bin_record
+	return bin_obj
 
 def update_bin(args, allow_negative_stock=False, via_landed_cost_voucher=False):
 	"""WARNING: This function is deprecated. Inline this function instead of using it."""