zeitgeist team mailing list archive

[Markus Korn <thekorn@xxxxxx>]

Markus Korn has proposed merging lp:~thekorn/zeitgeist/fix-598666-remove_cache_entry into lp:zeitgeist.

Requested reviews:
  Zeitgeist Framework Team (zeitgeist)
Related bugs:
  Bug #598666 in Zeitgeist Framework: "Invalid cache access (was: Error when trying to fetch items)"
  https://bugs.launchpad.net/zeitgeist/+bug/598666

For more details, see:
https://code.launchpad.net/~thekorn/zeitgeist/fix-598666-remove_cache_entry/+merge/56710

This branch adds a fix/workaround for bug 598666, where the caches are not updated properly if one cached item gets deleted in the database.
https://bugs.launchpad.net/zeitgeist/+bug/598666/comments/16 describes how this works.
This branch also factors in all review comments by Mikkel and Siegfried made in the bug.
-- 
https://code.launchpad.net/~thekorn/zeitgeist/fix-598666-remove_cache_entry/+merge/56710
Your team Zeitgeist Framework Team is requested to review the proposed merge of lp:~thekorn/zeitgeist/fix-598666-remove_cache_entry into lp:zeitgeist.

=== modified file '_zeitgeist/engine/main.py'
--- _zeitgeist/engine/main.py	2011-04-07 07:23:41 +0000
+++ _zeitgeist/engine/main.py	2011-04-07 07:56:30 +0000
@@ -5,7 +5,7 @@
 # Copyright © 2009 Mikkel Kamstrup Erlandsen <mikkel.kamstrup@xxxxxxxxx>
 # Copyright © 2009-2010 Siegfried-Angel Gevatter Pujals <rainct@xxxxxxxxxx>
 # Copyright © 2009-2011 Seif Lotfy <seif@xxxxxxxxx>
-# Copyright © 2009 Markus Korn <thekorn@xxxxxxx>
+# Copyright © 2009-2011 Markus Korn <thekorn@xxxxxxx>
 # Copyright © 2011 Collabora Ltd.
 # 		      By Seif Lotfy <seif@xxxxxxxxx>
 #
@@ -133,6 +133,11 @@
 	def close(self):
 		log.debug("Closing down the zeitgeist engine...")
 		self.extensions.unload()
+		# make sure all symbol table are in good shape
+		# this Exception should never be raised, it would indicate a bug
+		# in the workaround for (LP: #598666)
+		if self._cursor.execute("SELECT * FROM _fix_cache").fetchone():
+			raise RuntimeError("Symbol cache is in bad state")
 		self._cursor.connection.close()
 		self._cursor = None
 		unset_cursor()
@@ -732,7 +737,16 @@
 				% ",".join(["?"] * len(ids)), ids)
 			self._cursor.connection.commit()
 			log.debug("Deleted %s" % map(int, ids))
-			
+			clean_fix_cache = False
+			for cached_table, cached_id in self._cursor.execute("SELECT * FROM _fix_cache"):
+				if cached_table not in ("interpretation", "manifestation", "mimetype", "actor"):
+					raise ValueError("Unable to fix cache for '%s'" %cached_table)
+				clean_fix_cache = True
+				getattr(self, "_%s" %cached_table).remove_id(cached_id)
+			if clean_fix_cache:
+				# delete all rows from _fix_cache, as all caches are fixed by now
+				self._cursor.execute("DELETE FROM _fix_cache")
+				self._cursor.connection.commit()
 			self.extensions.apply_post_delete(ids, sender)
 			return timestamps
 		else:

=== modified file '_zeitgeist/engine/sql.py'
--- _zeitgeist/engine/sql.py	2011-03-25 18:30:46 +0000
+++ _zeitgeist/engine/sql.py	2011-04-07 07:56:30 +0000
@@ -4,7 +4,7 @@
 #
 # Copyright © 2009-2010 Siegfried-Angel Gevatter Pujals <rainct@xxxxxxxxxx>
 # Copyright © 2009 Mikkel Kamstrup Erlandsen <mikkel.kamstrup@xxxxxxxxx>
-# Copyright © 2009 Markus Korn <thekorn@xxxxxxx>
+# Copyright © 2009-2011 Markus Korn <thekorn@xxxxxxx>
 # Copyright © 2009 Seif Lotfy <seif@xxxxxxxxx>
 #
 # This program is free software: you can redistribute it and/or modify
@@ -166,6 +166,11 @@
 	# one connection to the database is allowed to revert this setting set locking_mode to NORMAL.
 	cursor.execute("PRAGMA locking_mode = EXCLUSIVE")
 	
+	# as part of the workaround for (LP: #598666) we need to
+	# create the '_fix_cache' TEMP table on every start,
+	# this table gets purged once the engine gets closed.
+	cursor.execute("CREATE TEMP TABLE _fix_cache (t VARCHAR, id INTEGER)")
+	
 	# Always assume that temporary memory backed DBs have good schemas
 	if constants.DATABASE_FILE != ":memory:":
 		if not new_database and _check_core_schema_upgrade(cursor):
@@ -453,6 +458,14 @@
 			self[row["value"]] = row["id"]
 		
 		self._inv_dict = dict((value, key) for key, value in self.iteritems())
+		
+		cursor.execute("""
+			CREATE TEMP TRIGGER update_cache_%(table)s
+			BEFORE DELETE ON %(table)s
+			BEGIN
+				INSERT INTO _fix_cache VALUES ("%(table)s", OLD.id);
+			END;
+			""" % {"table": table})
 	
 	def __getitem__(self, name):
 		# Use this for inserting new properties into the database
@@ -483,6 +496,11 @@
 		# database already. Eg., in find_eventids.
 		return super(TableLookup, self).__getitem__(name)
 		
+	def remove_id(self, id):
+		value = self.value(id)
+		del self._inv_dict[id]
+		del self[value]
+		
 def get_right_boundary(text):
 	""" returns the smallest string which is greater than `text` """
 	if not text: