[geary/wip/726281-text-attachment-crlf: 7/13] Improve how attachments are saved to the db and disk.



commit 037af0074018870efd1ea620e84729d258a4cee5
Author: Michael James Gratton <mike vee net>
Date:   Sat May 5 12:18:22 2018 +1000

    Improve how attachments are saved to the db and disk.
    
    This mostly aims to make the Geary.Attachment and ImapDB.Attachment
    objects usable more generally for managing attachments, allowing
    these to be instantiated once, persisted, and then reused, rather than
    going through a number of representations (GMime, SQlite, Geary) and
    having to be saved and re-loaded.
    
    * src/engine/api/geary-attachment.vala (Attachment): Remove id property
      and allow both file and filesize properties to be set after instances
      are constructed. Update call sites.
    
    * src/engine/api/geary-email.vala (Email): Remove get_attachment_by_id
      since it unused.
    
    * src/engine/imap-db/imap-db-attachment.vala (Attachment): Chase
      Geary.Attachment API changes, move object-specific persistence code
      into methods on the actual object class itself and modernise a
      bit. Rename static methods to be a bit more terse. Update call sites
      and add unit tests.
    
    * src/engine/imap-db/imap-db-folder.vala (Folder): Rather than saving
      attachments to the db then reloading them to add them to their email
      objects, just instantiate Attachment instances once, save and then add
      them.
    
    * src/engine/imap-db/imap-db-gc.vala (GC): Replace custom SQL with
      existing accessor for listing attachments.
    
    * src/engine/util/util-stream.vala (MimeOutputStream): New adaptor class
      for GMime streams to GIO output streams.

 .../conversation-viewer/conversation-email.vala    |    2 +-
 src/engine/api/geary-attachment.vala               |   38 +-
 src/engine/api/geary-email.vala                    |   19 -
 src/engine/imap-db/imap-db-account.vala            |    6 +-
 src/engine/imap-db/imap-db-attachment.vala         |  456 +++++++++++---------
 src/engine/imap-db/imap-db-database.vala           |    6 +-
 src/engine/imap-db/imap-db-folder.vala             |   32 +-
 src/engine/imap-db/imap-db-gc.vala                 |   35 +--
 src/engine/util/util-stream.vala                   |   57 +++
 test/engine/api/geary-attachment-test.vala         |   51 +--
 test/engine/imap-db/imap-db-attachment-test.vala   |  262 +++++++++++-
 test/test-engine.vala                              |    1 +
 12 files changed, 615 insertions(+), 350 deletions(-)
---
diff --git a/src/client/conversation-viewer/conversation-email.vala 
b/src/client/conversation-viewer/conversation-email.vala
index a7f3e0b..971c453 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 25ad46f..b97e837 100644
--- a/src/engine/api/geary-attachment.vala
+++ b/src/engine/api/geary-attachment.vala
@@ -12,13 +12,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}.
      */
     public Mime.ContentType content_type { get; private set; }
