Cleaned up Geary.Attachments implementation and interface

While working on #7345, I grew dissatisfied with how Geary.Attachment
was implemented.  It knew too much about imap-db's internal workings,
so I've broken it up a little bit.  It's now an abstract base class
completed by ImapDB.Attachment, a private implementation class.
This corresponds with the coding patterns used throughout the
Geary API.
This commit is contained in:
Jim Nelson 2013-09-09 15:05:22 -07:00
parent e09a056e8d
commit ddc6403aad
9 changed files with 104 additions and 56 deletions

View file

@ -166,6 +166,7 @@ engine/imap/transport/imap-deserializer.vala
engine/imap/transport/imap-serializer.vala
engine/imap-db/imap-db-account.vala
engine/imap-db/imap-db-attachment.vala
engine/imap-db/imap-db-contact.vala
engine/imap-db/imap-db-database.vala
engine/imap-db/imap-db-email-identifier.vala

View file

@ -904,12 +904,8 @@ public class ComposerWindow : Gtk.Window {
private void check_pending_attachments() {
if (pending_attachments != null) {
Gee.Set<string> filenames = new Gee.HashSet<string>();
foreach (File file in attachment_files)
filenames.add(file.get_path());
foreach (Geary.Attachment attachment in pending_attachments) {
if (!filenames.contains(attachment.filepath)) {
if (!attachment_files.contains(attachment.file)) {
pending_attachments_button.show();
return;
}
@ -990,11 +986,8 @@ public class ComposerWindow : Gtk.Window {
}
private void add_attachments(Gee.List<Geary.Attachment> attachments, bool alert_errors = true) {
foreach(Geary.Attachment attachment in attachments) {
File? attachment_file = File.new_for_path(attachment.filepath);
if (attachment_file != null)
add_attachment(attachment_file, alert_errors);
}
foreach(Geary.Attachment attachment in attachments)
add_attachment(attachment.file, alert_errors);
}
private void remove_attachment(File file, Gtk.Box box) {

View file

@ -1392,7 +1392,7 @@ public class GearyController : Geary.BaseObject {
private void on_open_attachment(Geary.Attachment attachment) {
if (GearyApplication.instance.config.ask_open_attachment) {
QuestionDialog ask_to_open = new QuestionDialog.with_checkbox(main_window,
_("Are you sure you want to open \"%s\"?").printf(attachment.filename),
_("Are you sure you want to open \"%s\"?").printf(attachment.file.get_basename()),
_("Attachments may cause damage to your system if opened. Only open files from trusted sources."),
Stock._OPEN_BUTTON, Stock._CANCEL, _("Don't _ask me again"), false);
if (ask_to_open.run() != Gtk.ResponseType.OK)
@ -1402,7 +1402,7 @@ public class GearyController : Geary.BaseObject {
GearyApplication.instance.config.ask_open_attachment = !ask_to_open.is_checked;
}
open_uri("file://" + attachment.filepath);
open_uri(attachment.file.get_uri());
}
private bool do_overwrite_confirmation(File to_overwrite) {
@ -1434,7 +1434,7 @@ public class GearyController : Geary.BaseObject {
if (last_save_directory != null)
dialog.set_current_folder(last_save_directory.get_path());
if (attachments.size == 1) {
dialog.set_current_name(attachments[0].filename);
dialog.set_current_name(attachments[0].file.get_basename());
dialog.set_do_overwrite_confirmation(true);
// use custom overwrite confirmation so it looks consistent whether one or many
// attachments are being saved
@ -1458,8 +1458,8 @@ public class GearyController : Geary.BaseObject {
// Save each one, checking for overwrite only if multiple attachments are being written
foreach (Geary.Attachment attachment in attachments) {
File source_file = File.new_for_path(attachment.filepath);
File dest_file = (attachments.size == 1) ? destination : destination.get_child(attachment.filename);
File source_file = attachment.file;
File dest_file = (attachments.size == 1) ? destination : destination.get_child(attachment.file.get_basename());
if (attachments.size > 1 && dest_file.query_exists() && !do_overwrite_confirmation(dest_file))
return;

View file

@ -826,7 +826,7 @@ public class ConversationViewer : Gtk.Box {
return null;
try {
return email.get_attachment(int64.parse(element.get_attribute("data-attachment-id")));
return email.get_attachment(element.get_attribute("data-attachment-id"));
} catch (Geary.EngineError err) {
return null;
}
@ -1260,7 +1260,7 @@ public class ConversationViewer : Gtk.Box {
}
private void on_attachment_clicked_self(WebKit.DOM.Element element) {
int64 attachment_id = int64.parse(element.get_attribute("data-attachment-id"));
string attachment_id = element.get_attribute("data-attachment-id");
Geary.Email? email = get_email_from_element(element);
if (email == null)
return;
@ -1754,18 +1754,17 @@ public class ConversationViewer : Gtk.Box {
}
// Generate the attachment table.
WebKit.DOM.HTMLElement attachment_table = Util.DOM.clone_node(attachment_template);
string filename = Geary.String.is_empty_or_whitespace(attachment.filename) ?
_("none") : attachment.filename;
string filename = !attachment.has_supplied_filename ? _("none") : attachment.file.get_basename();
Util.DOM.select(attachment_table, ".info .filename")
.set_inner_text(filename);
Util.DOM.select(attachment_table, ".info .filesize")
.set_inner_text(Files.get_filesize_as_string(attachment.filesize));
attachment_table.set_attribute("data-attachment-id", "%s".printf(attachment.id.to_string()));
attachment_table.set_attribute("data-attachment-id", attachment.id);
// Set the image preview and insert it into the container.
WebKit.DOM.HTMLImageElement img =
Util.DOM.select(attachment_table, ".preview img") as WebKit.DOM.HTMLImageElement;
web_view.set_image_src(img, attachment.mime_type, attachment.filepath, ATTACHMENT_PREVIEW_SIZE);
web_view.set_image_src(img, attachment.mime_type, attachment.file.get_path(), ATTACHMENT_PREVIEW_SIZE);
attachment_container.append_child(attachment_table);
}

View file

@ -4,9 +4,13 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
public class Geary.Attachment : BaseObject {
public const Email.Field REQUIRED_FIELDS = Email.REQUIRED_FOR_MESSAGE;
/**
* An attachment that was a part of an {@link Email}.
*
* @see Email.get_attachment
*/
public abstract class Geary.Attachment : BaseObject {
// NOTE: These values are persisted on disk and should not be modified unless you know what
// you're doing.
public enum Disposition {
@ -43,36 +47,52 @@ public class Geary.Attachment : BaseObject {
}
}
public string? filename { get; private set; }
public string filepath { get; private set; }
/**
* An identifier that can be used to locate the {@link Attachment} in an {@link Email}.
*
* @see Email.get_attachment
*/
public string id { get; private set; }
/**
* Returns true if the originating {@link Email} supplied a filename for the {@link Attachment}.
*
* Since all files must have a name, one is supplied for the Attachment by Geary if this is
* false. This is merely to indicate how the filename should be displayed, since Geary's will
* be an untranslated "none".
*/
public bool has_supplied_filename { get; private set; }
/**
* The on-disk File of the {@link Attachment}.
*/
public File file { get; private set; }
/**
* The MIME type of the {@link Attachment}.
*/
public string mime_type { get; private set; }
/**
* The file size (in bytes) if the {@link file}.
*/
public int64 filesize { get; private set; }
public int64 id { get; private set; }
/**
* The {@link Disposition} of the attachment, as specified by the {@link Email}.
*
* See [[https://tools.ietf.org/html/rfc2183]]
*/
public Disposition disposition { get; private set; }
// TODO: Move some of this into ImapDB.Attachment
internal Attachment(File data_dir, string? filename, string mime_type, int64 filesize,
int64 message_id, int64 attachment_id, Disposition disposition) {
this.filename = filename;
protected Attachment(string id, File file, bool has_supplied_filename, string mime_type, int64 filesize,
Disposition disposition) {
this.id = id;
this.file = file;
this.has_supplied_filename = has_supplied_filename;
this.mime_type = mime_type;
this.filesize = filesize;
this.filepath = get_path(data_dir, message_id, attachment_id, filename);
this.id = attachment_id;
this.disposition = disposition;
}
// TODO: Move this into ImapDB.Attachment
internal static string get_path(File data_dir, int64 message_id, int64 attachment_id,
string? filename) {
// "none" should not be translated, or the user will be unable to retrieve their
// attachments with no filenames after changing their language.
return "%s/attachments/%s/%s/%s".printf(data_dir.get_path(), message_id.to_string(),
attachment_id.to_string(), filename ?? "none");
}
internal static File get_attachments_dir(File data_dir) {
return data_dir.get_child("attachments");
}
}

View file

@ -253,7 +253,7 @@ public class Geary.Email : BaseObject {
public string get_searchable_attachment_list() {
StringBuilder search = new StringBuilder();
foreach (Geary.Attachment attachment in attachments) {
search.append(attachment.filename);
search.append(attachment.file.get_basename());
search.append("\n");
}
return search.str;
@ -279,7 +279,7 @@ public class Geary.Email : BaseObject {
* Requires the REQUIRED_FOR_MESSAGE fields be present; else
* EngineError.INCOMPLETE_MESSAGE is thrown.
*/
public Geary.Attachment? get_attachment(int64 attachment_id) throws EngineError {
public Geary.Attachment? get_attachment(string attachment_id) throws EngineError {
if (!fields.fulfills(REQUIRED_FOR_MESSAGE))
throw new EngineError.INCOMPLETE_MESSAGE("Parsed email requires HEADER and BODY");

View file

@ -58,7 +58,7 @@ private class Geary.ImapDB.Account : BaseObject {
public static void get_imap_db_storage_locations(File user_data_dir, out File db_file,
out File attachments_dir) {
db_file = ImapDB.Database.get_db_file(user_data_dir);
attachments_dir = Geary.Attachment.get_attachments_dir(user_data_dir);
attachments_dir = ImapDB.Attachment.get_attachments_dir(user_data_dir);
}
public async void open_async(File user_data_dir, File schema_dir, Cancellable? cancellable)

View file

@ -0,0 +1,35 @@
/* Copyright 2013 Yorba Foundation
*
* 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;
private const string ATTACHMENTS_DIR = "attachments";
protected Attachment(File data_dir, string? filename, string mime_type, int64 filesize,
int64 message_id, int64 attachment_id, Geary.Attachment.Disposition disposition) {
base (generate_id(attachment_id),generate_file(data_dir, message_id, attachment_id, filename),
!String.is_empty(filename), mime_type, filesize, disposition);
}
private static string generate_id(int64 attachment_id) {
return "imap-db:%s".printf(attachment_id.to_string());
}
public static File generate_file(File data_dir, int64 message_id, int64 attachment_id,
string? filename) {
// "none" should not be translated, or the user will be unable to retrieve their
// attachments with no filenames after changing their language.
return get_attachments_dir(data_dir)
.get_child(message_id.to_string())
.get_child(attachment_id.to_string())
.get_child(filename ?? "none");
}
public static File get_attachments_dir(File data_dir) {
return data_dir.get_child(ATTACHMENTS_DIR);
}
}

View file

@ -1329,7 +1329,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
internal static Geary.Email do_add_attachments(Db.Connection cx, Geary.Email email,
int64 message_id, Cancellable? cancellable = null) throws Error {
// Add attachments if available
if (email.fields.fulfills(Geary.Attachment.REQUIRED_FIELDS)) {
if (email.fields.fulfills(ImapDB.Attachment.REQUIRED_FIELDS)) {
Gee.List<Geary.Attachment>? attachments = do_list_attachments(cx, message_id,
cancellable);
if (attachments != null)
@ -1772,9 +1772,9 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
Gee.List<Geary.Attachment> list = new Gee.ArrayList<Geary.Attachment>();
do {
list.add(new Geary.Attachment(cx.database.db_file.get_parent(), results.string_at(1),
list.add(new ImapDB.Attachment(cx.database.db_file.get_parent(), results.string_at(1),
results.string_at(2), results.int64_at(3), message_id, results.rowid_at(0),
Attachment.Disposition.from_int(results.int_at(4))));
Geary.Attachment.Disposition.from_int(results.int_at(4))));
} while (results.next(cancellable));
return list;
@ -1814,12 +1814,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_string(1, filename);
stmt.bind_string(2, mime_type);
stmt.bind_uint(3, filesize);
stmt.bind_int(4, Attachment.Disposition.from_string(disposition));
stmt.bind_int(4, Geary.Attachment.Disposition.from_string(disposition));
int64 attachment_id = stmt.exec_insert(cancellable);
File saved_file = File.new_for_path(Attachment.get_path(db.db_file.get_parent(), message_id,
attachment_id, filename));
File saved_file = ImapDB.Attachment.generate_file(db.db_file.get_parent(), message_id,
attachment_id, filename);
debug("Saving attachment to %s", saved_file.get_path());