[geary/wip/726281-text-attachment-crlf: 8/13] Support default content types for both displayed and attached entities.

[Michael Gratton <mjog src gnome org>]

commit a2a95686b47f1c0df304d1ebdd398d7464cbe4c5
Author: Michael James Gratton <mike vee net>
Date:   Wed May 9 17:29:30 2018 +1000

    Support default content types for both displayed and attached entities.
    
    This replaces the const ContentType.DEFAULT_CONTENT_TYPE string and
    related code with static defaults for both displayed and attached MIME
    entities.
    
    * src/engine/mime/mime-content-type.vala (ContentType): Replace
      DEFAULT_CONTENT_TYPE const with static DISPLAY_DEFAULT and
      ATTACHMENT_DEFAULT static instances. Remove is_default() since that no
      longer makes sense. Update call sites and unit tests.
    
    * src/engine/mime/mime-content-parameters.vala (ContentParameters): Make
      the class immutable so it is safe to be used as a component of the
      static ContentType DISPLAY_DEFAULT and ATTACHMENT_DEFAULT members.

 src/engine/api/geary-attachment.vala             |    4 +-
 src/engine/imap-db/imap-db-attachment.vala       |    2 +-
 src/engine/mime/mime-content-parameters.vala     |   44 ++++++++----------
 src/engine/mime/mime-content-type.vala           |   41 +++++++++++------
 test/engine/api/geary-attachment-test.vala       |    2 +-
 test/engine/imap-db/imap-db-attachment-test.vala |    2 +-
 test/engine/mime-content-type-test.vala          |   53 +++++++++++----------
 7 files changed, 78 insertions(+), 70 deletions(-)
