[evolution-patches] Re: [Evolution-hackers] mark junk/not junk problem
- From: Not Zed <notzed ximian com>
- To: Radek Doulík <rodo ximian com>
- Cc: evolution-patches ximian com
- Subject: [evolution-patches] Re: [Evolution-hackers] mark junk/not junk problem
- Date: Fri, 12 Mar 2004 15:09:50 +0800
comments inline ... there are 2 small bugs in the code, and a couple of
minor stylistic things.
On Thu, 2004-03-11 at 22:44 +0100, Radek Doulík wrote:
> 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;
> + }
> +
You probably meant & not && in the second case. This can also be
written as:
if ((flags & (CAMEL_MESSAGE_JUNK|CAMEL_MESSAGE_JUNK_LEARN)) ==
CAMEL_MESSAGE_JUNK) {
}
(which personally i find easier to understand).
> 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) {
very minor, could rename nonjunk -> notjunk (to match the plugin api
names).
> + 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
Anohter minor thing, since the function's getting a bit longer now, the
changed != NULL check should probably be changed == NULL and a return
from the function. Just to stop it getting too deep.
> *)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;
if i read the diff right, this int is now defined after the first
statment - it wont build in c89.
> - 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;
Otherwise, it looks pretty good.
Thanks,
Michael
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
