From 814de5f0941da1da4e983634e0d8c3059bff033c Mon Sep 17 00:00:00 2001
From: Tim Van Baak <tim.vanbaak@gmail.com>
Date: Tue, 11 Aug 2020 21:56:38 -0700
Subject: [PATCH] Add exceptions to item triggers

---
 inquisitor/loader.py           |  8 ++++++++
 inquisitor/sources.py          | 27 +++++++++++++++++++--------
 sources/deletetriggerexdemo.py | 29 +++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 8 deletions(-)
 create mode 100644 sources/deletetriggerexdemo.py

diff --git a/inquisitor/loader.py b/inquisitor/loader.py
index 3c22d0e..e475851 100644
--- a/inquisitor/loader.py
+++ b/inquisitor/loader.py
@@ -128,6 +128,14 @@ def new_item(source_name, item):
 	return WritethroughDict.create(item_path, item)
 
 
+def delete_item(source_name, item_id):
+	"""
+	Delete an item.
+	"""
+	item_path = os.path.join(DUNGEON_PATH, source_name, f'{item_id}.item')
+	os.remove(item_path)
+
+
 def load_items(source_name):
 	"""
 	Returns a map of ids to items and a list of unreadable files.
diff --git a/inquisitor/sources.py b/inquisitor/sources.py
index 7d6af86..1d980ec 100644
--- a/inquisitor/sources.py
+++ b/inquisitor/sources.py
@@ -135,7 +135,14 @@ def update_source(source_name, source):
 		item_source = item.get('source', source_name)
 		created_item = loader.new_item(item_source, item)
 		if has_create_handler:
-			source.on_create(state, created_item)
+			# Because some sources do not return items more than once,
+			# exceptions in the on-create handler must be squashed.
+			try:
+				source.on_create(state, created_item)
+			except:
+				error.as_item(
+					f'Exception in {source_name}.on_create',
+					traceback.format_exc())
 
 	# Update the other items using the fetched items' values.
 	for new_item in updated_items:
@@ -177,14 +184,16 @@ def update_source(source_name, source):
 				remove = True
 		# Items to be removed are deleted
 		if remove:
-			if has_delete_handler:
-				source.on_delete(state, item)
-			del_count += 1
-			file_path = os.path.join(DUNGEON_PATH, item['source'], item['id'] + ".item")
 			try:
-				os.remove(file_path)
+				if has_delete_handler:
+					# Run the delete handler so exceptions prevent deletions
+					source.on_delete(state, item)
+				loader.delete_item(source_name, item['id'])
+				del_count += 1
 			except:
-				error.as_item("Failed to delete {}".format(file_path))
+				error.as_item(
+					f'Failed to delete {source_name}/{item["id"]}',
+					traceback.format_exc())
 
 	# Note update timestamp in state
 	state['last_updated'] = timestamp.now()
@@ -210,4 +219,6 @@ def item_callback(source_name, itemid):
 		item.flush()
 		state.flush()
 	except Exception:
-		error.as_item(f"Error executing callback for {source_name}/{itemid}", traceback.format_exc())
+		error.as_item(
+			f"Error executing callback for {source_name}/{itemid}",
+			traceback.format_exc())
diff --git a/sources/deletetriggerexdemo.py b/sources/deletetriggerexdemo.py
new file mode 100644
index 0000000..50478a3
--- /dev/null
+++ b/sources/deletetriggerexdemo.py
@@ -0,0 +1,29 @@
+"""
+Demonstrates the behavior of exceptions in create/delete triggers.
+To allow for deletions, it alternates returning a single item and
+returning nothing.
+"""
+# Standard library imports
+from datetime import datetime
+import json
+import random
+
+def fetch_new(state):
+	if state.get('return_item'):
+		state['return_item'] = False
+		return [{
+			'source': 'deletetriggerexdemo',
+			'id': 'deletetriggerexdemoitem',
+			'title': 'Delete trigger exception demo'
+		}]
+	else:
+		state['return_item'] = True
+		return []
+
+
+def on_create(state, item):
+	raise Exception('on_create')
+
+
+def on_delete(state, item):
+	raise Exception('on_delete')