Merge lp:~zeitgeist/zeitgeist/fix-592599-subject-AND into lp:zeitgeist/0.1

[Markus Korn]

Changing the connector of subjects in a query template from OR to AND

This branch changes the connector of subjects in a template from "OR" to "AND", so a template like

 (*event_metadata, [subject1, subject2])

does not expand to

 *event_metadata AND (subject1 or subject2)

anymore, but now it's

 *event_metadata AND subject1 AND subject2

(the connector for the metadata of an event is still AND)

[Mikkel Kamstrup Erlandsen (kamstrup)]

Looks good and all the tests are passing on my end. Good work Markus!

review: Approve (diff and run unit tests)

=== modified file '_zeitgeist/engine/main.py'
--- _zeitgeist/engine/main.py	2010-07-01 08:04:55 +0000
+++ _zeitgeist/engine/main.py	2010-07-26 09:02:01 +0000
@@ -214,11 +214,18 @@
 	
 		where_or = WhereClause(WhereClause.OR)
 		
-		for (event_template, subject_template) in self._build_templates(templates):
+		for template in templates:
+			event_template = Event((template[0], [], None))
+			if template[1]:
+				subject_templates = [Subject(data) for data in template[1]]
+			else:
+				subject_templates = None
+			# first of all we need to check if the query is supported at all
 			# we do not support searching by storage field for now
 			# see LP: #580364
-			if subject_template[Subject.Storage]:
-				raise ValueError("zeitgeist does not support searching by 'storage' field")
+			if subject_templates is not None:
+				if any(data[Subject.Storage] for data in subject_templates):
+					raise ValueError("zeitgeist does not support searching by 'storage' field")
 			
 			subwhere = WhereClause(WhereClause.AND)
 			
@@ -246,50 +253,51 @@
 				if event_manif_where:
 					subwhere.extend(event_manif_where)
 				
-				value, negation, wildcard = parse_operators(Subject, Subject.Interpretation, subject_template.interpretation)
-				# Expand subject interpretation children
-				su_interp_where = WhereClause(WhereClause.OR, negation)
-				for child_interp in (Symbol.find_child_uris_extended(value)):
-					if child_interp:
-						su_interp_where.add_text_condition("subj_interpretation",
-						                    child_interp, like=wildcard, cache=self._interpretation)
-				if su_interp_where:
-					subwhere.extend(su_interp_where)
-				
-				value, negation, wildcard = parse_operators(Subject, Subject.Manifestation, subject_template.manifestation)
-				# Expand subject manifestation children
-				su_manif_where = WhereClause(WhereClause.OR, negation)
-				for child_manif in (Symbol.find_child_uris_extended(value)):
-					if child_manif:
-						su_manif_where.add_text_condition("subj_manifestation",
-						                   child_manif, like=wildcard, cache=self._manifestation)
-				if su_manif_where:
-					subwhere.extend(su_manif_where)
-				
-				# FIXME: Expand mime children as well.
-				# Right now we only do exact matching for mimetypes
-				# thekorn: this will be fixed when wildcards are supported
-				value, negation, wildcard = parse_operators(Subject, Subject.Mimetype, subject_template.mimetype)
-				if value:
-					subwhere.add_text_condition("subj_mimetype",
-					             value, wildcard, negation, cache=self._mimetype)
-				
 				value, negation, wildcard = parse_operators(Event, Event.Actor, event_template.actor)
 				if value:
 					subwhere.add_text_condition("actor", value, wildcard, negation, cache=self._actor)