---
diff --git a/src/engine/api/geary-attachment.vala b/src/engine/api/geary-attachment.vala
index b97e837..1bdf764 100644
--- a/src/engine/api/geary-attachment.vala
+++ b/src/engine/api/geary-attachment.vala
@@ -127,12 +127,12 @@ public abstract class Geary.Attachment : BaseObject {
         }
 
         if (name_type == null ||
-            name_type.is_default() ||
+            name_type.is_same(Mime.ContentType.ATTACHMENT_DEFAULT) ||
             !name_type.is_same(mime_type)) {
             // Substitute file name either is of unknown type
             // (e.g. it does not have an extension) or is not the
             // same type as the declared type, so try to fix it.
-            if (mime_type.is_default()) {
+            if (mime_type.is_same(Mime.ContentType.ATTACHMENT_DEFAULT)) {
                 // Declared type is unknown, see if we can guess
                 // it. Don't use GFile.query_info however since
                 // that will attempt to use the filename, which is
diff --git a/src/engine/imap-db/imap-db-attachment.vala b/src/engine/imap-db/imap-db-attachment.vala
index 5897317..eecfd9d 100644
--- a/src/engine/imap-db/imap-db-attachment.vala
+++ b/src/engine/imap-db/imap-db-attachment.vala
@@ -42,7 +42,7 @@ private class Geary.ImapDB.Attachment : Geary.Attachment {
         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);
+            : Mime.ContentType.ATTACHMENT_DEFAULT;
 
         GMime.ContentDisposition? part_disposition = part.get_content_disposition();
         Mime.ContentDisposition disposition = (part_disposition != null)
diff --git a/src/engine/mime/mime-content-parameters.vala b/src/engine/mime/mime-content-parameters.vala
index 8691655..642eed8 100644
--- a/src/engine/mime/mime-content-parameters.vala
+++ b/src/engine/mime/mime-content-parameters.vala
@@ -6,8 +6,9 @@
 
 /**
  * Content parameters (for {@link ContentType} and {@link ContentDisposition}).
+ *
+ * This class is immutable.
  */
-
 public class Geary.Mime.ContentParameters : BaseObject {
     public int size {
         get {
@@ -37,14 +38,28 @@ public class Geary.Mime.ContentParameters : BaseObject {
         if (params != null && params.size > 0)
             Collection.map_set_all<string, string>(this.params, params);
     }
-    
+
+    /**
+     * Create a mapping of content parameters.
+     *
+     * Note that the given params must be a two-dimensional array,
+     * where each element contains a key/value pair.
+     */
+    public ContentParameters.from_array(string[,] params) {
+        for (int i = 0; i < params.length[0]; i++) {
+            this.params.set(params[i,0], params[i,1]);
+        }
+    }
+
     internal ContentParameters.from_gmime(GMime.Param? gmime_param) {
+        Gee.Map<string,string> params = new Gee.HashMap<string,string>();
         while (gmime_param != null) {
-            set_parameter(gmime_param.get_name(), gmime_param.get_value());
+            params.set(gmime_param.get_name(), gmime_param.get_value());
             gmime_param = gmime_param.get_next();
         }
+        this(params);
     }
-    
+
     /**
      * A read-only mapping of parameter attributes (names) and values.
      *
@@ -90,25 +105,4 @@ public class Geary.Mime.ContentParameters : BaseObject {
         return (stored != null) ? (stored == value) : false;
     }
 
-    /**
-     * Add or replace the parameter.
-     *
-     * Returns true if the parameter was added, false, otherwise.
-     */
-    public bool set_parameter(string attribute, string value) {
-        bool added = !params.has_key(attribute);
-        params.set(attribute, value);
-        
-        return added;
-    }
-    
-    /**
-     * Removes the parameter.
-     *
-     * Returns true if the parameter was present.
-     */
-    public bool remove_parameter(string attribute) {
-        return params.unset(attribute);
-    }
 }
-
diff --git a/src/engine/mime/mime-content-type.vala b/src/engine/mime/mime-content-type.vala
index 59d530b..fcaa1cb 100644
--- a/src/engine/mime/mime-content-type.vala
+++ b/src/engine/mime/mime-content-type.vala
@@ -8,8 +8,9 @@
  * A representation of an RFC 2045 MIME Content-Type field.
  *
  * See [[https://tools.ietf.org/html/rfc2045#section-5]]
+ *
+ * This class is immutable.
  */
-
 public class Geary.Mime.ContentType : Geary.BaseObject {
 
     /**
@@ -20,15 +21,32 @@ public class Geary.Mime.ContentType : Geary.BaseObject {
     public const string WILDCARD = "*";
 
     /**
-     * Default Content-Type for unknown or unmarked content.
+     * Default Content-Type for inline, displayed entities.
+     *
+     * This is as specified by RFC 2052 § 5.2.
+     */
+    public static ContentType DISPLAY_DEFAULT;
+
+    /**
+     * Default Content-Type for attached entities.
+     *
+     * Although RFC 2052 § 5.2 specifies US-ASCII as the default, for
+     * attachments assume a binary blob so that users aren't presented
+     * with garbled text editor content and warnings on opening it.
      */
-    public const string DEFAULT_CONTENT_TYPE = "application/octet-stream";
+    public static ContentType ATTACHMENT_DEFAULT;
 
 
     private static Gee.Map<string,string> TYPES_TO_EXTENSIONS =
         new Gee.HashMap<string,string>();
 
     static construct {
+        DISPLAY_DEFAULT = new ContentType(
+            "text", "plain",
+            new ContentParameters.from_array({{"charset", "us-ascii"}})
+        );
+        ATTACHMENT_DEFAULT = new ContentType("application", "octet-stream", null);
+
         // XXX We should be loading file name extension information
         // from /etc/mime.types and/or the XDG Shared MIME-info
         // Database globs2 file, usually located at
@@ -61,8 +79,11 @@ public class Geary.Mime.ContentType : Geary.BaseObject {
 
     /**
      * Attempts to guess the content type for a buffer using GIO sniffing.
+     *
+     * Returns null if it could not be guessed.
      */
-    public static ContentType guess_type(string? file_name, Geary.Memory.Buffer? buf) throws Error {
+    public static ContentType? guess_type(string? file_name, Geary.Memory.Buffer? buf)
+        throws Error {
         string? mime_type = null;
 
         if (file_name != null) {
@@ -89,10 +110,7 @@ public class Geary.Mime.ContentType : Geary.BaseObject {
             mime_type = GLib.ContentType.get_mime_type(glib_type);
         }
 
-        if (Geary.String.is_empty(mime_type)) {
-            mime_type = DEFAULT_CONTENT_TYPE;
-        }
-        return deserialize(mime_type);
+        return !Geary.String.is_empty(mime_type) ? deserialize(mime_type) : null;
     }
 
 
@@ -234,13 +252,6 @@ public class Geary.Mime.ContentType : Geary.BaseObject {
         return is_type(mime_media_type, mime_media_subtype);
     }
 
-    /**
-     * Determines if this type is the same as the default content type.
-     */
-    public bool is_default() {
-        return get_mime_type() == DEFAULT_CONTENT_TYPE;
-    }
-
     public string serialize() {
         StringBuilder builder = new StringBuilder();
         builder.append_printf("%s/%s", media_type, media_subtype);
diff --git a/test/engine/api/geary-attachment-test.vala b/test/engine/api/geary-attachment-test.vala
index 4088343..63ac9fa 100644
--- a/test/engine/api/geary-attachment-test.vala
+++ b/test/engine/api/geary-attachment-test.vala
@@ -62,7 +62,7 @@ class Geary.AttachmentTest : TestCase {
     public override void set_up() {
         try {
             this.content_type = Mime.ContentType.deserialize(CONTENT_TYPE);
-            this.default_type = Mime.ContentType.deserialize(Mime.ContentType.DEFAULT_CONTENT_TYPE);
+            this.default_type = Mime.ContentType.ATTACHMENT_DEFAULT;
             this.content_disposition = new Mime.ContentDisposition("attachment", null);
 
             File source = File.new_for_path(_SOURCE_ROOT_DIR);
diff --git a/test/engine/imap-db/imap-db-attachment-test.vala 
b/test/engine/imap-db/imap-db-attachment-test.vala
index 621de77..ae11db8 100644
--- a/test/engine/imap-db/imap-db-attachment-test.vala
+++ b/test/engine/imap-db/imap-db-attachment-test.vala
@@ -24,7 +24,7 @@ class Geary.ImapDB.AttachmentTest : TestCase {
 
         Attachment test = new Attachment.from_part(1, part);
         assert_string(
-            Geary.Mime.ContentType.DEFAULT_CONTENT_TYPE,
+            Geary.Mime.ContentType.ATTACHMENT_DEFAULT.to_string(),
             test.content_type.to_string()
         );
         assert_null_string(test.content_id, "content_id");
diff --git a/test/engine/mime-content-type-test.vala b/test/engine/mime-content-type-test.vala
index b1bdd4d..4b76600 100644
--- a/test/engine/mime-content-type-test.vala
+++ b/test/engine/mime-content-type-test.vala
@@ -9,14 +9,21 @@ class Geary.Mime.ContentTypeTest : TestCase {
 
     public ContentTypeTest() {
         base("Geary.Mime.ContentTypeTest");
-        add_test("is_default", is_default);
+        add_test("static_defaults", static_defaults);
         add_test("get_file_name_extension", get_file_name_extension);
         add_test("guess_type_from_name", guess_type_from_name);
         add_test("guess_type_from_buf", guess_type_from_buf);
     }
 
-    public void is_default() throws Error {
-        assert(new ContentType("application", "octet-stream", null).is_default());
+    public void static_defaults() throws Error {
+        assert_string(
+            "text/plain; charset=us-ascii",
+            ContentType.DISPLAY_DEFAULT.to_string()
+        );
+        assert_string(
+            "application/octet-stream",
+            ContentType.ATTACHMENT_DEFAULT.to_string()
+        );
     }
 
     public void get_file_name_extension() throws Error {
@@ -25,17 +32,15 @@ class Geary.Mime.ContentTypeTest : TestCase {
     }
 
     public void guess_type_from_name() throws Error {
-        try {
-            assert(ContentType.guess_type("test.png", null).is_type("image", "png"));
-        } catch (Error err) {
-            assert_not_reached();
-        }
-
-        try {
-            assert(ContentType.guess_type("foo.test", null).get_mime_type() == 
ContentType.DEFAULT_CONTENT_TYPE);
-        } catch (Error err) {
-            assert_not_reached();
-        }
+        assert_true(
+            ContentType.guess_type("test.png", null).is_type("image", "png"),
+            "Expected image/png"
+        );
+        assert_true(
+            ContentType.guess_type("foo.test", null)
+            .is_same(ContentType.ATTACHMENT_DEFAULT),
+            "Expected ContentType.ATTACHMENT_DEFAULT"
+        );
     }
 
     public void guess_type_from_buf() throws Error {
@@ -44,17 +49,15 @@ class Geary.Mime.ContentTypeTest : TestCase {
         );
         Memory.ByteBuffer empty = new Memory.ByteBuffer({0x0}, 1);
 
-        try {
-            assert(ContentType.guess_type(null, png).is_type("image", "png"));
-        } catch (Error err) {
-            assert_not_reached();
-        }
-
-        try {
-            assert(ContentType.guess_type(null, empty).get_mime_type() == ContentType.DEFAULT_CONTENT_TYPE);
-        } catch (Error err) {
-            assert_not_reached();
-        }
+        assert_true(
+            ContentType.guess_type(null, png).is_type("image", "png"),
+            "Expected image/png"
+        );
+        assert_true(
+            ContentType.guess_type(null, empty)
+            .is_same(ContentType.ATTACHMENT_DEFAULT),
+            "Expected ContentType.ATTACHMENT_DEFAULT"
+        );
     }
 
 }