From 9f1e2628d118474d3e1223cfd2e8c3d22e4e39a8 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sun, 9 Sep 2012 13:25:08 +2000 Subject: [PATCH] Re: [PATCH 06/11] lib: store thread recipients in thread structure --- b1/21b8daa5557cdc2a507c7d183b60f6023371bf | 356 ++++++++++++++++++++++ 1 file changed, 356 insertions(+) create mode 100644 b1/21b8daa5557cdc2a507c7d183b60f6023371bf diff --git a/b1/21b8daa5557cdc2a507c7d183b60f6023371bf b/b1/21b8daa5557cdc2a507c7d183b60f6023371bf new file mode 100644 index 000000000..b4f5ceb2f --- /dev/null +++ b/b1/21b8daa5557cdc2a507c7d183b60f6023371bf @@ -0,0 +1,356 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 504AE431FAE + for ; Sat, 8 Sep 2012 10:25:12 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 g3XTud5bu3VA for ; + Sat, 8 Sep 2012 10:25:11 -0700 (PDT) +Received: from dmz-mailsec-scanner-5.mit.edu (DMZ-MAILSEC-SCANNER-5.MIT.EDU + [18.7.68.34]) + by olra.theworths.org (Postfix) with ESMTP id 0E410431FAF + for ; Sat, 8 Sep 2012 10:25:10 -0700 (PDT) +X-AuditID: 12074422-b7f456d000000926-7e-504b7f767116 +Received: from mailhub-auth-1.mit.edu ( [18.9.21.35]) + by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP + id 3B.E4.02342.67F7B405; Sat, 8 Sep 2012 13:25:10 -0400 (EDT) +Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) + by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id q88HP9t9022746; + Sat, 8 Sep 2012 13:25:10 -0400 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q88HP875026425 + (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); + Sat, 8 Sep 2012 13:25:09 -0400 (EDT) +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) + (envelope-from ) + id 1TAOmK-00061t-64; Sat, 08 Sep 2012 13:25:08 -0400 +Date: Sat, 8 Sep 2012 13:25:08 -0400 +From: Austin Clements +To: Jameson Graef Rollins +Subject: Re: [PATCH 06/11] lib: store thread recipients in thread structure +Message-ID: <20120908172507.GA22299@mit.edu> +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> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: <1345427570-26518-7-git-send-email-jrollins@finestructure.net> +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFmpileLIzCtJLcpLzFFi42IR4hRV1i2r9w4waPinYrFnn5fF9ZszmR2Y + PO6e5vJ4tuoWcwBTFJdNSmpOZllqkb5dAldG54R+1oK9YRUXNl1mbmB859zFyMkhIWAiMXH/ + fRYIW0ziwr31bF2MXBxCAvsYJXqvnmKGcNYzSjya/osdwjnBJDHlyVYoZwmjxNPuh0A9HBws + AipALaEgo9gENCS27V/OCGKLCJhJ9Hz5A2YzC2hJbN34AcwWFvCWuHDhPJjNK6AjseVhLyvE + zAvMEvt/vWOFSAhKnJz5hAWm+ca/l0wgu5gFpCWW/+MACXMCzblyv50JxBYFOmHKyW1sExiF + ZiHpnoWkexZC9wJG5lWMsim5Vbq5iZk5xanJusXJiXl5qUW6pnq5mSV6qSmlmxhBQc3uorSD + 8edBpUOMAhyMSjy8G+S8AoRYE8uKK3MPMUpyMCmJ8u6u8Q4Q4kvKT6nMSCzOiC8qzUktPsQo + wcGsJMJ7PR0ox5uSWFmVWpQPk5LmYFES572WctNfSCA9sSQ1OzW1ILUIJivDwaEkwWtbB9Qo + WJSanlqRlplTgpBm4uAEGc4DNLwApIa3uCAxtzgzHSJ/ilFRSpzXG+QiAZBERmkeXC8s6bxi + FAd6RRhiBQ8wYcF1vwIazAQ0WOSZB8jgkkSElFQDY8N63rKHHOvm3JX8GyIpMCuwX45JZfKC + 2BfvxWViamfesnJ0bWdi+hJp1H5+bs/JtF2JDZPehRubPzq25WhWvoJllv3Nb8+lzgbwHD92 + +K4zS8TO0lBG5+/px4/YT3R5IbVFSbU795ckw8KJmlUbXUSTfU23ujPWup3fUm+8gqdM9cNl + g93flViKMxINtZiLihMBX4BzmxUDAAA= +Cc: Notmuch Mail +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: Sat, 08 Sep 2012 17:25:12 -0000 + +Quoth Jameson Graef Rollins on Aug 19 at 6:52 pm: +> 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. +> --- +> 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); + +Same thing about not calling this explicitly. + +> 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) + +Could we instead omit this argument and construct the recipients list +when notmuch_thread_get_recipients is called for the first time? That +makes things a little harder here, because you'll have to keep a list +of messages in the order they were added and scan it on demand (and +use the matched flag), but it will avoid the public API major version +bump as well as generally minimizing interface complexity. Also, I +think that list will prove valuable for other things in the future, +since currently there's no way to iterate over the messages in a +thread in date order; the best you can do is recursive walk the thread +structure. Conveniently, we already have a message list abstraction. + +> { +> 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); +> + 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; + +-- +Austin Clements MIT/'06/PhD/CSAIL +amdragon@mit.edu http://web.mit.edu/amdragon + Somewhere in the dream we call reality you will find me, + searching for the reality we call dreams. -- 2.26.2