+				
+				if subject_templates is not None:
+					for subject_template in subject_templates:
+						value, negation, wildcard = parse_operators(Subject, Subject.Interpretation, subject_template.interpretation)
+						# Expand subject interpretation children
+						su_interp_where = WhereClause(WhereClause.OR, negation)
+						for child_interp in (Symbol.find_child_uris_extended(value)):
+							if child_interp:
+								su_interp_where.add_text_condition("subj_interpretation",
+													child_interp, like=wildcard, cache=self._interpretation)
+						if su_interp_where:
+							subwhere.extend(su_interp_where)
+						
+						value, negation, wildcard = parse_operators(Subject, Subject.Manifestation, subject_template.manifestation)
+						# Expand subject manifestation children
+						su_manif_where = WhereClause(WhereClause.OR, negation)
+						for child_manif in (Symbol.find_child_uris_extended(value)):
+							if child_manif:
+								su_manif_where.add_text_condition("subj_manifestation",
+												   child_manif, like=wildcard, cache=self._manifestation)
+						if su_manif_where:
+							subwhere.extend(su_manif_where)
+						
+						# FIXME: Expand mime children as well.
+						# Right now we only do exact matching for mimetypes
+						# thekorn: this will be fixed when wildcards are supported
+						value, negation, wildcard = parse_operators(Subject, Subject.Mimetype, subject_template.mimetype)
+						if value:
+							subwhere.add_text_condition("subj_mimetype",
+										 value, wildcard, negation, cache=self._mimetype)
+				
+						for key in ("uri", "origin", "text"):
+							value = getattr(subject_template, key)
+							if value:
+								value, negation, wildcard = parse_operators(Subject, getattr(Subject, key.title()), value)
+								subwhere.add_text_condition("subj_%s" %key, value, wildcard, negation)
 			except KeyError, e:
 				# Value not in DB
 				log.debug("Unknown entity in query: %s" % e)
 				where_or.register_no_result()
 				continue
-				
-			for key in ("uri", "origin", "text"):
-				value = getattr(subject_template, key)
-				if value:
-					value, negation, wildcard = parse_operators(Subject, getattr(Subject, key.title()), value)
-					subwhere.add_text_condition("subj_%s" %key, value, wildcard, negation)
-			where_or.extend(subwhere)
-		
+			where_or.extend(subwhere) 
 		return where_or
 	
 	def _build_sql_event_filter(self, time_range, templates, storage_state):

=== modified file 'test/engine-test.py'
--- test/engine-test.py	2010-07-22 09:19:02 +0000
+++ test/engine-test.py	2010-07-26 09:02:01 +0000
@@ -402,7 +402,18 @@
 			StorageState.Any,
 			100,
 			0,)
-		self.assertEquals(2, len(result))
+		self.assertEquals(0, len(result)) # no subject with two different
+										  # interpretations at the same time
+		subj = Subject.new_for_values(uri="file:///tmp/foo.txt")
+		subj1 = Subject.new_for_values(interpretation="stfu:Image")
+		event_template = Event.new_for_values(subjects=[subj, subj1])
+		result = self.engine.find_eventids(
+			(0, 200),
+			[event_template, ],
+			StorageState.Any,
+			100,
+			0,)
+		self.assertEquals(1, len(result))		
 	
 	def testJsonImport(self):
 		import_events("test/data/single_event.js", self.engine)

=== modified file 'test/remote-test.py'
--- test/remote-test.py	2010-07-14 13:16:59 +0000
+++ test/remote-test.py	2010-07-26 09:02:01 +0000
@@ -87,21 +87,19 @@
 			self.assertEquals("Boogaloo", event.actor)
 		
 		# Search for everything
-		import dbus
-		ids = self.findEventIdsAndWait([], num_events=3) # dbus.Array(signature="(asaasay)")
+		ids = self.findEventIdsAndWait([], num_events=3)
 		self.assertEquals(3, len(ids)) # (we can not trust the ids because we don't have a clean test environment)
 		
 		# Search for some specific templates
-		subj_templ1 = Subject.new_for_values(uri="foo://bar")
-		subj_templ2 = Subject.new_for_values(uri="foo://baz")
+		subj_templ1 = Subject.new_for_values(manifestation=Manifestation.FILE_DATA_OBJECT)
+		subj_templ2 = Subject.new_for_values(interpretation=Interpretation.IMAGE)
 		event_template = Event.new_for_values(
 					actor="Boogaloo",
 					interpretation=Interpretation.ACCESS_EVENT,
 					subjects=[subj_templ1,subj_templ2])
 		ids = self.findEventIdsAndWait([event_template],
 						num_events=10)
-		logging.debug("RESULTS %s" %map(int, ids))
-		self.assertEquals(2, len(ids))
+		self.assertEquals(1, len(ids))
 		
 	def testUnicodeInsert(self):
 		events = parse_events("test/data/unicode_event.js")