From 4bd2285159c5bc95b9c18249a270a18870be0c99 Mon Sep 17 00:00:00 2001
From: Rushabh Mehta <rmehta@gmail.com>
Date: Fri, 25 Nov 2016 16:14:00 +0530
Subject: [PATCH] File Based Locking at Document Level (#2374)

* [redesign] improved locking in documents and redesigned recent documents

* [minor] patch to update doctype in existing documents
---
 .../core/doctype/error_log/error_log_list.js  |  7 +-
 frappe/database.py                            |  3 +-
 frappe/exceptions.py                          |  1 +
 frappe/model/document.py                      | 64 ++++++++--------
 frappe/patches.txt                            |  1 +
 frappe/patches/v7_2/__init__.py               |  0
 frappe/patches/v7_2/set_doctype_engine.py     |  6 ++
 frappe/public/js/frappe/form/grid.js          |  6 ++
 frappe/public/js/frappe/model/indicator.js    | 14 +---
 frappe/public/js/frappe/model/model.js        |  3 -
 frappe/public/js/frappe/router.js             |  6 ++
 frappe/public/js/frappe/ui/filters/filters.js |  5 +-
 .../js/frappe/ui/toolbar/awesome_bar.js       | 73 +++++++++++--------
 .../public/js/frappe/ui/toolbar/navbar.html   |  2 +
 frappe/public/js/legacy/form.js               |  3 -
 frappe/tests/test_document_locks.py           | 18 +++++
 frappe/utils/file_lock.py                     | 10 ++-
 17 files changed, 132 insertions(+), 90 deletions(-)
 create mode 100644 frappe/patches/v7_2/__init__.py
 create mode 100644 frappe/patches/v7_2/set_doctype_engine.py
 create mode 100644 frappe/tests/test_document_locks.py

diff --git a/frappe/core/doctype/error_log/error_log_list.js b/frappe/core/doctype/error_log/error_log_list.js
index 7e91292e1f..59c4a3011c 100644
--- a/frappe/core/doctype/error_log/error_log_list.js
+++ b/frappe/core/doctype/error_log/error_log_list.js
@@ -10,7 +10,12 @@ frappe.listview_settings['Error Log'] = {
 	order_by: "seen asc, modified desc",
 	onload: function(listview) {
 		listview.page.add_menu_item(__("Clear Error Logs"), function() {
-			frappe.call({method:'frappe.core.doctype.error_log.error_log.clear_error_logs'});
+			frappe.call({
+				method:'frappe.core.doctype.error_log.error_log.clear_error_logs',
+				callback: function() {
+					listview.refresh();
+				}
+			});
 		});
 	}
 };
diff --git a/frappe/database.py b/frappe/database.py
index 78beb74554..0a0bb766f5 100644
--- a/frappe/database.py
+++ b/frappe/database.py
@@ -712,8 +712,7 @@ class Database:
 			return frappe.defaults.get_defaults(parent)
 
 	def begin(self):
-		pass
-		#self.sql("start transaction")
+		self.sql("start transaction")
 
 	def commit(self):
 		"""Commit current transaction. Calls SQL `COMMIT`."""
diff --git a/frappe/exceptions.py b/frappe/exceptions.py
index 2bd5e94851..c09ebd26e0 100644
--- a/frappe/exceptions.py
+++ b/frappe/exceptions.py
@@ -62,3 +62,4 @@ class AppNotInstalledError(ValidationError): pass
 class IncorrectSitePath(NotFound): pass
 class ImplicitCommitError(ValidationError): pass
 class RetryBackgroundJobError(Exception): pass
+class DocumentLockedError(ValidationError): pass
diff --git a/frappe/model/document.py b/frappe/model/document.py
index b582b7f3ae..f22b8b22c0 100644
--- a/frappe/model/document.py
+++ b/frappe/model/document.py
@@ -3,8 +3,9 @@
 
 from __future__ import unicode_literals
 import frappe
+import time
 from frappe import _, msgprint
-from frappe.utils import flt, cstr, now, get_datetime_str
+from frappe.utils import flt, cstr, now, get_datetime_str, file_lock
 from frappe.utils.background_jobs import enqueue
 from frappe.model.base_document import BaseDocument, get_controller
 from frappe.model.naming import set_new_name
@@ -156,23 +157,6 @@ class Document(BaseDocument):
 		frappe.msgprint(msg)
 		raise frappe.PermissionError(msg)
 
-	def lock(self):
-		'''Will set docstatus to 3 + the current docstatus and mark it as queued
-
-		3 = queued for saving
-		4 = queued for submission
-		5 = queued for cancellation
-		'''
-		self.db_set('docstatus', 3 + self.docstatus, update_modified = False)
-
-	def unlock(self):
-		'''set the original docstatus at the time it was locked in the controller'''
-		current_docstatus = self.db_get('docstatus') - 4
-		if current_docstatus < 0:
-			current_docstatus = 0
-
-		self.db_set('docstatus', current_docstatus, update_modified = False)
-
 	def insert(self, ignore_permissions=None):
 		"""Insert the document in the database (as a new document).
 		This will check for user permissions and execute `before_insert`,
@@ -494,7 +478,9 @@ class Document(BaseDocument):
 		self._action = "save"
 		if not self.get('__islocal'):
 			if self.meta.issingle:
-				modified = frappe.db.get_value(self.doctype, self.name, "modified")
+				modified = frappe.db.sql('''select value from tabSingles
+					where doctype=%s and field='modified' for update''', self.doctype)
+				modified = modified and modified[0][0]
 				if cstr(modified) and cstr(modified) != cstr(self._original_modified):
 					conflict = True
 			else:
@@ -530,13 +516,7 @@ class Document(BaseDocument):
 		- Submit (1) > Submit (1)
 		- Submit (1) > Cancel (2)
 
-		If docstatus is > 2, it will throw exception as document is deemed queued
 		"""
-
-		if self.docstatus > 2:
-			frappe.throw(_('This document is currently queued for execution. Please try again'),
-				title=_('Document Queued'), indicator='red')
-
 		if not self.docstatus:
 			self.docstatus = 0
 		if docstatus==0:
@@ -1014,28 +994,46 @@ class Document(BaseDocument):
 	def queue_action(self, action, **kwargs):
 		'''Run an action in background. If the action has an inner function,
 		like _submit for submit, it will call that instead'''
-		if action in ('save', 'submit', 'cancel'):
-			# set docstatus explicitly again due to inconsistent action
-			self.docstatus = {'save':0, 'submit':1, 'cancel': 2}[action]
-		else:
-			raise 'Action must be one of save, submit, cancel'
-
 		# call _submit instead of submit, so you can override submit to call
 		# run_delayed based on some action
 		# See: Stock Reconciliation
 		if hasattr(self, '_' + action):
 			action = '_' + action
 
+		if file_lock.lock_exists(self.get_signature()):
+			frappe.throw(_('This document is currently queued for execution. Please try again'),
+				title=_('Document Queued'), indicator='red')
+
 		self.lock()
-		frappe.db.commit()
 		enqueue('frappe.model.document.execute_action', doctype=self.doctype, name=self.name,
 			action=action, **kwargs)
 
+	def lock(self, timeout=None):
+		'''Creates a lock file for the given document. If timeout is set,
+		it will retry every 1 second for acquiring the lock again
+
+		:param timeout: Timeout in seconds, default 0'''
+		signature = self.get_signature()
+		if file_lock.lock_exists(signature):
+			lock_exists = True
+			if timeout:
+				for i in range(timeout):
+					time.sleep(1)
+					if not file_lock.lock_exists(signature):
+						lock_exists = False
+						break
+			if lock_exists:
+				raise frappe.DocumentLockedError
+		file_lock.create_lock(signature)
+
+	def unlock(self):
+		'''Delete the lock file for this document'''
+		file_lock.delete_lock(self.get_signature())
+
 def execute_action(doctype, name, action, **kwargs):
 	'''Execute an action on a document (called by background worker)'''
 	doc = frappe.get_doc(doctype, name)
 	doc.unlock()
-	frappe.db.commit()
 	try:
 		getattr(doc, action)(**kwargs)
 	except Exception:
diff --git a/frappe/patches.txt b/frappe/patches.txt
index 7bd4faef17..80e6b98f58 100644
--- a/frappe/patches.txt
+++ b/frappe/patches.txt
@@ -146,3 +146,4 @@ execute:frappe.db.set_default('language', '')
 frappe.patches.v7_1.refactor_integration_broker
 frappe.patches.v7_1.set_backup_limit
 frappe.patches.v7_1.disabled_print_settings_for_custom_print_format
+frappe.patches.v7_2.set_doctype_engine
diff --git a/frappe/patches/v7_2/__init__.py b/frappe/patches/v7_2/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/frappe/patches/v7_2/set_doctype_engine.py b/frappe/patches/v7_2/set_doctype_engine.py
new file mode 100644
index 0000000000..fa914003f3
--- /dev/null
+++ b/frappe/patches/v7_2/set_doctype_engine.py
@@ -0,0 +1,6 @@
+import frappe
+
+def execute():
+	for t in frappe.db.sql('show table status'):
+		if t[0].startswith('tab'):
+			frappe.db.sql('update tabDocType set engine=%s where name=%s', (t[1], t[0][3:]))
\ No newline at end of file
diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js
index 551194b600..76dc3dee8e 100644
--- a/frappe/public/js/frappe/form/grid.js
+++ b/frappe/public/js/frappe/form/grid.js
@@ -836,7 +836,13 @@ frappe.ui.form.GridRow = Class.extend({
 			field.$input.on('keydown', function(e) {
 				var values = me.frm.doc[me.grid.df.fieldname];
 				var fieldname = $(this).attr('data-fieldname');
+				var fieldtype = $(this).attr('data-fieldtype');
+
 				// TAB
+				if(in_list(['Text', 'Small Text'], fieldtype)) {
+					return;
+				}
+
 				if(e.which==TAB) {
 					// last column
 					if(me.grid.wrapper.find('input:enabled:last').get(0)===this) {
diff --git a/frappe/public/js/frappe/model/indicator.js b/frappe/public/js/frappe/model/indicator.js
index 2474333187..d033b3e8c9 100644
--- a/frappe/public/js/frappe/model/indicator.js
+++ b/frappe/public/js/frappe/model/indicator.js
@@ -26,24 +26,12 @@ frappe.get_indicator = function(doc, doctype) {
 	var is_submittable = frappe.model.is_submittable(doctype),
 		workflow_fieldname = frappe.workflow.get_state_fieldname(doctype);
 
-	if(doc.docstatus==3) {
-		return [__("Queued for saving"), "orange", "docstatus,=,3"];
-	}
-
-	if(doc.docstatus==4) {
-		return [__("Queued for submission"), "orange", "docstatus,=,4"];
-	}
-
-	if(doc.docstatus==5) {
-		return [__("Queued for cancellation"), "orange", "docstatus,=,5"];
-	}
-
 	// workflow
 	if(workflow_fieldname) {
 		var value = doc[workflow_fieldname];
 		if(value) {
 			var colour = "";
-			
+
 			if(locals["Workflow State"][value] && locals["Workflow State"][value].style) {
 				var colour = {
 					"Success": "green",
diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js
index ad3e4c2a91..bbfee815b4 100644
--- a/frappe/public/js/frappe/model/model.js
+++ b/frappe/public/js/frappe/model/model.js
@@ -438,9 +438,6 @@ $.extend(frappe.model, {
 				parent_doc[parentfield] = newlist;
 			});
 		}
-
-		if(frappe.ui.toolbar.recent)
-			frappe.ui.toolbar.recent.remove(doctype, docname);
 	},
 
 	get_no_copy_list: function(doctype) {
diff --git a/frappe/public/js/frappe/router.js b/frappe/public/js/frappe/router.js
index 9261ba63b1..e4455e01b8 100644
--- a/frappe/public/js/frappe/router.js
+++ b/frappe/public/js/frappe/router.js
@@ -50,8 +50,13 @@ frappe.route = function() {
 		frappe.views.pageview.show(route[0]);
 	}
 
+
 	if(frappe.route_titles[window.location.hash]) {
 		frappe.utils.set_title(frappe.route_titles[window.location.hash]);
+	} else {
+		setTimeout(function() {
+			frappe.route_titles[frappe.get_route_str()] = frappe._original_title || document.title;
+		}, 1000);
 	}
 }
 
@@ -145,4 +150,5 @@ $(window).on('hashchange', function() {
 		cur_dialog.hide();
 
 	frappe.route();
+
 });
diff --git a/frappe/public/js/frappe/ui/filters/filters.js b/frappe/public/js/frappe/ui/filters/filters.js
index 374be857a2..d592286dd0 100644
--- a/frappe/public/js/frappe/ui/filters/filters.js
+++ b/frappe/public/js/frappe/ui/filters/filters.js
@@ -538,10 +538,7 @@ frappe.ui.Filter = Class.extend({
 			df.options=[
 				{value:0, label:__("Draft")},
 				{value:1, label:__("Submitted")},
-				{value:2, label:__("Cancelled")},
-				{value:3, label:__("Queued for saving")},
-				{value:4, label:__("Queued for submission")},
-				{value:5, label:__("Queued for cancellation")},
+				{value:2, label:__("Cancelled")}
 			]
 		} else if(df.fieldtype=='Check') {
 			df.fieldtype='Select';
diff --git a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js
index 6c2abdf493..5bce9e600a 100644
--- a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js
+++ b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js
@@ -118,27 +118,45 @@ frappe.search = {
 		});
 	},
 	add_recent: function(txt) {
-		var doctypes = frappe.utils.unique(keys(locals).concat(keys(frappe.search.recent)));
-		for(var i in doctypes) {
-			var doctype = doctypes[i];
-			if(doctype[0]!==":" && !frappe.model.is_table(doctype)
-				&& !in_list(frappe.boot.single_types, doctype)
-				&& !in_list(["DocType", "DocField", "DocPerm", "Page", "Country",
-					"Currency", "Page Role", "Print Format", "Report"], doctype)) {
+		values = [];
+		$.each(frappe.search.recent, function(i, doctype) {
+			values.push([doctype[1], ['Form', doctype[0], doctype[1]]]);
+		});
 
-				var values = frappe.utils.remove_nulls(frappe.utils.unique(
-					keys(locals[doctype]).concat(frappe.search.recent[doctype] || [])
-				));
+		values = values.reverse();
 
-				var ret = frappe.search.find(values, txt, function(match) {
-					return {
-						label: __(doctype) + " " + match.bold(),
-						value: __(doctype) + " " + match,
-						route: ["Form", doctype, match]
-					}
-				}, true);
+		$.each(frappe.route_history, function(i, route) {
+			if(route[0]==='Form') {
+				values.push([route[2], route]);
 			}
-		}
+			else if(in_list(['List', 'Report', 'modules'], route[0])) {
+				if(route[1]) {
+					values.push([route[1], route]);
+				}
+			}
+			else if(route[0]) {
+				values.push([frappe.route_titles[route[0]] || route[0], route]);
+			}
+		});
+
+		frappe.search.find(values, txt, function(match) {
+			out = {
+				route: match[1]
+			}
+			if(match[1][0]==='Form') {
+				out.label = __(match[1][1]) + " " + match[1][2].bold();
+				out.value = __(match[1][1]) + " " + match[1][2];
+			} else if(in_list(['List', 'Report', 'modules'], match[1][0])) {
+				var type = match[1][0];
+				if(type==='modules') type = 'Module';
+				out.label = __(match[1][1]).bold() + " " + __(type);
+				out.value = __(match[1][1]) + " " + __(type);
+			} else {
+				out.label = match[0].bold();
+				out.value = match[0];
+			}
+			return out;
+		}, true);
 	},
 	make_page_title_map: function() {
 		frappe.search.pages = {};
@@ -148,21 +166,16 @@ frappe.search = {
 		});
 	},
 	setup_recent: function() {
-		var recent = JSON.parse(frappe.boot.user.recent || "[]") || [];
-		frappe.search.recent = {};
-		for (var i=0, l=recent.length; i < l; i++) {
-			var d = recent[i];
-			if (!(d[0] && d[1])) continue;
-
-			if (!frappe.search.recent[d[0]]) {
-				frappe.search.recent[d[0]] = [];
-			}
-			frappe.search.recent[d[0]].push(d[1]);
-		}
+		frappe.search.recent = JSON.parse(frappe.boot.user.recent || "[]") || [];
 	},
 	find: function(list, txt, process, prepend) {
 		$.each(list, function(i, item) {
-			_item = __(item).toLowerCase().replace(/-/g, " ");
+			if($.isArray(item)) {
+				_item = item[0];
+			} else {
+				_item = item;
+			}
+			_item = __(_item).toLowerCase().replace(/-/g, " ");
 			if(txt===_item || _item.indexOf(txt) !== -1) {
 				var option = process(item);
 
diff --git a/frappe/public/js/frappe/ui/toolbar/navbar.html b/frappe/public/js/frappe/ui/toolbar/navbar.html
index 378dc648ec..95cf1d92b1 100644
--- a/frappe/public/js/frappe/ui/toolbar/navbar.html
+++ b/frappe/public/js/frappe/ui/toolbar/navbar.html
@@ -30,6 +30,8 @@
 					    {%= __("Reload") %}</a></li>
 					<li><a href="/index" target="_blank">
 						{%= __("View Website") %}</a></li>
+					<li><a href="#background_jobs">
+						{%= __("Background Jobs") %}</a></li>
 					<li class="divider"></li>
 					<li><a href="#" onclick="return frappe.app.logout();">
 						{%= __("Logout") %}</a></li>
diff --git a/frappe/public/js/legacy/form.js b/frappe/public/js/legacy/form.js
index d04c703999..3b77efae6d 100644
--- a/frappe/public/js/legacy/form.js
+++ b/frappe/public/js/legacy/form.js
@@ -334,9 +334,6 @@ _f.Frm.prototype.refresh_header = function(is_a_different_doc) {
 		frappe.utils.set_title(this.meta.issingle ? this.doctype : this.docname);
 	}
 
-	if(frappe.ui.toolbar.recent)
-		frappe.ui.toolbar.recent.add(this.doctype, this.docname, 1);
-
 	// show / hide buttons
 	if(this.toolbar) {
 		if (is_a_different_doc) {
diff --git a/frappe/tests/test_document_locks.py b/frappe/tests/test_document_locks.py
new file mode 100644
index 0000000000..c270b1bf28
--- /dev/null
+++ b/frappe/tests/test_document_locks.py
@@ -0,0 +1,18 @@
+# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# MIT License. See license.txt
+from __future__ import unicode_literals
+
+import frappe, unittest
+
+class TestDocumentLocks(unittest.TestCase):
+	def test_locking(self):
+		todo = frappe.get_doc(dict(doctype='ToDo', description='test')).insert()
+		todo_1 = frappe.get_doc('ToDo', todo.name)
+
+		todo.lock()
+		self.assertRaises(frappe.DocumentLockedError, todo_1.lock)
+		todo.unlock()
+
+		todo_1.lock()
+		self.assertRaises(frappe.DocumentLockedError, todo.lock)
+		todo_1.unlock()
diff --git a/frappe/utils/file_lock.py b/frappe/utils/file_lock.py
index eeffb0d069..b85ace7db9 100644
--- a/frappe/utils/file_lock.py
+++ b/frappe/utils/file_lock.py
@@ -3,8 +3,11 @@
 
 from __future__ import unicode_literals
 
+'''
+File based locking utility
+'''
+
 import os
-import frappe
 from time import time
 from frappe.utils import get_site_path, touch_file
 
@@ -12,12 +15,17 @@ class LockTimeoutError(Exception):
     pass
 
 def create_lock(name):
+	'''Creates a file in the /locks folder by the given name'''
 	lock_path = get_lock_path(name)
 	if not check_lock(lock_path):
 		return touch_file(lock_path)
 	else:
 		return False
 
+def lock_exists(name):
+	'''Returns True if lock of the given name exists'''
+	return os.path.exists(get_lock_path(name))
+
 def check_lock(path, timeout=600):
 	if not os.path.exists(path):
 		return False