From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 56372431FB6 for ; Sun, 2 Sep 2012 00:52:25 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -1.098 X-Spam-Level: X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5 tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id SPCrILXDRfq2 for ; Sun, 2 Sep 2012 00:52:24 -0700 (PDT) Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id DA000431FAF for ; Sun, 2 Sep 2012 00:52:23 -0700 (PDT) Received: from smtp.qmul.ac.uk ([138.37.6.40]) by mail2.qmul.ac.uk with esmtp (Exim 4.71) (envelope-from ) id 1T84yf-0002nP-7D; Sun, 02 Sep 2012 08:52:19 +0100 Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost) by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69) (envelope-from ) id 1T84ye-0006fi-IO; Sun, 02 Sep 2012 08:52:17 +0100 From: Mark Walters To: Jameson Graef Rollins , Notmuch Mail Subject: Re: [PATCH 06/11] lib: store thread recipients in thread structure In-Reply-To: <1345427570-26518-7-git-send-email-jrollins@finestructure.net> References: <1345427570-26518-1-git-send-email-jrollins@finestructure.net> <1345427570-26518-2-git-send-email-jrollins@finestructure.net> <1345427570-26518-3-git-send-email-jrollins@finestructure.net> <1345427570-26518-4-git-send-email-jrollins@finestructure.net> <1345427570-26518-5-git-send-email-jrollins@finestructure.net> <1345427570-26518-6-git-send-email-jrollins@finestructure.net> <1345427570-26518-7-git-send-email-jrollins@finestructure.net> User-Agent: Notmuch/0.14+28~ge689a44 (http://notmuchmail.org) Emacs/23.4.1 (x86_64-pc-linux-gnu) Date: Sun, 02 Sep 2012 08:52:15 +0100 Message-ID: <87ligsx3o0.fsf@qmul.ac.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Sender-Host-Address: 93.97.24.31 X-QM-SPAM-Info: Sender has good ham record. :) X-QM-Body-MD5: d7b69fec47cff9dcbc3c3c3e010f9da1 (of first 20000 bytes) X-SpamAssassin-Score: -2.3 X-SpamAssassin-SpamBar: -- X-SpamAssassin-Report: The QM spam filters have analysed this message to determine if it is spam. We require at least 5.0 points to mark a message as spam. This message scored -2.3 points. Summary of the scoring: * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, * medium trust * [138.37.6.40 listed in list.dnswl.org] * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider * (markwalters1009[at]gmail.com) X-QM-Scan-Virus: ClamAV says the message is clean X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Sep 2012 07:52:25 -0000 On Mon, 20 Aug 2012, Jameson Graef Rollins wrote: > This utilizes the new thread addresses struct to store thread > recipients, again in parallel to authors. > > Since message recipients are not stored in the database, including > recipients in the thread structure exacts a significant overhead as > the recipients are retrieved from the original message files. Because > of this, a new boolean argument, include_recipients, is added to the > necessary functions (_notmuch_thread_create, _thread_add_message and > _thread_add_matched_message) that controls whether the recipients are > fetched and included. If message recipients are ever stored in the > database this new argument could probably be removed. > --- Hi I have briefly looked through the series and overall it looks good. > lib/notmuch-private.h | 3 +- > lib/notmuch.h | 14 +++++++++ > lib/query.cc | 3 +- > lib/thread.cc | 77 +++++++++++++++++++++++++++++++++++++------------ > 4 files changed, 76 insertions(+), 21 deletions(-) > > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h > index 27a41b6..32d1523 100644 > --- a/lib/notmuch-private.h > +++ b/lib/notmuch-private.h > @@ -232,7 +232,8 @@ _notmuch_thread_create (void *ctx, > unsigned int seed_doc_id, > notmuch_doc_id_set_t *match_set, > notmuch_string_list_t *excluded_terms, > - notmuch_sort_t sort); > + notmuch_sort_t sort, > + notmuch_bool_t include_recipients); > > /* message.cc */ > > diff --git a/lib/notmuch.h b/lib/notmuch.h > index 6acd38d..f9e71c1 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -759,6 +759,20 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread); > const char * > notmuch_thread_get_authors (notmuch_thread_t *thread); > > +/* Get the recipients of 'thread' > + * > + * The returned string is a comma-separated list of the names of the > + * recipients of mail messages in the query results that belong to this > + * thread. > + * > + * The returned string belongs to 'thread' and as such, should not be > + * modified by the caller and will only be valid for as long as the > + * thread is valid, (which is until notmuch_thread_destroy or until > + * the query from which it derived is destroyed). > + */ > +const char * > +notmuch_thread_get_recipients (notmuch_thread_t *thread); > + > /* Get the subject of 'thread' > * > * The subject is taken from the first message (according to the query > diff --git a/lib/query.cc b/lib/query.cc > index e9c1a2d..54833a7 100644 > --- a/lib/query.cc > +++ b/lib/query.cc > @@ -486,7 +486,8 @@ notmuch_threads_get (notmuch_threads_t *threads) > doc_id, > &threads->match_set, > threads->query->exclude_terms, > - threads->query->sort); > + threads->query->sort, > + FALSE); > } > > void > diff --git a/lib/thread.cc b/lib/thread.cc > index 757e143..baf07c2 100644 > --- a/lib/thread.cc > +++ b/lib/thread.cc > @@ -37,6 +37,7 @@ struct visible _notmuch_thread { > char *thread_id; > char *subject; > notmuch_thread_addresses_t *authors; > + notmuch_thread_addresses_t *recipients; > GHashTable *tags; > > notmuch_message_list_t *message_list; > @@ -63,6 +64,7 @@ static int > _notmuch_thread_destructor (notmuch_thread_t *thread) > { > _notmuch_thread_addresses_destructor (thread->authors); > + _notmuch_thread_addresses_destructor (thread->recipients); > g_hash_table_unref (thread->tags); > g_hash_table_unref (thread->message_hash); > return 0; > @@ -204,14 +206,17 @@ _thread_cleanup_address (notmuch_thread_t *thread, > static void > _thread_add_message (notmuch_thread_t *thread, > notmuch_message_t *message, > - notmuch_string_list_t *exclude_terms) > + notmuch_string_list_t *exclude_terms, > + notmuch_bool_t include_recipients) > { > notmuch_tags_t *tags; > const char *tag; > - InternetAddressList *list = NULL; > + InternetAddressList *from_list = NULL; > + InternetAddressList *to_list = NULL; > InternetAddress *address; > const char *from, *author; > - char *clean_author; > + const char *to, *recipient; > + char *clean_address; > > _notmuch_message_list_add_message (thread->message_list, > talloc_steal (thread, message)); > @@ -223,10 +228,9 @@ _thread_add_message (notmuch_thread_t *thread, > > from = notmuch_message_get_header (message, "from"); > if (from) > - list = internet_address_list_parse_string (from); > - > - if (list) { > - address = internet_address_list_get_address (list, 0); > + from_list = internet_address_list_parse_string (from); > + if (from_list) { > + address = internet_address_list_get_address (from_list, 0); > if (address) { > author = internet_address_get_name (address); > if (author == NULL) { > @@ -234,11 +238,32 @@ _thread_add_message (notmuch_thread_t *thread, > mailbox = INTERNET_ADDRESS_MAILBOX (address); > author = internet_address_mailbox_get_addr (mailbox); > } > - clean_author = _thread_cleanup_author (thread, author, from); > - _thread_add_address (thread->authors, clean_author, FALSE); > - notmuch_message_set_author (message, clean_author); > + clean_address = _thread_cleanup_address (thread, author, from); > + _thread_add_address (thread->authors, clean_address, FALSE); > + notmuch_message_set_author (message, clean_address); > + } > + g_object_unref (G_OBJECT (from_list)); > + } > + > + if (include_recipients) { > + to = notmuch_message_get_header (message, "to"); > + if (to) > + to_list = internet_address_list_parse_string (to); > + if (to_list) { > + address = internet_address_list_get_address (to_list, 0); I think you are just adding the first address on the to: line. Is that deliberate? The comment before notmuch_thread_get_recipients (above) suggests all recipients are being added, but doing so may make the string too large to be useful. Best wishes Mark > + if (address) { > + recipient = internet_address_get_name (address); > + if (recipient == NULL) { > + InternetAddressMailbox *mailbox; > + mailbox = INTERNET_ADDRESS_MAILBOX (address); > + recipient = internet_address_mailbox_get_addr (mailbox); > + } > + clean_address = _thread_cleanup_address (thread, recipient, to); > + _thread_add_address (thread->recipients, clean_address, FALSE); > + notmuch_message_set_recipients (message, clean_address); > } > - g_object_unref (G_OBJECT (list)); > + g_object_unref (G_OBJECT (to_list)); > + } > } > > if (! thread->subject) { > @@ -301,7 +326,8 @@ _thread_set_subject_from_message (notmuch_thread_t *thread, > static void > _thread_add_matched_message (notmuch_thread_t *thread, > notmuch_message_t *message, > - notmuch_sort_t sort) > + notmuch_sort_t sort, > + notmuch_bool_t include_recipients) > { > time_t date; > notmuch_message_t *hashed_message; > @@ -331,6 +357,8 @@ _thread_add_matched_message (notmuch_thread_t *thread, > } > > _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE); > + if (include_recipients) > + _thread_add_address (thread->recipients, notmuch_message_get_recipients (hashed_message), TRUE); > } > > static void > @@ -399,10 +427,10 @@ _thread_addresses_init (const void *ctx) > * > * Creating the thread will perform a database search to get all > * messages belonging to the thread and will get the first subject > - * line, the total count of messages, and all authors in the thread. > - * Each message in the thread is checked against match_set to allow > - * for a separate count of matched messages, and to allow a viewer to > - * display these messages differently. > + * line, the total count of messages, and all authors and recipients > + * of the thread. Each message in the thread is checked against > + * match_set to allow for a separate count of matched messages, and to > + * allow a viewer to display these messages differently. > * > * Here, 'ctx' is talloc context for the resulting thread object. > * > @@ -414,7 +442,8 @@ _notmuch_thread_create (void *ctx, > unsigned int seed_doc_id, > notmuch_doc_id_set_t *match_set, > notmuch_string_list_t *exclude_terms, > - notmuch_sort_t sort) > + notmuch_sort_t sort, > + notmuch_bool_t include_recipients) > { > notmuch_thread_t *thread; > notmuch_message_t *seed_message; > @@ -453,6 +482,9 @@ _notmuch_thread_create (void *ctx, > thread->authors = _thread_addresses_init (thread); > if (unlikely (thread->authors == NULL)) > return NULL; > + thread->recipients = _thread_addresses_init (thread); > + if (unlikely (thread->recipients == NULL)) > + return NULL; > > thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal, > free, NULL); > @@ -486,11 +518,11 @@ _notmuch_thread_create (void *ctx, > if (doc_id == seed_doc_id) > message = seed_message; > > - _thread_add_message (thread, message, exclude_terms); > + _thread_add_message (thread, message, exclude_terms, include_recipients); > > if ( _notmuch_doc_id_set_contains (match_set, doc_id)) { > _notmuch_doc_id_set_remove (match_set, doc_id); > - _thread_add_matched_message (thread, message, sort); > + _thread_add_matched_message (thread, message, sort, include_recipients); > } > > _notmuch_message_close (message); > @@ -499,6 +531,7 @@ _notmuch_thread_create (void *ctx, > notmuch_query_destroy (thread_id_query); > > _resolve_thread_addresses_string (thread->authors); > + _resolve_thread_addresses_string (thread->recipients); > > _resolve_thread_relationships (thread); > > @@ -536,6 +569,12 @@ notmuch_thread_get_authors (notmuch_thread_t *thread) > } > > const char * > +notmuch_thread_get_recipients (notmuch_thread_t *thread) > +{ > + return thread->recipients->string; > +} > + > +const char * > notmuch_thread_get_subject (notmuch_thread_t *thread) > { > return thread->subject; > -- > 1.7.10.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch