[evolution-patches] Re: [Evolution-hackers] mark junk/not junk problem

[Radek Doulík <rodo ximian com>]

On Thu, 2004-03-11 at 13:31 +0100, Radek Doulík wrote:

> On Thu, 2004-03-11 at 18:04 +0800, Not Zed wrote:
> > > > Anyway I had a thought while i couldn't get to sleep last night, which
> > > > should hopefully be acceptable to you, because it:
> > > >  - simplifies the api to simply setting or unsetting bits on the
> > > > messageinfo
> > > >  - applies the learner asynchronously, but as soon as possible
> > > 
> > > that sounds good
> > > 
> > > > In camel-folder.c:folder_changed
> > > >  - add another bit to camelmessageinfo for 'learned' stuff,
> > > >  - add a check for changed uid's that change the junk status
> > > >  - add these uid's to the existing filter_folder structure and fire off
> > > > the existing filter folder code.  you probably need 2 lists, one for
> > > > stuff to mark as junk and one for stuff to mark as not junk
> > > > In camel-folder.c:filter_filter
> > > >  - if there are junk messages, set/unset the junk status
> > > >  - if there is anything to filter, filter it
> > > > In camel-folder.c:camel_folder_set_message_flags
> > > >  - if you're setting or unsetting the junk status, and the learned bit
> > > > isn't being set too, then clear the learned bit.
> > > 
> > > I don't understand parts of it so I will look at the code and ask you on
> > > irc. I didn't sleep very well either. It's a pity we didn't solve it
> > 
> > It should be pretty obvious to see from the code once you get in there.
> > Its essentially but not quite exactly the same way imap filtering works.
> > And there's only a couple of places you need to add any code.  And you
> > can get rid of all the stuff in mail-ops.c
> > 
> > I wont be on tonight, jeff should be able to help if required, but it
> > shouldn't be needed.
> 
> OK. I will try to cook the patch and send it to review before
> committing.

Patches to camel and mail directory attached.

Radek

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/camel/ChangeLog,v
retrieving revision 1.2033
diff -u -p -r1.2033 ChangeLog
--- ChangeLog	11 Mar 2004 07:47:36 -0000	1.2033
+++ ChangeLog	11 Mar 2004 21:05:02 -0000
@@ -1,3 +1,24 @@
+2004-03-11  Radek Doulik  <rodo ximian com>
+
+	* camel-folder.c (camel_folder_set_message_flags): watch for
+	setting JUNK flag, if JUNK_LEARN is not set as well then reset
+	JUNK_LEARN bit
+	(folder_changed): look for junk changes in uid_changed's messages,
+	if these changes request junk filter learning
+	(CAMEL_MESSAGE_JUNK_LEARN bit set) then prepare junk and nonjunk
+	uid arrays, clear CAMEL_MESSAGE_JUNK_LEARN bit so that we don't
+	process it again
+	(folder_changed): start filter thread if there's junk and/or
+	nonjunk arrays
+	(filter_filter): if junk/nonjunk arrays are non-NULL, call junk
+	filter report to learn junk/non-junk messages
+	(filter_free): free junk/nonjunk uids and arrays
+
+	* camel-folder-summary.h: added CAMEL_MESSAGE_JUNK_LEARN to
+	CamelMessageFlags, used when setting CAMEL_MESSAGE_JUNK flag to
+	say that we request junk plugin to learn that message as
+	junk/non-junk
+
 2004-03-11  Not Zed  <NotZed Ximian com>
 
 	* providers/imap/camel-imap-store.c (no_such_folder): removed
Index: camel-folder-summary.h
===================================================================
RCS file: /cvs/gnome/evolution/camel/camel-folder-summary.h,v
retrieving revision 1.72
diff -u -p -r1.72 camel-folder-summary.h
--- camel-folder-summary.h	25 Feb 2004 03:47:02 -0000	1.72
+++ camel-folder-summary.h	11 Mar 2004 21:05:02 -0000
@@ -70,6 +70,9 @@ enum _CamelMessageFlags {
 
 	/* following flags are for the folder, and are not really permanent flags */
 	CAMEL_MESSAGE_FOLDER_FLAGGED = 1<<16, /* for use by the folder implementation */
+	CAMEL_MESSAGE_JUNK_LEARN = 1<<17, /* used when setting CAMEL_MESSAGE_JUNK flag
+					     to say that we request junk plugin
+					     to learn that message as junk/non junk */
 	CAMEL_MESSAGE_USER = 1<<31 /* supports user flags */
 };
 