@@ -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 1240150..dccaf75 100644
--- a/src/engine/api/geary-email.vala
+++ b/src/engine/api/geary-email.vala
@@ -373,25 +373,6 @@ public class Geary.Email : BaseObject {
     }
 
     /**
-     * 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.
      *
      * Requires the REQUIRED_FOR_MESSAGE fields be present; else
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index 41b86da..41eac27 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 19f5b0d..5897317 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";
-
-    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);
+    private const string NULL_FILE_NAME = "none";
+
+
+    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());
+    internal Attachment.from_part(int64 message_id, GMime.Part part)
+        throws Error {
+        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);
+
+        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
+            );
+
+        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)
+        );
     }
 
-    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 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;
+        }
+
+        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 static void do_save_attachments(Db.Connection cx,
-                                             GLib.File attachments_path,
-                                             int64 message_id,
-                                             Gee.List<GMime.Part>? attachments,
-                                             Cancellable? cancellable)
+    internal void save(Db.Connection cx,
+                       GMime.Part part,
+                       GLib.File attachments_dir,
+                       Cancellable? cancellable)
         throws Error {
-        // nothing to do if no attachments
-        if (attachments == null || attachments.size == 0)
-            return;
-
-        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);
-
-            // 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;
-
-            // 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;
-
-            // Insert it into the database.
-            Db.Statement stmt = cx.prepare("""
+        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);
-
-            int64 attachment_id = stmt.exec_insert(cancellable);
-            File saved_file = ImapDB.Attachment.generate_file(
-                attachments_path, message_id, attachment_id, filename
+        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);
+
+        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());
+            set_file_info(target, file_size);
+        }
+    }
 
-            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;
-            }
+    private void update_db(Db.Connection cx, Cancellable? cancellable)
+        throws Error {
+        // 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);
+
+        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<Attachment> save_attachments(Db.Connection cx,
+                                                          GLib.File attachments_path,
+                                                          int64 message_id,
+                                                          Gee.List<GMime.Part> attachments,
+                                                          Cancellable? cancellable)
+        throws Error {
+        Gee.List<Attachment> list = new Gee.LinkedList<Attachment>();
+        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 do_delete_attachments(Db.Connection cx,
-                                               GLib.File attachments_path,
-                                               int64 message_id)
+    internal static void delete_attachments(Db.Connection cx,
+                                            GLib.File attachments_path,
+                                            int64 message_id,
+                                            Cancellable? cancellable = null)
         throws Error {
-        Gee.List<Geary.Attachment>? attachments = do_list_attachments(
-            cx, attachments_path, message_id, null
+        Gee.List<Attachment>? attachments = list_attachments(
+            cx, attachments_path, message_id, cancellable
         );
-        if (attachments == null || attachments.size == 0)
-            return;
-
-        // 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);
-            }
+        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<Geary.Attachment>? 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<Geary.Attachment>?
-        do_list_attachments(Db.Connection cx,
-                            GLib.File attachments_path,
-                            int64 message_id,
-                            Cancellable? cancellable)
+    internal static Gee.List<Attachment> 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<Geary.Attachment> list = new Gee.ArrayList<Geary.Attachment>();
-        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<Attachment> list = new Gee.LinkedList<Attachment>();
+        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 8028865..1fd86cf 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 0bb5085..d155e4a 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 36c7870..004dd7c 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<File> attachment_files = new Gee.ArrayList<File>();
-            
-            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<Attachment> 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 3ac1839..dc3e6b8 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 8127b8d..4088343 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 7600f75..621de77 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);
+
+        Gee.List<Attachment> attachments = Attachment.save_attachments(
+            this.db.get_master_connection(),
+            this.tmp_dir,
+            1,
+            new Gee.ArrayList<GMime.Part>.wrap({ part }),
+            null
         );
 
-        GMime.DataWrapper body = new GMime.DataWrapper.with_stream(
-            new GMime.StreamMem.with_buffer(TEXT_ATTACHMENT.data),
-            GMime.ContentEncoding.DEFAULT
+        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"
         );
-        GMime.Part attachment = new GMime.Part.with_type("text", "plain");
-        attachment.set_content_object(body);
-        attachment.encode(GMime.EncodingConstraint.7BIT);
 
-        Gee.List<GMime.Part> attachments = new Gee.LinkedList<GMime.Part>();
-        attachments.add(attachment);
+        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.ImapDB.Attachment.do_save_attachments(
+        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_int(
+            TEXT_ATTACHMENT.data.length,
+            result.int_for("filesize"),
+            "Row file size"
+        );
+        assert_false(result.next(), "Multiple rows inserted");
+    }
+
+    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";
+
+        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<Attachment> attachments = Attachment.save_attachments(
             this.db.get_master_connection(),
-            tmp_dir,
+            this.tmp_dir,
             1,
-            attachments,
+            new Gee.ArrayList<GMime.Part>.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"), "Message id");
+        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");
 
-        Geary.Files.recursive_delete_async.begin(tmp_dir);
     }
 
+    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<Attachment> loaded = Attachment.list_attachments(
+            this.db.get_master_connection(),
+            GLib.File.new_for_path("/tmp"),
+            1,
+            null
+        );
 
-    private const string TEXT_ATTACHMENT = "This is an attachment.\n";
+        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<Attachment> attachments = Attachment.save_attachments(
+            this.db.get_master_connection(),
+            this.tmp_dir,
+            1,
+            new Gee.ArrayList<GMime.Part>.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 89bf5d1..a6d60f1 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());


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]