Re: filters

[Emmanuel <e allaud wanadoo fr>]

On 22.05.2002 05:39 Tim O'Brien wrote:
> Hi,
> Just started using balsa 1.3.6 Seems like the filters are not working 
> very well,i created a couple of mailboxs for mailing lists, and set a 
> filter on my inbox, to move messages with a specific TO: field to a 
> mailing list mailbox on reception. Sometimes they move, sometimes
> they dont, sometimes they move but leave deleted messages in my
> inbox. So i go and do a reapply filters on my inbox, and the mailing list
> mail boxes get filled up with duplicate messages... (that were not in the
> inbox in the first place)
> 
> Is this  a bug or am using the program wrongly?
> 

It's both ;-)

The deleted messages you see could be a bug or perhaps you have not 
checked the pref saying that you want to hide deleted messages (I don't 
remember where it is exactly). Could you tell me if it's the case?

Please tell me which type of mailbox is your inbox : pop3 or local 
(mbox,mh or maildir), it could help.
Anyway I know that the code for filtering on reception is still not robust 
enough. The problem is bad interactions with filters and the mailbox code.
I've sent a patch to Pawel which improves the locking of mailbox during 
filtering, this could fix what you're seeing. BTW Pawel had you time to 
review it ?
But if you don't mind trying to patch your tree perhaps you could try it 
(attached file) and tell me if it improves things or not.
Bye
Manu
--- /home/manu/prog/balsa-cvs/balsa/libbalsa/filter.h	Thu May 16 06:34:55 2002
+++ balsa/libbalsa/filter.h	Fri May 17 19:46:29 2002
@@ -178,13 +178,27 @@
 
 gint filters_prepare_to_run(GSList * filters);
 
-/* filters_run_on_messages run all filters on the list of messages
+/* libbalsa_filter_match run all filters on the list of messages
+   each filter is stuffed with the list of its matching messages
+   you must call libbalsa_filter_apply after to make the filters
+   act on their matching messages (this split is needed for proper
+   locking)
+ */
+
+void libbalsa_filter_match(GSList * filter_list, GList * messages);
+
+/* Same but on mailbox, convenience function that locks the mailbox
+   before calling libbalsa_filter_match */
+
+void libbalsa_filter_match_mailbox(GSList * filter_list, LibBalsaMailbox * mbox);
+
+/* libbalsa_filter_apply will let all filters to apply on their
+ * matching messages (you must call libbalsa_filters_match before)
  * It returns TRUE if the trash bin has been filled with something
  * this is used to call enable_empty_trash after
- * FIXME : No locking is done for now
  */
 
-gboolean filters_run_on_messages(GSList * filter_list, GList * messages);
+gboolean libbalsa_filter_apply(GSList * filter_list);
 
 /* libalsa_extract_new_messages : returns a sublist of the messages list containing all
    "new" messages, ie just retrieved mails
--- /home/manu/prog/balsa-cvs/balsa/libbalsa/filter.c	Thu May 16 06:34:55 2002
+++ balsa/libbalsa/filter.c	Fri May 17 19:47:43 2002
@@ -262,8 +262,6 @@
 
 /*--------- Filtering functions -------------------------------*/
 
