diff --git a/src/client/conversation-viewer/conversation-email.vala b/src/client/conversation-viewer/conversation-email.vala index a7f3e0b5..971c4536 100644 --- a/src/client/conversation-viewer/conversation-email.vala +++ b/src/client/conversation-viewer/conversation-email.vala @@ -184,7 +184,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { } } catch (Error error) { debug("Failed to load icon for attachment '%s': %s", - this.attachment.id, + this.attachment.file.get_path(), error.message); } diff --git a/src/engine/api/geary-attachment.vala b/src/engine/api/geary-attachment.vala index 25ad46fb..b97e8375 100644 --- a/src/engine/api/geary-attachment.vala +++ b/src/engine/api/geary-attachment.vala @@ -11,13 +11,6 @@ public abstract class Geary.Attachment : BaseObject { - /** - * An identifier that can be used to locate the {@link Attachment} in an {@link Email}. - * - * @see Email.get_attachment_by_id - */ - public string id { get; private set; } - /** * The {@link Mime.ContentType} of the {@link Attachment}. */ @@ -62,31 +55,30 @@ public abstract class Geary.Attachment : BaseObject { public string? content_filename { get; private set; } /** - * The on-disk File of the {@link Attachment}. + * The attachment's on-disk File, if any. + * + * This will be null if the attachment has not been saved to disk. */ - public File file { get; private set; } + public GLib.File? file { get; private set; default = null; } /** * The file size (in bytes) if the {@link file}. + * + * This will be -1 if the attachment has not been saved to disk. */ - public int64 filesize { get; private set; } + public int64 filesize { get; private set; default = -1; } - protected Attachment(string id, - Mime.ContentType content_type, + + protected Attachment(Mime.ContentType content_type, string? content_id, string? content_description, Mime.ContentDisposition content_disposition, - string? content_filename, - File file, - int64 filesize) { - this.id = id; + string? content_filename) { this.content_type = content_type; this.content_id = content_id; this.content_description = content_description; this.content_disposition = content_disposition; this.content_filename = content_filename; - this.file = file; - this.filesize = filesize; } /** @@ -111,7 +103,7 @@ public abstract class Geary.Attachment : BaseObject { string[] others = { alt_file_name, this.content_id, - this.id ?? "attachment", + "attachment", }; int i = 0; @@ -162,4 +154,12 @@ public abstract class Geary.Attachment : BaseObject { return file_name; } + /** + * Sets the attachment's on-disk location and size. + */ + protected void set_file_info(GLib.File file, int64 file_size) { + this.file = file; + this.filesize = file_size; + } + } diff --git a/src/engine/api/geary-email.vala b/src/engine/api/geary-email.vala index 1240150e..dccaf75e 100644 --- a/src/engine/api/geary-email.vala +++ b/src/engine/api/geary-email.vala @@ -372,25 +372,6 @@ public class Geary.Email : BaseObject { return message; } - /** - * Returns the attachment with the given {@link Geary.Attachment.id}. - * - * Requires the REQUIRED_FOR_MESSAGE fields be present; else - * EngineError.INCOMPLETE_MESSAGE is thrown. - */ - public Geary.Attachment? get_attachment_by_id(string attachment_id) - throws EngineError { - if (!fields.fulfills(REQUIRED_FOR_MESSAGE)) - throw new EngineError.INCOMPLETE_MESSAGE("Parsed email requires HEADER and BODY"); - - foreach (Geary.Attachment attachment in attachments) { - if (attachment.id == attachment_id) { - return attachment; - } - } - return null; - } - /** * Returns the attachment with the given MIME Content ID. * diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index 41b86da0..41eac271 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -728,7 +728,7 @@ private class Geary.ImapDB.Account : BaseObject { // Ignore any messages that don't have the required fields. if (partial_ok || row.fields.fulfills(requested_fields)) { Geary.Email email = row.to_email(new Geary.ImapDB.EmailIdentifier(id, null)); - Attachment.do_add_attachments( + Attachment.add_attachments( cx, this.db.attachments_path, email, id, cancellable ); @@ -1393,7 +1393,7 @@ private class Geary.ImapDB.Account : BaseObject { email_id.to_string(), row.fields, required_fields); email = row.to_email(email_id); - Attachment.do_add_attachments( + Attachment.add_attachments( cx, this.db.attachments_path, email, email_id.message_id, cancellable ); @@ -1556,7 +1556,7 @@ private class Geary.ImapDB.Account : BaseObject { MessageRow row = Geary.ImapDB.Folder.do_fetch_message_row( cx, message_id, search_fields, out db_fields, cancellable); Geary.Email email = row.to_email(new Geary.ImapDB.EmailIdentifier(message_id, null)); - Attachment.do_add_attachments( + Attachment.add_attachments( cx, this.db.attachments_path, email, message_id, cancellable ); diff --git a/src/engine/imap-db/imap-db-attachment.vala b/src/engine/imap-db/imap-db-attachment.vala index 19f5b0dd..5897317d 100644 --- a/src/engine/imap-db/imap-db-attachment.vala +++ b/src/engine/imap-db/imap-db-attachment.vala @@ -1,261 +1,307 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. +/* + * Copyright 2016 Software Freedom Conservancy Inc. * * This software is licensed under the GNU Lesser General Public License * (version 2.1 or later). See the COPYING file in this distribution. */ private class Geary.ImapDB.Attachment : Geary.Attachment { + + public const Email.Field REQUIRED_FIELDS = Email.REQUIRED_FOR_MESSAGE; - internal const string NULL_FILE_NAME = "none"; + private const string NULL_FILE_NAME = "none"; - public Attachment(int64 message_id, - int64 attachment_id, - Mime.ContentType content_type, - string? content_id, - string? content_description, - Mime.ContentDisposition content_disposition, - string? content_filename, - File data_dir, - int64 filesize) { - base (generate_id(attachment_id), - content_type, - content_id, - content_description, - content_disposition, - content_filename, - generate_file(data_dir, message_id, attachment_id, content_filename), - filesize); + + internal int64 message_id { get; private set; } + + private int64 attachment_id; + + + private Attachment(int64 message_id, + int64 attachment_id, + Mime.ContentType content_type, + string? content_id, + string? content_description, + Mime.ContentDisposition content_disposition, + string? content_filename) { + base( + content_type, + content_id, + content_description, + content_disposition, + content_filename + ); + + this.message_id = message_id; + this.attachment_id = attachment_id; } - private static string generate_id(int64 attachment_id) { - return "imap-db:%s".printf(attachment_id.to_string()); - } - - public static File generate_file(File attachements_dir, int64 message_id, int64 attachment_id, - string? filename) { - return attachements_dir - .get_child(message_id.to_string()) - .get_child(attachment_id.to_string()) - .get_child(filename ?? NULL_FILE_NAME); - } - - internal static void do_save_attachments(Db.Connection cx, - GLib.File attachments_path, - int64 message_id, - Gee.List? attachments, - Cancellable? cancellable) + internal Attachment.from_part(int64 message_id, GMime.Part part) throws Error { - // nothing to do if no attachments - if (attachments == null || attachments.size == 0) - return; + GMime.ContentType? part_type = part.get_content_type(); + Mime.ContentType type = (part_type != null) + ? new Mime.ContentType.from_gmime(part_type) + : Mime.ContentType.deserialize(Mime.ContentType.DEFAULT_CONTENT_TYPE); - foreach (GMime.Part attachment in attachments) { - GMime.ContentType? content_type = attachment.get_content_type(); - string mime_type = (content_type != null) - ? content_type.to_string() - : Mime.ContentType.DEFAULT_CONTENT_TYPE; - string? disposition = attachment.get_disposition(); - string? content_id = attachment.get_content_id(); - string? description = attachment.get_content_description(); - string? filename = RFC822.Utils.get_clean_attachment_filename(attachment); + GMime.ContentDisposition? part_disposition = part.get_content_disposition(); + Mime.ContentDisposition disposition = (part_disposition != null) + ? new Mime.ContentDisposition.from_gmime(part_disposition) + : new Mime.ContentDisposition.simple( + Geary.Mime.DispositionType.UNSPECIFIED + ); - // Convert the attachment content into a usable ByteArray. - GMime.DataWrapper? attachment_data = attachment.get_content_object(); - ByteArray byte_array = new ByteArray(); - GMime.StreamMem stream = new GMime.StreamMem.with_byte_array(byte_array); - stream.set_owner(false); - if (attachment_data != null) - attachment_data.write_to_stream(stream); // data is null if it's 0 bytes - uint filesize = byte_array.len; + this( + message_id, + -1, // This gets set only after saving + type, + part.get_content_id(), + part.get_content_description(), + disposition, + RFC822.Utils.get_clean_attachment_filename(part) + ); + } - // convert into DispositionType enum, which is stored as int - // (legacy code stored UNSPECIFIED as NULL, which is zero, which is ATTACHMENT, so preserve - // this behavior) - Mime.DispositionType disposition_type = Mime.DispositionType.deserialize(disposition, - null); - if (disposition_type == Mime.DispositionType.UNSPECIFIED) - disposition_type = Mime.DispositionType.ATTACHMENT; + internal Attachment.from_row(Geary.Db.Result result, File attachments_dir) + throws Error { + string? content_filename = result.string_for("filename"); + if (content_filename == ImapDB.Attachment.NULL_FILE_NAME) { + // Prior to 0.12, Geary would store the untranslated + // string "none" as the filename when none was + // specified by the MIME content disposition. Check + // for that and clean it up. + content_filename = null; + } - // Insert it into the database. - Db.Statement stmt = cx.prepare(""" + Mime.ContentDisposition disposition = new Mime.ContentDisposition.simple( + Mime.DispositionType.from_int(result.int_for("disposition")) + ); + + this( + result.rowid_for("message_id"), + result.rowid_for("id"), + Mime.ContentType.deserialize(result.nonnull_string_for("mime_type")), + result.string_for("content_id"), + result.string_for("description"), + disposition, + content_filename + ); + + set_file_info( + generate_file(attachments_dir), result.int64_for("filesize") + ); + } + + internal void save(Db.Connection cx, + GMime.Part part, + GLib.File attachments_dir, + Cancellable? cancellable) + throws Error { + insert_db(cx, cancellable); + try { + save_file(part, attachments_dir, cancellable); + update_db(cx, cancellable); + } catch (Error err) { + // Don't honour the cancellable here, we need to delete + // it. + this.delete(cx, cancellable); + throw err; + } + } + + // This isn't async since its only callpaths are via db async + // transactions, which run in independent threads. + internal void delete(Db.Connection cx, Cancellable? cancellable) { + if (this.attachment_id >= 0) { + try { + Db.Statement remove_stmt = cx.prepare( + "DELETE FROM MessageAttachmentTable WHERE id=?"); + remove_stmt.bind_rowid(0, this.attachment_id); + + remove_stmt.exec(); + } catch (Error err) { + debug("Error attempting to remove added attachment row for %s: %s", + this.file.get_path(), err.message); + } + } + + if (this.file != null) { + try { + this.file.delete(cancellable); + } catch (Error err) { + debug("Error attempting to remove attachment file %s: %s", + this.file.get_path(), err.message); + } + } + } + + private void insert_db(Db.Connection cx, Cancellable? cancellable) + throws Error { + // Insert it into the database. + Db.Statement stmt = cx.prepare(""" INSERT INTO MessageAttachmentTable (message_id, filename, mime_type, filesize, disposition, content_id, description) 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, disposition_type); - stmt.bind_string(5, content_id); - stmt.bind_string(6, description); + stmt.bind_rowid(0, this.message_id); + stmt.bind_string(1, this.content_filename); + stmt.bind_string(2, this.content_type.to_string()); + stmt.bind_int64(3, 0); // This is updated after saving the file + stmt.bind_int(4, this.content_disposition.disposition_type); + stmt.bind_string(5, this.content_id); + stmt.bind_string(6, this.content_description); - int64 attachment_id = stmt.exec_insert(cancellable); - File saved_file = ImapDB.Attachment.generate_file( - attachments_path, message_id, attachment_id, filename + this.attachment_id = stmt.exec_insert(cancellable); + } + + // This isn't async since its only callpaths are via db async + // transactions, which run in independent threads + private void save_file(GMime.Part part, + GLib.File attachments_dir, + Cancellable? cancellable) + throws Error { + if (this.attachment_id < 0) { + throw new IOError.NOT_FOUND("No attachment id assigned"); + } + + File target = generate_file(attachments_dir); + + // create directory, but don't throw exception if already exists + try { + target.get_parent().make_directory_with_parents(cancellable); + } catch (IOError ioe) { + // fall through if already exists + if (!(ioe is IOError.EXISTS)) + throw ioe; + } + + // Delete any existing file now since we might not be creating + // it again below. + try { + target.delete(cancellable); + } catch (IOError err) { + // All good + } + + // Save the data to disk if there is any. + GMime.DataWrapper? attachment_data = part.get_content_object(); + if (attachment_data != null) { + GLib.OutputStream target_stream = target.create( + FileCreateFlags.NONE, cancellable + ); + GMime.Stream stream = new Geary.Stream.MimeOutputStream( + target_stream + ); + stream = new GMime.StreamBuffer( + stream, GMime.StreamBufferMode.BLOCK_WRITE ); - // On the off-chance this is marked for deletion, unmark it - try { - stmt = cx.prepare(""" - DELETE FROM DeleteAttachmentFileTable - WHERE filename = ? - """); - stmt.bind_string(0, saved_file.get_path()); + attachment_data.write_to_stream(stream); - stmt.exec(cancellable); - } catch (Error err) { - debug("Unable to delete from DeleteAttachmentFileTable: %s", err.message); + // Using the stream's length is a bit of a hack, but at + // least on one system we are getting 0 back for the file + // size if we use target.query_info(). + stream.flush(); + int64 file_size = stream.length(); - // not a deal-breaker, fall through - } + stream.close(); - debug("Saving attachment to %s", saved_file.get_path()); - - try { - // create directory, but don't throw exception if already exists - try { - saved_file.get_parent().make_directory_with_parents(cancellable); - } catch (IOError ioe) { - // fall through if already exists - if (!(ioe is IOError.EXISTS)) - throw ioe; - } - - // REPLACE_DESTINATION doesn't seem to work as advertised all the time ... just - // play it safe here - if (saved_file.query_exists(cancellable)) - saved_file.delete(cancellable); - - // Create the file where the attachment will be saved and get the output stream. - FileOutputStream saved_stream = saved_file.create(FileCreateFlags.REPLACE_DESTINATION, - cancellable); - - // Save the data to disk and flush it. - size_t written; - if (filesize != 0) - saved_stream.write_all(byte_array.data[0:filesize], out written, cancellable); - - saved_stream.flush(cancellable); - } catch (Error error) { - // An error occurred while saving the attachment, so lets remove the attachment from - // the database and delete the file (in case it's partially written) - debug("Failed to save attachment %s: %s", saved_file.get_path(), error.message); - - try { - saved_file.delete(); - } catch (Error delete_error) { - debug("Error attempting to delete partial attachment %s: %s", saved_file.get_path(), - delete_error.message); - } - - try { - Db.Statement remove_stmt = cx.prepare( - "DELETE FROM MessageAttachmentTable WHERE id=?"); - remove_stmt.bind_rowid(0, attachment_id); - - remove_stmt.exec(); - } catch (Error remove_error) { - debug("Error attempting to remove added attachment row for %s: %s", - saved_file.get_path(), remove_error.message); - } - - throw error; - } + set_file_info(target, file_size); } } - internal static void do_delete_attachments(Db.Connection cx, - GLib.File attachments_path, - int64 message_id) + private void update_db(Db.Connection cx, Cancellable? cancellable) throws Error { - Gee.List? attachments = do_list_attachments( - cx, attachments_path, message_id, null - ); - if (attachments == null || attachments.size == 0) - return; + // Update the file size now we know what it is + Db.Statement stmt = cx.prepare(""" + UPDATE MessageAttachmentTable + SET filesize = ? + WHERE id = ? + """); + stmt.bind_int64(0, this.filesize); + stmt.bind_rowid(1, this.attachment_id); - // delete all files - foreach (Geary.Attachment attachment in attachments) { - try { - attachment.file.delete(null); - } catch (Error err) { - debug("Unable to delete file %s: %s", attachment.file.get_path(), err.message); - } + stmt.exec(cancellable); + } + + private GLib.File generate_file(GLib.File attachments_dir) { + return attachments_dir + .get_child(this.message_id.to_string()) + .get_child(this.attachment_id.to_string()) + .get_child(this.content_filename ?? NULL_FILE_NAME); + } + + + internal static Gee.List save_attachments(Db.Connection cx, + GLib.File attachments_path, + int64 message_id, + Gee.List attachments, + Cancellable? cancellable) + throws Error { + Gee.List list = new Gee.LinkedList(); + foreach (GMime.Part part in attachments) { + Attachment attachment = new Attachment.from_part(message_id, part); + attachment.save(cx, part, attachments_path, cancellable); + list.add(attachment); + } + return list; + } + + internal static void delete_attachments(Db.Connection cx, + GLib.File attachments_path, + int64 message_id, + Cancellable? cancellable = null) + throws Error { + Gee.List? attachments = list_attachments( + cx, attachments_path, message_id, cancellable + ); + foreach (Attachment attachment in attachments) { + attachment.delete(cx, cancellable); } - // remove all from attachment table + // Ensure they're dead, Jim. Db.Statement stmt = new Db.Statement(cx, """ DELETE FROM MessageAttachmentTable WHERE message_id = ? """); stmt.bind_rowid(0, message_id); - stmt.exec(); } - internal static Geary.Email do_add_attachments(Db.Connection cx, - GLib.File attachments_path, - Geary.Email email, - int64 message_id, - Cancellable? cancellable = null) + // XXX this really should be a member of some internal + // ImapDB.Email class. + internal static void add_attachments(Db.Connection cx, + GLib.File attachments_path, + Geary.Email email, + int64 message_id, + Cancellable? cancellable = null) throws Error { - // Add attachments if available if (email.fields.fulfills(ImapDB.Attachment.REQUIRED_FIELDS)) { - Gee.List? attachments = do_list_attachments( - cx, attachments_path, message_id, cancellable + email.add_attachments( + list_attachments( + cx, attachments_path, message_id, cancellable + ) ); - if (attachments != null) - email.add_attachments(attachments); } - - return email; } - private static Gee.List? - do_list_attachments(Db.Connection cx, - GLib.File attachments_path, - int64 message_id, - Cancellable? cancellable) + internal static Gee.List list_attachments(Db.Connection cx, + GLib.File attachments_path, + int64 message_id, + Cancellable? cancellable) throws Error { Db.Statement stmt = cx.prepare(""" - SELECT id, filename, mime_type, filesize, disposition, content_id, description + SELECT * FROM MessageAttachmentTable WHERE message_id = ? ORDER BY id """); stmt.bind_rowid(0, message_id); - Db.Result results = stmt.exec(cancellable); - if (results.finished) - return null; - - Gee.List list = new Gee.ArrayList(); - do { - string? content_filename = results.string_at(1); - if (content_filename == ImapDB.Attachment.NULL_FILE_NAME) { - // Prior to 0.12, Geary would store the untranslated - // string "none" as the filename when none was - // specified by the MIME content disposition. Check - // for that and clean it up. - content_filename = null; - } - Mime.ContentDisposition disposition = new Mime.ContentDisposition.simple( - Mime.DispositionType.from_int(results.int_at(4))); - list.add( - new ImapDB.Attachment( - message_id, - results.rowid_at(0), - Mime.ContentType.deserialize(results.nonnull_string_at(2)), - results.string_at(5), - results.string_at(6), - disposition, - content_filename, - attachments_path, - results.int64_at(3) - ) - ); - } while (results.next(cancellable)); + Gee.List list = new Gee.LinkedList(); + while (!results.finished) { + list.add(new ImapDB.Attachment.from_row(results, attachments_path)); + results.next(cancellable); + } return list; } diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala index 80288659..1fd86cf2 100644 --- a/src/engine/imap-db/imap-db-database.vala +++ b/src/engine/imap-db/imap-db-database.vala @@ -336,7 +336,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { Mime.DispositionType target_disposition = Mime.DispositionType.UNSPECIFIED; if (message.get_sub_messages().is_empty) target_disposition = Mime.DispositionType.INLINE; - Attachment.do_save_attachments( + Attachment.save_attachments( cx, this.attachments_path, id, @@ -500,7 +500,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { message.get_attachments(); try { - Attachment.do_delete_attachments( + Attachment.delete_attachments( cx, this.attachments_path, message_id ); } catch (Error err) { @@ -511,7 +511,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { // rebuild all try { - Attachment.do_save_attachments( + Attachment.save_attachments( cx, this.attachments_path, message_id, diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index 0bb5085a..d155e4a0 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -1453,7 +1453,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { // TODO: Because this involves saving files, it potentially means holding up access to the // database while they're being written; may want to do this outside of transaction. if (email.fields.fulfills(Attachment.REQUIRED_FIELDS)) { - Attachment.do_save_attachments( + Attachment.save_attachments( cx, this.attachments_path, message_id, @@ -1601,12 +1601,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { "Message %s in folder %s only fulfills %Xh fields (required: %Xh)", location.email_id.to_string(), to_string(), row.fields, required_fields); } - - Geary.Email email = row.to_email(location.email_id); - return Attachment.do_add_attachments( + Geary.Email email = row.to_email(location.email_id); + Attachment.add_attachments( cx, this.attachments_path, email, location.message_id, cancellable ); + return email; } private static string fields_to_columns(Geary.Email.Field fields) { @@ -2034,25 +2034,17 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { // Update attachments if not already in the database if (!fetched_fields.fulfills(Attachment.REQUIRED_FIELDS) && combined_email.fields.fulfills(Attachment.REQUIRED_FIELDS)) { - Attachment.do_save_attachments( - cx, - this.attachments_path, - location.message_id, - combined_email.get_message().get_attachments(), - cancellable + combined_email.add_attachments( + Attachment.save_attachments( + cx, + this.attachments_path, + location.message_id, + combined_email.get_message().get_attachments(), + cancellable + ) ); } - // Must add attachments to the email object after they're saved to - // the database. - Attachment.do_add_attachments( - cx, - this.attachments_path, - combined_email, - location.message_id, - cancellable - ); - Geary.Email.Field new_fields; do_merge_message_row(cx, row, out new_fields, out updated_contacts, ref new_unread_count, cancellable); diff --git a/src/engine/imap-db/imap-db-gc.vala b/src/engine/imap-db/imap-db-gc.vala index 36c7870b..004dd7c7 100644 --- a/src/engine/imap-db/imap-db-gc.vala +++ b/src/engine/imap-db/imap-db-gc.vala @@ -418,29 +418,11 @@ private class Geary.ImapDB.GC { // // Fetch all on-disk attachments for this message // - - Gee.ArrayList attachment_files = new Gee.ArrayList(); - - stmt = cx.prepare(""" - SELECT id, filename - FROM MessageAttachmentTable - WHERE message_id = ? - """); - stmt.bind_rowid(0, message_id); - - result = stmt.exec(cancellable); - while (!result.finished) { - File file = Attachment.generate_file( - this.db.attachments_path, - message_id, - result.rowid_for("id"), - result.string_for("filename") - ); - attachment_files.add(file); - result.next(cancellable); - } - + Gee.List attachments = Attachment.list_attachments( + cx, this.db.attachments_path, message_id, cancellable + ); + // // Delete from search table // @@ -483,17 +465,16 @@ private class Geary.ImapDB.GC { // commits without error and the attachment files can be deleted without being // referenced by the database, in a way that's resumable. // - - foreach (File attachment_file in attachment_files) { + + foreach (Attachment attachment in attachments) { stmt = cx.prepare(""" INSERT INTO DeleteAttachmentFileTable (filename) VALUES (?) """); - stmt.bind_string(0, attachment_file.get_path()); - + stmt.bind_string(0, attachment.file.get_path()); stmt.exec(cancellable); } - + // // Increment the reap count since last vacuum // diff --git a/src/engine/util/util-stream.vala b/src/engine/util/util-stream.vala index 3ac1839b..dc3e6b83 100644 --- a/src/engine/util/util-stream.vala +++ b/src/engine/util/util-stream.vala @@ -114,4 +114,61 @@ namespace Geary.Stream { } } + + /** + * Adaptor from a GMime stream to a GLib OutputStream. + */ + public class MimeOutputStream : GMime.Stream { + + GLib.OutputStream dest; + int64 written = 0; + + + public MimeOutputStream(GLib.OutputStream dest) { + this.dest = dest; + } + + public override int64 length() { + // This is a bit of a kludge, but we use it in + // ImapDB.Attachment + return this.written; + } + + public override ssize_t write(string buf, size_t len) { + ssize_t ret = -1; + try { + ret = this.dest.write(buf.data[0:len]); + this.written += len; + } catch (IOError err) { + // Oh well + } + return ret; + } + + public override int close() { + int ret = -1; + try { + ret = this.dest.close() ? 0 : -1; + } catch (IOError err) { + // Oh well + } + return ret; + } + + public override int flush () { + int ret = -1; + try { + ret = this.dest.flush() ? 0 : -1; + } catch (Error err) { + // Oh well + } + return ret; + } + + public override bool eos () { + return this.dest.is_closed() || this.dest.is_closing(); + } + } + + } diff --git a/test/engine/api/geary-attachment-test.vala b/test/engine/api/geary-attachment-test.vala index 8127b8d3..40883439 100644 --- a/test/engine/api/geary-attachment-test.vala +++ b/test/engine/api/geary-attachment-test.vala @@ -10,7 +10,6 @@ extern const string _SOURCE_ROOT_DIR; class Geary.AttachmentTest : TestCase { - private const string ATTACHMENT_ID = "test-id"; private const string CONTENT_TYPE = "image/png"; private const string CONTENT_ID = "test-content-id"; private const string CONTENT_DESC = "Mea navis volitans anguillis plena est"; @@ -21,19 +20,19 @@ class Geary.AttachmentTest : TestCase { private Mime.ContentDisposition? content_disposition; private File? file; + private class TestAttachment : Attachment { // A test article - internal TestAttachment(string id, - Mime.ContentType content_type, + internal TestAttachment(Mime.ContentType content_type, string? content_id, string? content_description, Mime.ContentDisposition content_disposition, string? content_filename, - File file, - int64 filesize) { - base(id, content_type, content_id, content_description, - content_disposition, content_filename, file, filesize); + GLib.File file) { + base(content_type, content_id, content_description, + content_disposition, content_filename); + set_file_info(file, 742); } } @@ -76,14 +75,12 @@ class Geary.AttachmentTest : TestCase { public void get_safe_file_name_with_content_name() throws Error { const string TEST_FILENAME = "test-filename.png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.content_type, CONTENT_ID, CONTENT_DESC, content_disposition, TEST_FILENAME, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(null, (obj, ret) => { @@ -97,14 +94,12 @@ class Geary.AttachmentTest : TestCase { const string TEST_FILENAME = "test-filename.jpg"; const string RESULT_FILENAME = "test-filename.jpg.png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.content_type, CONTENT_ID, CONTENT_DESC, content_disposition, TEST_FILENAME, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(null, (obj, ret) => { @@ -118,14 +113,12 @@ class Geary.AttachmentTest : TestCase { const string TEST_FILENAME = "test-filename"; const string RESULT_FILENAME = "test-filename.png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.content_type, CONTENT_ID, CONTENT_DESC, content_disposition, TEST_FILENAME, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(null, (obj, ret) => { @@ -138,14 +131,12 @@ class Geary.AttachmentTest : TestCase { public void get_safe_file_name_with_no_content_name() throws Error { const string RESULT_FILENAME = CONTENT_ID + ".png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.content_type, CONTENT_ID, CONTENT_DESC, content_disposition, null, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(null, (obj, ret) => { @@ -156,16 +147,14 @@ class Geary.AttachmentTest : TestCase { } public void get_safe_file_name_with_no_content_name_or_id() throws Error { - const string RESULT_FILENAME = ATTACHMENT_ID + ".png"; + const string RESULT_FILENAME = "attachment.png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.content_type, null, CONTENT_DESC, content_disposition, null, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(null, (obj, ret) => { @@ -179,14 +168,12 @@ class Geary.AttachmentTest : TestCase { const string ALT_TEXT = "some text"; const string RESULT_FILENAME = "some text.png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.content_type, null, CONTENT_DESC, content_disposition, null, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(ALT_TEXT, (obj, ret) => { @@ -199,14 +186,12 @@ class Geary.AttachmentTest : TestCase { public void get_safe_file_name_with_default_content_type() throws Error { const string TEST_FILENAME = "test-filename.png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.default_type, CONTENT_ID, CONTENT_DESC, content_disposition, TEST_FILENAME, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(null, (obj, ret) => { @@ -221,14 +206,12 @@ class Geary.AttachmentTest : TestCase { const string TEST_FILENAME = "test-filename.jpg"; const string RESULT_FILENAME = "test-filename.jpg.png"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.default_type, CONTENT_ID, CONTENT_DESC, content_disposition, TEST_FILENAME, - this.file, - 742 + this.file ); test.get_safe_file_name.begin(null, (obj, ret) => { @@ -242,14 +225,12 @@ class Geary.AttachmentTest : TestCase { throws Error { const string TEST_FILENAME = "test-filename.unlikely"; Attachment test = new TestAttachment( - ATTACHMENT_ID, this.default_type, CONTENT_ID, CONTENT_DESC, content_disposition, TEST_FILENAME, - File.new_for_path(TEST_FILENAME), - 742 + File.new_for_path(TEST_FILENAME) ); test.get_safe_file_name.begin(null, (obj, ret) => { diff --git a/test/engine/imap-db/imap-db-attachment-test.vala b/test/engine/imap-db/imap-db-attachment-test.vala index 7600f750..621de774 100644 --- a/test/engine/imap-db/imap-db-attachment-test.vala +++ b/test/engine/imap-db/imap-db-attachment-test.vala @@ -5,17 +5,102 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ +private const string TEXT_ATTACHMENT = "This is an attachment.\n"; + + class Geary.ImapDB.AttachmentTest : TestCase { + public AttachmentTest() { + base("Geary.ImapDB.AttachmentTest"); + add_test("new_from_minimal_mime_part", new_from_minimal_mime_part); + add_test("new_from_complete_mime_part", new_from_complete_mime_part); + add_test("new_from_inline_mime_part", new_from_inline_mime_part); + } + + public void new_from_minimal_mime_part() throws Error { + GMime.Part part = new_part(null, TEXT_ATTACHMENT.data); + part.set_header("Content-Type", ""); + + Attachment test = new Attachment.from_part(1, part); + assert_string( + Geary.Mime.ContentType.DEFAULT_CONTENT_TYPE, + test.content_type.to_string() + ); + assert_null_string(test.content_id, "content_id"); + assert_null_string(test.content_description, "content_description"); + assert_int( + Geary.Mime.DispositionType.UNSPECIFIED, + test.content_disposition.disposition_type, + "content disposition type" + ); + assert_false(test.has_content_filename, "has_content_filename"); + assert_null_string(test.content_filename, "content_filename"); + } + + public void new_from_complete_mime_part() throws Error { + const string TYPE = "text/plain"; + const string ID = "test-id"; + const string DESC = "test description"; + const string NAME = "test.txt"; + + GMime.Part part = new_part(null, TEXT_ATTACHMENT.data); + part.set_content_id(ID); + part.set_content_description(DESC); + part.set_content_disposition( + new GMime.ContentDisposition.from_string( + "attachment; filename=%s".printf(NAME) + ) + ); + + Attachment test = new Attachment.from_part(1, part); + + assert_string(TYPE, test.content_type.to_string()); + assert_string(ID, test.content_id); + assert_string(DESC, test.content_description); + assert_int( + Geary.Mime.DispositionType.ATTACHMENT, + test.content_disposition.disposition_type + ); + assert_true(test.has_content_filename, "has_content_filename"); + assert_string(test.content_filename, NAME, "content_filename"); + } + + public void new_from_inline_mime_part() throws Error { + GMime.Part part = new_part(null, TEXT_ATTACHMENT.data); + part.set_content_disposition( + new GMime.ContentDisposition.from_string("inline") + ); + + Attachment test = new Attachment.from_part(1, part); + + assert_int( + Geary.Mime.DispositionType.INLINE, + test.content_disposition.disposition_type + ); + } + +} + +class Geary.ImapDB.AttachmentIoTest : TestCase { + + + private GLib.File? tmp_dir; private Geary.Db.Database? db; - public AttachmentTest() { - base("Geary.ImapDB.FolderTest"); + public AttachmentIoTest() { + base("Geary.ImapDB.AttachmentIoTest"); add_test("save_minimal_attachment", save_minimal_attachment); + add_test("save_complete_attachment", save_complete_attachment); + add_test("list_attachments", list_attachments); + add_test("delete_attachments", delete_attachments); } public override void set_up() throws Error { + this.tmp_dir = GLib.File.new_for_path( + GLib.DirUtils.make_tmp("geary-impadb-attachment-io-test-XXXXXX") + ); + this.db = new Geary.Db.Database.transient(); this.db.open.begin( Geary.Db.DatabaseFlags.NONE, null, null, @@ -47,43 +132,184 @@ CREATE TABLE MessageAttachmentTable ( public override void tear_down() throws Error { this.db.close(); this.db = null; + + Geary.Files.recursive_delete_async.begin( + this.tmp_dir, + null, + (obj, res) => { async_complete(res); } + ); + Geary.Files.recursive_delete_async.end(async_result()); + this.tmp_dir = null; } public void save_minimal_attachment() throws Error { - GLib.File tmp_dir = GLib.File.new_for_path( - GLib.DirUtils.make_tmp("geary-impadb-foldertest-XXXXXX") - ); + GMime.Part part = new_part(null, TEXT_ATTACHMENT.data); - GMime.DataWrapper body = new GMime.DataWrapper.with_stream( - new GMime.StreamMem.with_buffer(TEXT_ATTACHMENT.data), - GMime.ContentEncoding.DEFAULT - ); - GMime.Part attachment = new GMime.Part.with_type("text", "plain"); - attachment.set_content_object(body); - attachment.encode(GMime.EncodingConstraint.7BIT); - - Gee.List attachments = new Gee.LinkedList(); - attachments.add(attachment); - - Geary.ImapDB.Attachment.do_save_attachments( + Gee.List attachments = Attachment.save_attachments( this.db.get_master_connection(), - tmp_dir, + this.tmp_dir, 1, - attachments, + new Gee.ArrayList.wrap({ part }), null ); + assert_int(1, attachments.size, "No attachment provided"); + + Geary.Attachment attachment = attachments[0]; + assert_non_null(attachment.file, "Attachment file"); + assert_int( + TEXT_ATTACHMENT.data.length, + (int) attachment.filesize, + "Attachment file size" + ); + + uint8[] buf = new uint8[4096]; + size_t len = 0; + attachments[0].file.read().read_all(buf, out len); + assert_string(TEXT_ATTACHMENT, (string) buf[0:len]); + Geary.Db.Result result = this.db.query( "SELECT * FROM MessageAttachmentTable;" ); assert_false(result.finished, "Row not inserted"); - assert_int(1, result.int_for("message_id"), "Message id"); + assert_int(1, result.int_for("message_id"), "Row message id"); + assert_int( + TEXT_ATTACHMENT.data.length, + result.int_for("filesize"), + "Row file size" + ); assert_false(result.next(), "Multiple rows inserted"); - - Geary.Files.recursive_delete_async.begin(tmp_dir); } + public void save_complete_attachment() throws Error { + const string TYPE = "text/plain"; + const string ID = "test-id"; + const string DESCRIPTION = "test description"; + const Geary.Mime.DispositionType DISPOSITION_TYPE = + Geary.Mime.DispositionType.INLINE; + const string FILENAME = "test.txt"; - private const string TEXT_ATTACHMENT = "This is an attachment.\n"; + GMime.Part part = new_part(TYPE, TEXT_ATTACHMENT.data); + part.set_content_id(ID); + part.set_content_description(DESCRIPTION); + part.set_content_disposition( + new GMime.ContentDisposition.from_string( + "inline; filename=%s;".printf(FILENAME) + )); + + Gee.List attachments = Attachment.save_attachments( + this.db.get_master_connection(), + this.tmp_dir, + 1, + new Gee.ArrayList.wrap({ part }), + null + ); + + assert_int(1, attachments.size, "No attachment provided"); + + Geary.Attachment attachment = attachments[0]; + assert_string(TYPE, attachment.content_type.to_string()); + assert_string(ID, attachment.content_id); + assert_string(DESCRIPTION, attachment.content_description); + assert_string(FILENAME, attachment.content_filename); + assert_int( + DISPOSITION_TYPE, + attachment.content_disposition.disposition_type, + "Attachment disposition type" + ); + + uint8[] buf = new uint8[4096]; + size_t len = 0; + attachment.file.read().read_all(buf, out len); + assert_string(TEXT_ATTACHMENT, (string) buf[0:len]); + + Geary.Db.Result result = this.db.query( + "SELECT * FROM MessageAttachmentTable;" + ); + assert_false(result.finished, "Row not inserted"); + assert_int(1, result.int_for("message_id"), "Row message id"); + assert_string(TYPE, result.string_for("mime_type")); + assert_string(ID, result.string_for("content_id")); + assert_string(DESCRIPTION, result.string_for("description")); + assert_int( + DISPOSITION_TYPE, + result.int_for("disposition"), + "Row disposition type" + ); + assert_string(FILENAME, result.string_for("filename")); + assert_false(result.next(), "Multiple rows inserted"); + + } + + public void list_attachments() throws Error { + this.db.exec(""" +INSERT INTO MessageAttachmentTable ( message_id, mime_type ) +VALUES (1, 'text/plain'); +"""); + this.db.exec(""" +INSERT INTO MessageAttachmentTable ( message_id, mime_type ) +VALUES (2, 'text/plain'); +"""); + + Gee.List loaded = Attachment.list_attachments( + this.db.get_master_connection(), + GLib.File.new_for_path("/tmp"), + 1, + null + ); + + assert_int(1, loaded.size, "Expected one row loaded"); + assert_int(1, (int) loaded[0].message_id, "Unexpected message id"); + } + + public void delete_attachments() throws Error { + GMime.Part part = new_part(null, TEXT_ATTACHMENT.data); + + Gee.List attachments = Attachment.save_attachments( + this.db.get_master_connection(), + this.tmp_dir, + 1, + new Gee.ArrayList.wrap({ part }), + null + ); + + assert_true(attachments[0].file.query_exists(null), + "Attachment not saved to disk"); + + this.db.exec(""" +INSERT INTO MessageAttachmentTable ( message_id, mime_type ) +VALUES (2, 'text/plain'); +"""); + + Attachment.delete_attachments( + this.db.get_master_connection(), this.tmp_dir, 1, null + ); + + Geary.Db.Result result = this.db.query( + "SELECT * FROM MessageAttachmentTable;" + ); + assert_false(result.finished); + assert_int(2, result.int_for("message_id"), "Unexpected message_id"); + assert_false(result.next(), "Attachment not deleted from db"); + + assert_false(attachments[0].file.query_exists(null), + "Attachment not deleted from disk"); + } } + +private GMime.Part new_part(string? mime_type, + uint8[] body, + GMime.ContentEncoding encoding = GMime.ContentEncoding.DEFAULT) { + GMime.Part part = new GMime.Part(); + if (mime_type != null) { + part.set_content_type(new GMime.ContentType.from_string(mime_type)); + } + GMime.DataWrapper body_wrapper = new GMime.DataWrapper.with_stream( + new GMime.StreamMem.with_buffer(body), + GMime.ContentEncoding.DEFAULT + ); + part.set_content_object(body_wrapper); + part.encode(GMime.EncodingConstraint.7BIT); + return part; +} \ No newline at end of file diff --git a/test/test-engine.vala b/test/test-engine.vala index 89bf5d14..a6d60f1a 100644 --- a/test/test-engine.vala +++ b/test/test-engine.vala @@ -37,6 +37,7 @@ int main(string[] args) { engine.add_suite(new Geary.Imap.CreateCommandTest().get_suite()); engine.add_suite(new Geary.Imap.NamespaceResponseTest().get_suite()); engine.add_suite(new Geary.ImapDB.AttachmentTest().get_suite()); + engine.add_suite(new Geary.ImapDB.AttachmentIoTest().get_suite()); engine.add_suite(new Geary.ImapDB.DatabaseTest().get_suite()); engine.add_suite(new Geary.ImapEngine.AccountProcessorTest().get_suite()); engine.add_suite(new Geary.Inet.Test().get_suite());