Index: camel-folder.c
===================================================================
RCS file: /cvs/gnome/evolution/camel/camel-folder.c,v
retrieving revision 1.186
diff -u -p -r1.186 camel-folder.c
--- camel-folder.c	3 Mar 2004 06:36:35 -0000	1.186
+++ camel-folder.c	11 Mar 2004 21:05:03 -0000
@@ -43,7 +43,7 @@
 #include "camel-vtrash-folder.h"
 #include "filter/filter-rule.h"
 
-#define d(x) 
+#define d(x)
 #define w(x)
 
 static CamelObjectClass *parent_class = NULL;
@@ -724,6 +724,11 @@ camel_folder_set_message_flags(CamelFold
 {
 	g_return_val_if_fail(CAMEL_IS_FOLDER(folder), FALSE);
 
+	if ((flags & CAMEL_MESSAGE_JUNK) && !(flags && CAMEL_MESSAGE_JUNK_LEARN)) {
+		flags |= CAMEL_MESSAGE_JUNK_LEARN;
+		set &= ~CAMEL_MESSAGE_JUNK_LEARN;
+	}
+
 	return CF_CLASS(folder)->set_message_flags(folder, uid, flags, set);
 }
 
@@ -1553,6 +1558,8 @@ struct _folder_filter_msg {
 	CamelSessionThreadMsg msg;
 
 	GPtrArray *recents;
+	GPtrArray *junk;
+	GPtrArray *nonjunk;
 	CamelFolder *folder;
 	CamelFilterDriver *driver;
 	CamelException ex;
@@ -1568,46 +1575,79 @@ filter_filter(CamelSession *session, Cam
 	char *source_url;
 	CamelException ex;
 
-	camel_operation_start(NULL, _("Filtering new message(s)"));
+	if (m->junk || m->nonjunk) {
+		CamelJunkPlugin *csp = ((CamelService *)m->folder->parent_store)->session->junk_plugin;
 
-	source_url = camel_service_get_url((CamelService *)m->folder->parent_store);
-	uri = camel_url_new(source_url, NULL);
-	g_free(source_url);
-	if (m->folder->full_name && m->folder->full_name[0] != '/') {
-		char *tmp = alloca(strlen(m->folder->full_name)+2);
+		camel_operation_start (NULL, _("Learning junk and/or non junk message(s)"));
 
-		sprintf(tmp, "/%s", m->folder->full_name);
-		camel_url_set_path(uri, tmp);
-	} else
-		camel_url_set_path(uri, m->folder->full_name);
-	source_url = camel_url_to_string(uri, CAMEL_URL_HIDE_ALL);
-	camel_url_free(uri);
-
-	for (i=0;status == 0 && i<m->recents->len;i++) {
-		char *uid = m->recents->pdata[i];
-		int pc = 100 * i / m->recents->len;
-
-		camel_operation_progress(NULL, pc);
-
-		info = camel_folder_get_message_info(m->folder, uid);
-		if (info == NULL) {
-			g_warning("uid %s vanished from folder: %s", uid, source_url);
-			continue;
+		if (m->junk) {
+			for (i = 0; i < m->junk->len; i ++) {
+				CamelMimeMessage *msg = camel_folder_get_message(m->folder, m->junk->pdata[i], NULL);
+
+				if (msg) {
+					camel_junk_plugin_report_junk (csp, msg);
+					camel_object_unref (msg);
+				}
+			}
+		}
+		if (m->nonjunk) {
+			for (i = 0; i < m->nonjunk->len; i ++) {
+				CamelMimeMessage *msg = camel_folder_get_message(m->folder, m->nonjunk->pdata[i], NULL);
+
+				if (msg) {
+					camel_junk_plugin_report_notjunk (csp, msg);
+					camel_object_unref (msg);
+				}
+			}
 		}
 
-		status = camel_filter_driver_filter_message(m->driver, NULL, info, uid, m->folder, source_url, source_url, &m->ex);
+		camel_junk_plugin_commit_reports (csp);
 
-		camel_folder_free_message_info(m->folder, info);
+		camel_operation_end (NULL);
 	}
 
-	camel_exception_init(&ex);
-	camel_filter_driver_flush(m->driver, &ex);
-	if (!camel_exception_is_set(&m->ex))
-		camel_exception_xfer(&m->ex, &ex);
+	if (m->driver && m->recents) {
+		camel_operation_start(NULL, _("Filtering new message(s)"));
 
-	g_free(source_url);
+		source_url = camel_service_get_url((CamelService *)m->folder->parent_store);
+		uri = camel_url_new(source_url, NULL);
+		g_free(source_url);
+		if (m->folder->full_name && m->folder->full_name[0] != '/') {
+			char *tmp = alloca(strlen(m->folder->full_name)+2);
 
-	camel_operation_end(NULL);
+			sprintf(tmp, "/%s", m->folder->full_name);
+			camel_url_set_path(uri, tmp);
+		} else
+			camel_url_set_path(uri, m->folder->full_name);
+		source_url = camel_url_to_string(uri, CAMEL_URL_HIDE_ALL);
+		camel_url_free(uri);
+
+		for (i=0;status == 0 && i<m->recents->len;i++) {
+			char *uid = m->recents->pdata[i];
+			int pc = 100 * i / m->recents->len;
+
+			camel_operation_progress(NULL, pc);
+
+			info = camel_folder_get_message_info(m->folder, uid);
+			if (info == NULL) {
+				g_warning("uid %s vanished from folder: %s", uid, source_url);
+				continue;
+			}
+
+			status = camel_filter_driver_filter_message(m->driver, NULL, info, uid, m->folder, source_url, source_url, &m->ex);
+
+			camel_folder_free_message_info(m->folder, info);
+		}
+
+		camel_exception_init(&ex);
+		camel_filter_driver_flush(m->driver, &ex);
+		if (!camel_exception_is_set(&m->ex))
+			camel_exception_xfer(&m->ex, &ex);
+
+		g_free(source_url);
+
+		camel_operation_end(NULL);
+	}
 }
 
 static void
@@ -1618,10 +1658,23 @@ filter_free(CamelSession *session, Camel
 
 	camel_folder_thaw(m->folder);
 	camel_object_unref((CamelObject *)m->folder);
-	camel_object_unref((CamelObject *)m->driver);
-	for (i=0;i<m->recents->len;i++)
-		g_free(m->recents->pdata[i]);
-	g_ptr_array_free(m->recents, TRUE);
+	if (m->driver)
+		camel_object_unref((CamelObject *)m->driver);
+	if (m->recents) {
+		for (i=0;i<m->recents->len;i++)
+			g_free(m->recents->pdata[i]);
+		g_ptr_array_free(m->recents, TRUE);
+	}
+	if (m->junk) {
+		for (i=0;i<m->junk->len;i++)
+			g_free(m->junk->pdata[i]);
+		g_ptr_array_free(m->junk, TRUE);
+	}
+	if (m->nonjunk) {
+		for (i=0;i<m->nonjunk->len;i++)
+			g_free(m->nonjunk->pdata[i]);
+		g_ptr_array_free(m->nonjunk, TRUE);
+	}
 }
 
 static CamelSessionThreadOps filter_ops = {
@@ -1646,6 +1699,34 @@ folder_changed (CamelObject *obj, gpoint
 	if (changed != NULL) {
 		CamelSession *session = ((CamelService *)folder->parent_store)->session;
 		CamelFilterDriver *driver = NULL;
+		GPtrArray *junk = NULL;
+		GPtrArray *nonjunk = NULL;
+		GPtrArray *recents = NULL;
+
+		if (changed->uid_changed->len) {
+			int i;
+			guint32 flags;
+
+			for (i = 0; i < changed->uid_changed->len; i ++) {
+				flags = camel_folder_get_message_flags (folder, changed->uid_changed->pdata [i]);
+				if (flags & CAMEL_MESSAGE_JUNK_LEARN) {
+					if (flags & CAMEL_MESSAGE_JUNK) {
+						if (!junk)
+							junk = g_ptr_array_new();
+						g_ptr_array_add (junk, g_strdup (changed->uid_changed->pdata [i]));
+					} else {
+						if (!nonjunk)
+							nonjunk = g_ptr_array_new();
+						g_ptr_array_add (nonjunk, g_strdup (changed->uid_changed->pdata [i]));
+					}
+				}
+
+				/* reset junk learn flag so that we don't process it again */
+				camel_folder_set_message_flags (folder, changed->uid_changed->pdata [i], CAMEL_MESSAGE_JUNK_LEARN, 0);
+			}
+			d(if (junk || nonjunk) printf("** Have '%d' messages for junk filter to learn, launching thread to process them\n",
+						     (junk ? junk->len : 0) + (nonjunk ? nonjunk->len : 0)));
+		}
 
 		if ((folder->folder_flags & (CAMEL_FOLDER_FILTER_RECENT|CAMEL_FOLDER_FILTER_JUNK))
 		    && changed->uid_recent->len > 0)
@@ -1655,17 +1736,23 @@ folder_changed (CamelObject *obj, gpoint
 		CAMEL_FOLDER_LOCK(folder, change_lock);
 
 		if (driver) {
-			GPtrArray *recents = g_ptr_array_new();
+			recents = g_ptr_array_new();
 			int i;
-			struct _folder_filter_msg *msg;
 			
+			for (i=0;i<changed->uid_recent->len;i++)
+				g_ptr_array_add(recents, g_strdup(changed->uid_recent->pdata[i]));
+
 			d(printf("** Have '%d' recent messages, launching thread to process them\n", changed->uid_recent->len));
+		}
+
+		if (driver || junk || nonjunk) {
+			struct _folder_filter_msg *msg;
 
 			folder->priv->frozen++;
 			msg = camel_session_thread_msg_new(session, &filter_ops, sizeof(*msg));
-			for (i=0;i<changed->uid_recent->len;i++)
-				g_ptr_array_add(recents, g_strdup(changed->uid_recent->pdata[i]));
 			msg->recents = recents;
+			msg->junk = junk;
+			msg->nonjunk = nonjunk;
 			msg->folder = folder;
 			camel_object_ref((CamelObject *)folder);
 			msg->driver = driver;

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
retrieving revision 1.3160
diff -u -p -r1.3160 ChangeLog
--- ChangeLog	11 Mar 2004 16:14:22 -0000	1.3160
+++ ChangeLog	11 Mar 2004 21:09:10 -0000
@@ -1,3 +1,12 @@
+2004-03-11  Radek Doulik  <rodo ximian com>
+
+	* mail-ops.c: remove junk learning thread, it's handled in
+	camel-folder.c:folder_changed now
+
+	* em-folder-view.c (emfv_popup_mark_junk): use
+	em_folder_view_mark_selected, set junk, junk-learn and seen flags
+	(emfv_popup_mark_nojunk): similar, reset junk, set junk-learn flags
+
 2004-03-11  Jeffrey Stedfast  <fejj ximian com>
 
 	* em-format-html.c (efh_format_header): Fixed a bug where txt
Index: em-folder-view.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/em-folder-view.c,v
retrieving revision 1.41
diff -u -p -r1.41 em-folder-view.c
--- em-folder-view.c	8 Mar 2004 17:53:35 -0000	1.41
+++ em-folder-view.c	11 Mar 2004 21:09:12 -0000
@@ -560,37 +560,16 @@ emfv_popup_mark_unimportant(GtkWidget *w
 static void
 emfv_popup_mark_junk (GtkWidget *w, EMFolderView *emfv)
 {
-	GPtrArray *uids, *uidsjunk;
-	int i;
-
-	if (emfv->folder == NULL)
-		return;
+	GPtrArray *uids;
 	
-	uidsjunk = g_ptr_array_new();
 	uids = message_list_get_selected(emfv->list);
-	camel_folder_freeze(emfv->folder);
-
-	for (i=0; i<uids->len; i++) {
-		char *uid = uids->pdata[i];
-		
-		if (camel_folder_set_message_flags(emfv->folder, uid,
-						   CAMEL_MESSAGE_SEEN|CAMEL_MESSAGE_JUNK,
-						   CAMEL_MESSAGE_SEEN|CAMEL_MESSAGE_JUNK)) {
-			g_ptr_array_add(uidsjunk, g_strdup(uid));
-		}
-	}
-
-	camel_folder_thaw(emfv->folder);
-
+	em_folder_view_mark_selected(emfv,
+				     CAMEL_MESSAGE_SEEN|CAMEL_MESSAGE_JUNK|CAMEL_MESSAGE_JUNK_LEARN,
+				     CAMEL_MESSAGE_SEEN|CAMEL_MESSAGE_JUNK|CAMEL_MESSAGE_JUNK_LEARN);
 	if (uids->len == 1)
 		message_list_select(emfv->list, MESSAGE_LIST_SELECT_NEXT, 0, 0, FALSE);
 
 	message_list_free_uids(emfv->list, uids);
-
-	if (uidsjunk->len)
-		mail_mark_junk(emfv->folder, uidsjunk, TRUE);
-	else
-		em_utils_uids_free(uidsjunk);
 }
 
 static void
@@ -599,10 +578,13 @@ emfv_popup_mark_nojunk (GtkWidget *w, EM
 	GPtrArray *uids;
 	
 	uids = message_list_get_selected(emfv->list);
-	em_folder_view_mark_selected(emfv, CAMEL_MESSAGE_JUNK, 0);
+	em_folder_view_mark_selected(emfv,
+				     CAMEL_MESSAGE_JUNK|CAMEL_MESSAGE_JUNK_LEARN,
+				     CAMEL_MESSAGE_JUNK_LEARN);
 	if (uids->len == 1)
 		message_list_select(emfv->list, MESSAGE_LIST_SELECT_NEXT, 0, 0, FALSE);
-	mail_mark_junk(emfv->folder, uids, FALSE);
+
+	message_list_free_uids(emfv->list, uids);
 }
 
 static void
Index: mail-ops.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-ops.c,v
retrieving revision 1.421
diff -u -p -r1.421 mail-ops.c
--- mail-ops.c	11 Mar 2004 03:05:43 -0000	1.421
+++ mail-ops.c	11 Mar 2004 21:09:13 -0000
@@ -2396,88 +2396,3 @@ mail_execute_shell_command (CamelFilterD
 	
 	gnome_execute_async_fds (NULL, argc, argv, TRUE);
 }
-
-/* [Un]mark junk flag */
-
-struct _mark_junk_mail_msg {
-	struct _mail_msg msg;
-	
-	CamelFolder *folder;
-	GPtrArray *uids;
-	gboolean junk;
-};
-
-static char *
-mark_junk_describe (struct _mail_msg *mm, int complete)
-{
-	return g_strdup (_("Changing junk status"));
-}
-
-static void
-mark_junk_mark (struct _mail_msg *mm)
-{
-	struct _mark_junk_mail_msg *m = (struct _mark_junk_mail_msg *) mm;
-	CamelJunkPlugin *csp = ((CamelService *)m->folder->parent_store)->session->junk_plugin;
-	gboolean commit_reports = FALSE;
-	void (*report)(CamelJunkPlugin *, CamelMimeMessage *);
-	int i;
-
-	if (csp == NULL)
-		return;
-
-	/* FIXME: This should probably be implictly handled by the
-	   folder when you apply the junk bit, e.g. at sync time. */
-
-	if (m->junk)
-		report = camel_junk_plugin_report_junk;
-	else
-		report = camel_junk_plugin_report_notjunk;
-
-	for (i=0; i<m->uids->len; i++) {
-		CamelMimeMessage *msg = camel_folder_get_message(m->folder, m->uids->pdata[i], NULL);
-
-		if (msg) {
-			report(csp, msg);
-			commit_reports = TRUE;
-			camel_object_unref(msg);
-		}
-	}
-
-	if (commit_reports)
-		camel_junk_plugin_commit_reports(csp);
-}
-
-static void
-mark_junk_marked (struct _mail_msg *mm)
-{
-}
-
-static void
-mark_junk_free (struct _mail_msg *mm)
-{
-	struct _mark_junk_mail_msg *m = (struct _mark_junk_mail_msg *)mm;
-	
-	camel_object_unref(m->folder);
-	em_utils_uids_free(m->uids);
-}
-
-static struct _mail_msg_op mark_junk_op = {
-	mark_junk_describe,
-	mark_junk_mark,
-	mark_junk_marked,
-	mark_junk_free,
-};
-
-void
-mail_mark_junk(CamelFolder *folder, GPtrArray *uids, gboolean junk)
-{
-	struct _mark_junk_mail_msg *m;
-	
-	m = mail_msg_new(&mark_junk_op, NULL, sizeof (*m));
-	m->folder = folder;
-	camel_object_ref(folder);
-	m->uids = uids;
-	m->junk = junk;
-	
-	e_thread_put(mail_thread_queued, (EMsg *) m);
-}
Index: mail-ops.h
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-ops.h,v
retrieving revision 1.67
diff -u -p -r1.67 mail-ops.h
--- mail-ops.h	8 Mar 2004 17:53:37 -0000	1.67
+++ mail-ops.h	11 Mar 2004 21:09:13 -0000
@@ -159,8 +159,6 @@ int mail_store_set_offline(CamelStore *s
 /* filter driver execute shell command async callback */
 void mail_execute_shell_command (CamelFilterDriver *driver, int argc, char **argv, void *data);
 
-void mail_mark_junk(CamelFolder *folder, GPtrArray *uids, int junk);
-
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */