From 6867987074ee2db42ff355aec3ae44ba474e2476 Mon Sep 17 00:00:00 2001 From: Avi Levy Date: Fri, 19 Jul 2013 16:28:56 -0700 Subject: [PATCH] Non-image inline attachments not shown in client: Closes #5748 This processes existing email for inline attachments and adds them to the attachments table, doing the same for new mail as it arrives. --- sql/CMakeLists.txt | 1 + sql/version-013.sql | 9 +++++ src/client/views/conversation-viewer.vala | 43 ++++++++++++++++++--- src/engine/api/geary-attachment.vala | 40 ++++++++++++++++++- src/engine/db/db-versioned-database.vala | 2 +- src/engine/imap-db/imap-db-account.vala | 4 +- src/engine/imap-db/imap-db-database.vala | 45 +++++++++++++++++++++- src/engine/imap-db/imap-db-folder.vala | 28 ++++++++++---- src/engine/rfc822/rfc822-message.vala | 47 ++++++++++++++++------- 9 files changed, 186 insertions(+), 33 deletions(-) create mode 100644 sql/version-013.sql diff --git a/sql/CMakeLists.txt b/sql/CMakeLists.txt index 60abb38d..c44647c0 100644 --- a/sql/CMakeLists.txt +++ b/sql/CMakeLists.txt @@ -12,3 +12,4 @@ install(FILES version-009.sql DESTINATION ${SQL_DEST}) install(FILES version-010.sql DESTINATION ${SQL_DEST}) install(FILES version-011.sql DESTINATION ${SQL_DEST}) install(FILES version-012.sql DESTINATION ${SQL_DEST}) +install(FILES version-013.sql DESTINATION ${SQL_DEST}) diff --git a/sql/version-013.sql b/sql/version-013.sql new file mode 100644 index 00000000..ca78125b --- /dev/null +++ b/sql/version-013.sql @@ -0,0 +1,9 @@ +-- +-- Add the disposition column as a string so the client can decide which attachments to show. +-- Since all attachments up to this point have been non-inline, set it to that value (which +-- is defined in src/engine/api/geary-attachment.vala +-- + +ALTER TABLE MessageAttachmentTable ADD COLUMN disposition INTEGER; +UPDATE MessageAttachmentTable SET disposition=0; + diff --git a/src/client/views/conversation-viewer.vala b/src/client/views/conversation-viewer.vala index bf241005..711b601b 100644 --- a/src/client/views/conversation-viewer.vala +++ b/src/client/views/conversation-viewer.vala @@ -15,6 +15,9 @@ public class ConversationViewer : Gtk.Box { | Geary.Email.Field.FLAGS | Geary.Email.Field.PREVIEW; + public const string INLINE_MIME_TYPES = + "image/png image/gif image/jpeg image/pjpeg image/bmp image/x-icon image/x-xbitmap image/x-xbm"; + private const int ATTACHMENT_PREVIEW_SIZE = 50; private const int SELECT_CONVERSATION_TIMEOUT_MSEC = 100; private const string MESSAGE_CONTAINER_ID = "message_container"; @@ -473,9 +476,10 @@ public class ConversationViewer : Gtk.Box { } } - // Set attachment icon and add the attachments container if we have any attachments. - set_attachment_icon(div_message, email.attachments.size > 0); - if (email.attachments.size > 0) { + // Set attachment icon and add the attachments container if there are displayed attachments. + int displayed = displayed_attachments(email); + set_attachment_icon(div_message, displayed > 0); + if (displayed > 0) { insert_attachments(div_message, email.attachments); } @@ -1284,7 +1288,7 @@ public class ConversationViewer : Gtk.Box { save_attachment_item.activate.connect(() => save_attachment(attachment)); menu.append(save_attachment_item); - if (email.attachments.size > 1) { + if (displayed_attachments(email) > 1) { Gtk.MenuItem save_all_item = new Gtk.MenuItem.with_mnemonic(_("Save All A_ttachments...")); save_all_item.activate.connect(() => save_attachments(email.attachments)); menu.append(save_all_item); @@ -1303,9 +1307,10 @@ public class ConversationViewer : Gtk.Box { Gtk.Menu menu = new Gtk.Menu(); menu.selection_done.connect(on_message_menu_selection_done); - if (email.attachments.size > 0) { + int displayed = displayed_attachments(email); + if (displayed > 0) { string mnemonic = ngettext("Save A_ttachment...", "Save All A_ttachments...", - email.attachments.size); + displayed); Gtk.MenuItem save_all_item = new Gtk.MenuItem.with_mnemonic(mnemonic); save_all_item.activate.connect(() => save_attachments(email.attachments)); menu.append(save_all_item); @@ -1588,6 +1593,29 @@ public class ConversationViewer : Gtk.Box { header_text += create_header_row(Geary.HTML.escape_markup(title), value, important); } + private static bool should_show_attachment(Geary.Attachment attachment) { + switch (attachment.disposition) { + case Geary.Attachment.Disposition.ATTACHMENT: + return true; + + case Geary.Attachment.Disposition.INLINE: + return !(attachment.mime_type in INLINE_MIME_TYPES); + + default: + assert_not_reached(); + } + } + + private static int displayed_attachments(Geary.Email email) { + int ret = 0; + foreach (Geary.Attachment attachment in email.attachments) { + if (should_show_attachment(attachment)) { + ret++; + } + } + return ret; + } + private void insert_attachments(WebKit.DOM.HTMLElement email_container, Gee.List attachments) { @@ -1618,6 +1646,9 @@ public class ConversationViewer : Gtk.Box { // Create an attachment table for each attachment. foreach (Geary.Attachment attachment in attachments) { + if (!should_show_attachment(attachment)) { + continue; + } // Generate the attachment table. WebKit.DOM.HTMLElement attachment_table = Util.DOM.clone_node(attachment_template); string filename = Geary.String.is_empty_or_whitespace(attachment.filename) ? diff --git a/src/engine/api/geary-attachment.vala b/src/engine/api/geary-attachment.vala index 68a3449c..2ab193f2 100644 --- a/src/engine/api/geary-attachment.vala +++ b/src/engine/api/geary-attachment.vala @@ -7,20 +7,58 @@ public class Geary.Attachment : BaseObject { public const Email.Field REQUIRED_FIELDS = Email.REQUIRED_FOR_MESSAGE; + // NOTE: These values are persisted on disk and should not be modified unless you know what + // you're doing. + public enum Disposition { + ATTACHMENT = 0, + INLINE = 1; + + public static Disposition? from_string(string? str) { + // Returns null to indicate an unknown disposition + if (str == null) { + return null; + } + + switch (str.down()) { + case "attachment": + return ATTACHMENT; + + case "inline": + return INLINE; + + default: + return null; + } + } + + public static Disposition from_int(int i) { + switch (i) { + case INLINE: + return INLINE; + + case ATTACHMENT: + default: + return ATTACHMENT; + } + } + } + public string? filename { get; private set; } public string filepath { get; private set; } public string mime_type { get; private set; } public int64 filesize { get; private set; } public int64 id { get; private set; } + public Disposition disposition { get; private set; } internal Attachment(File data_dir, string? filename, string mime_type, int64 filesize, - int64 message_id, int64 attachment_id) { + int64 message_id, int64 attachment_id, Disposition disposition) { this.filename = filename; this.mime_type = mime_type; this.filesize = filesize; this.filepath = get_path(data_dir, message_id, attachment_id, filename); this.id = attachment_id; + this.disposition = disposition; } internal static string get_path(File data_dir, int64 message_id, int64 attachment_id, diff --git a/src/engine/db/db-versioned-database.vala b/src/engine/db/db-versioned-database.vala index 18cd9cb4..784ef1a1 100644 --- a/src/engine/db/db-versioned-database.vala +++ b/src/engine/db/db-versioned-database.vala @@ -52,7 +52,7 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database { check_cancelled("VersionedDatabase.open", cancellable); try { - debug("Upgrading database to to version %d with %s", db_version, upgrade_script.get_path()); + debug("Upgrading database to version %d with %s", db_version, upgrade_script.get_path()); cx.exec_transaction(TransactionType.EXCLUSIVE, (cx) => { cx.exec_file(upgrade_script, cancellable); cx.set_user_version_number(db_version); diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index d0526923..12aed121 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -1065,7 +1065,7 @@ private class Geary.ImapDB.Account : BaseObject { // For a message row id, return a set of all folders it's in, or null if // it's not in any folders. - private Gee.Set? do_find_email_folders(Db.Connection cx, int64 message_id, + private static Gee.Set? do_find_email_folders(Db.Connection cx, int64 message_id, Cancellable? cancellable) throws Error { Db.Statement stmt = cx.prepare("SELECT folder_id FROM MessageLocationTable WHERE message_id=?"); stmt.bind_int64(0, message_id); @@ -1090,7 +1090,7 @@ private class Geary.ImapDB.Account : BaseObject { // For a folder row id, return the folder path (constructed with default // separator and case sensitivity) of that folder, or null in the event // it's not found. - private Geary.FolderPath? do_find_folder_path(Db.Connection cx, int64 folder_id, + private static Geary.FolderPath? do_find_folder_path(Db.Connection cx, int64 folder_id, Cancellable? cancellable) throws Error { Db.Statement stmt = cx.prepare("SELECT parent_id, name FROM FolderTable WHERE id=?"); stmt.bind_int64(0, folder_id); diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala index c68ac85c..d9ba067a 100644 --- a/src/engine/imap-db/imap-db-database.vala +++ b/src/engine/imap-db/imap-db-database.vala @@ -43,6 +43,10 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { case 12: post_upgrade_populate_internal_date_time_t(); break; + + case 13: + post_upgrade_populate_inline_attachments(); + break; } } @@ -186,7 +190,46 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { return Db.TransactionOutcome.COMMIT; }); } catch (Error e) { - debug("Error populating internaldate_time_t column during upgrade to database schema 11: %s", + debug("Error populating internaldate_time_t column during upgrade to database schema 12: %s", + e.message); + } + } + + // Version 13. + private void post_upgrade_populate_inline_attachments() { + try { + exec_transaction(Db.TransactionType.RO, (cx) => { + Db.Statement stmt = cx.prepare(""" + SELECT id, header, body + FROM MessageTable + WHERE (fields & ?) = ? + """); + stmt.bind_int(0, Geary.Email.REQUIRED_FOR_MESSAGE); + stmt.bind_int(1, Geary.Email.REQUIRED_FOR_MESSAGE); + Db.Result select = stmt.exec(); + while (!select.finished) { + int64 id = select.rowid_at(0); + Geary.Memory.Buffer header = select.string_buffer_at(1); + Geary.Memory.Buffer body = select.string_buffer_at(2); + + try { + Geary.RFC822.Message message = new Geary.RFC822.Message.from_parts( + new RFC822.Header(header), new RFC822.Text(body)); + Geary.ImapDB.Folder.do_save_attachments_db(cx, id, + message.get_attachments(Geary.Attachment.Disposition.INLINE), + this, null); + } catch (Error e) { + debug("Error fetching inline Mime parts: %s", e.message); + } + + select.next(); + pump_event_loop(); + } + + return Db.TransactionOutcome.COMMIT; + }); + } catch (Error e) { + debug("Error populating old inline attachments during upgrade to database schema 13: %s", e.message); } } diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index bb863d57..87fd0b11 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -1523,9 +1523,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { private static Gee.List? do_list_attachments(Db.Connection cx, int64 message_id, Cancellable? cancellable) throws Error { - Db.Statement stmt = cx.prepare( - "SELECT id, filename, mime_type, filesize FROM MessageAttachmentTable WHERE message_id=? " - + "ORDER BY id"); + Db.Statement stmt = cx.prepare(""" + SELECT id, filename, mime_type, filesize, disposition + FROM MessageAttachmentTable + WHERE message_id = ? + ORDER BY id + """); stmt.bind_rowid(0, message_id); Db.Result results = stmt.exec(cancellable); @@ -1535,20 +1538,27 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { Gee.List list = new Gee.ArrayList(); do { list.add(new Geary.Attachment(cx.database.db_file.get_parent(), results.string_at(1), - results.string_at(2), results.int64_at(3), message_id, results.rowid_at(0))); + results.string_at(2), results.int64_at(3), message_id, results.rowid_at(0), + Attachment.Disposition.from_int(results.int_at(4)))); } while (results.next(cancellable)); return list; } - + private void do_save_attachments(Db.Connection cx, int64 message_id, Gee.List? attachments, Cancellable? cancellable) throws Error { + do_save_attachments_db(cx, message_id, attachments, db, cancellable); + } + + public static void do_save_attachments_db(Db.Connection cx, int64 message_id, + Gee.List? attachments, ImapDB.Database db, Cancellable? cancellable) throws Error { // nothing to do if no attachments if (attachments == null || attachments.size == 0) return; foreach (GMime.Part attachment in attachments) { string mime_type = attachment.get_content_type().to_string(); + string disposition = attachment.get_disposition(); string? filename = attachment.get_filename(); if (String.is_empty(filename)) { /// Placeholder filename for attachments with no filename. @@ -1565,13 +1575,15 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { uint filesize = byte_array.len; // Insert it into the database. - Db.Statement stmt = cx.prepare( - "INSERT INTO MessageAttachmentTable (message_id, filename, mime_type, filesize) " + - "VALUES (?, ?, ?, ?)"); + Db.Statement stmt = cx.prepare(""" + INSERT INTO MessageAttachmentTable (message_id, filename, mime_type, filesize, disposition) + VALUES (?, ?, ?, ?, ?) + """); stmt.bind_rowid(0, message_id); stmt.bind_string(1, filename); stmt.bind_string(2, mime_type); stmt.bind_uint(3, filesize); + stmt.bind_int(4, Attachment.Disposition.from_string(disposition)); int64 attachment_id = stmt.exec_insert(cancellable); diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala index 07d8e3f8..7d185992 100644 --- a/src/engine/rfc822/rfc822-message.vala +++ b/src/engine/rfc822/rfc822-message.vala @@ -576,29 +576,48 @@ public class Geary.RFC822.Message : BaseObject { } return null; } - - internal Gee.List get_attachments() throws RFC822Error { + + internal Gee.List get_attachments(Geary.Attachment.Disposition? disposition = null) + throws RFC822Error { + // A null disposition means "return all Mime parts recognized by Geary.Attachment.Disposition" Gee.List attachments = new Gee.ArrayList(); - find_attachments(attachments, message.get_mime_part() ); + get_attachments_recursively(attachments, message.get_mime_part(), disposition); return attachments; } - - private void find_attachments(Gee.List attachments, GMime.Object root) - throws RFC822Error { - + + private void get_attachments_recursively(Gee.List attachments, GMime.Object root, + Geary.Attachment.Disposition? requested_disposition) throws RFC822Error { // If this is a multipart container, dive into each of its children. - if (root is GMime.Multipart) { - GMime.Multipart multipart = root as GMime.Multipart; + GMime.Multipart? multipart = root as GMime.Multipart; + if (multipart != null) { int count = multipart.get_count(); for (int i = 0; i < count; ++i) { - find_attachments(attachments, multipart.get_part(i)); + get_attachments_recursively(attachments, multipart.get_part(i), requested_disposition); } return; } - - // Otherwise see if it has a content disposition of "attachment." - if (root is GMime.Part && String.nullable_stri_equal(root.get_disposition(), "attachment")) { - attachments.add(root as GMime.Part); + + // Otherwise, check if this part should be an attachment + GMime.Part? part = root as GMime.Part; + if (part == null) { + return; + } + + Geary.Attachment.Disposition? part_disposition = Geary.Attachment.Disposition.from_string( + part.get_disposition()); + if (part_disposition == null) { + // The part disposition was unknown to Geary.Attachment.Disposition + return; + } + + if (requested_disposition == null) { + // Return any attachment whose disposition is recognized by Geary.Attachment.Disposition + attachments.add(part); + return; + } + + if (part_disposition == requested_disposition) { + attachments.add(part); } }