-/* FIXME : Add error reporting for each filter */
-
 gint
 filters_prepare_to_run(GSList * filters)
 {
@@ -287,21 +285,18 @@
 /*
  * Run all filters until one matches (so order of filter is important)
  * filters must be valid and compiled (ie filters_prepare_to_run have been called before)
- * Assume that all messages come from ONE mailbox
- * returns TRUE if the trash bin has been filled
- * FIXME : Should position filter_errno on errors (bad command action,bad destination mailbox...)
+ * In general you'll call this function with mailbox lock held (ie you locked the mailbox
+ * you're filtering before calling this function)
  */
 
-gboolean
-filters_run_on_messages(GSList * filter_list, GList * messages)
+void
+libbalsa_filter_match(GSList * filter_list, GList * messages)
 {
     gint match;
     GSList * lst;
-    GList * lst_messages;
     LibBalsaFilter * filt=NULL;
-    gboolean result=FALSE;
 
-    if (!filter_list || ! messages) return FALSE;
+    if (!filter_list || ! messages) return;
 
     for (;messages;messages=g_list_next(messages)) {
 
@@ -316,11 +311,32 @@
 	    filt->matching_messages=g_list_prepend(filt->matching_messages,LIBBALSA_MESSAGE(messages->data));
 	}
     }
+}
+
+void libbalsa_filter_match_mailbox(GSList * filter_list, LibBalsaMailbox * mbox)
+{
+    LOCK_MAILBOX(mbox);
+    libbalsa_filter_match(filter_list, mbox->message_list);
+    UNLOCK_MAILBOX(mbox);
+}
+
+/* Apply all filters on their matching messages (call libbalsa_filter_match before)
+ * returns TRUE if the trash bin has been filled
+ * FIXME : Should position filter_errno on errors (bad command action,bad destination mailbox...)
+ */
+
+gboolean
+libbalsa_filter_apply(GSList * filter_list)
+{
+    GSList * lst;
+    GList * lst_messages;
+    LibBalsaFilter * filt=NULL;
+    gboolean result=FALSE;
+    LibBalsaMailbox *mbox;
 
-    /* OK we have done all the matching thing, now we take every action for matching messages */
+    if (!filter_list) return FALSE;
 
     for (lst=filter_list;lst;lst=g_slist_next(lst)) {
-	LibBalsaMailbox *mbox;
  
 	filt=(LibBalsaFilter*)lst->data;
 	if (FILTER_CHKFLAG(filt,FILTER_SOUND)) {
--- /home/manu/prog/balsa-cvs/balsa/libbalsa/mailbox_local.c	Fri May 17 08:15:11 2002
+++ balsa/libbalsa/mailbox_local.c	Fri May 17 20:01:06 2002
@@ -239,14 +246,18 @@
                                             FILTER_WHEN_INCOMING);
     /* We apply filter if needed */
     if (filters) {
+	LOCK_MAILBOX(mailbox);
 	new_messages=libbalsa_extract_new_messages(mailbox->message_list);
 	if (new_messages) {
-	    if (filters_prepare_to_run(filters))
-		filters_run_on_messages(filters, new_messages);
-	    /* FIXME : do better error report */
-	    else g_warning("Filter error\n");
+	    if (filters_prepare_to_run(filters)) {
+		libbalsa_filter_match(filters, new_messages);
+		UNLOCK_MAILBOX(mailbox);
+		libbalsa_filter_apply(filters);
+	    }
+	    else UNLOCK_MAILBOX(mailbox);
 	    g_list_free(new_messages);
 	}
+	else UNLOCK_MAILBOX(mailbox);
 	g_slist_free(filters);
     }
 }
--- /home/manu/prog/balsa-cvs/balsa/libbalsa/mailbox_pop3.c	Thu May 16 06:35:01 2002
+++ balsa/libbalsa/mailbox_pop3.c	Fri May 17 19:37:37 2002
@@ -308,10 +308,10 @@
         filters = libbalsa_mailbox_filters_when(mailbox->filters,
 						FILTER_WHEN_INCOMING);
 	if (filters) {
-	    if (filters_prepare_to_run(filters))
-		filters_run_on_messages(filters, tmp_mailbox->message_list);
-	    /* FIXME : do better error report */
-	    else g_warning("Filter error\n");
+	    if (filters_prepare_to_run(filters)) {
+		libbalsa_filter_match(filters, tmp_mailbox->message_list);
+		libbalsa_filter_apply(filters);
+	    }
 	    g_slist_free(filters);
 	}
 
--- /home/manu/prog/balsa-cvs/balsa/src/filter-run-callbacks.c	Sun Dec 16 10:29:21 2001
+++ balsa/src/filter-run-callbacks.c	Fri May 17 19:48:05 2002
@@ -28,6 +28,7 @@
 #include <gnome.h>
 
 #include <string.h>
+
 #include "mailbox-filter.h"
 #include "filter-private.h"
 #include "filter-funcs.h"
@@ -95,7 +96,8 @@
     if (!filters_prepare_to_run(filters))
 	return FALSE;
     gtk_clist_freeze(GTK_CLIST(balsa_app.mblist));
-    if (filters_run_on_messages(filters,mbox->message_list))
+    libbalsa_filter_match_mailbox(filters,mbox);
+    if (libbalsa_filter_apply(filters))
 	enable_empty_trash(TRASH_FULL);
     gtk_clist_thaw(GTK_CLIST(balsa_app.mblist));
     g_slist_free(filters);