[geary/wip/fix-date-handling] Clean up protocol date handling substantially



commit c81eeecb2be8e4983d11097e47b2aab1a313f15d
Author: Michael Gratton <mike vee net>
Date:   Sun Mar 3 15:51:35 2019 +1100

    Clean up protocol date handling substantially
    
    In RFC822.Date, don't use duplicate time_t and GLib.DateTime properties
    -- just use the latter, fix formatting of half-hour time zones, fix
    where the that class refers to ISO 8601 instead of RFC822 strings since
    that is just plain wrong, and finally when parsing an RFC 822 string,
    take note of the timezone offset and store that in the DateTime object,
    so it is round-tripped correctly.
    
    Stop passing time_t around everywhere else, just use the UNIX time from
    the DateTimes we store for protocol objects anyway, so the time zone is
    obvious.
    
    Add unit tests.

 po/POTFILES.in                                     |  1 -
 src/engine/imap-db/imap-db-database.vala           |  9 ++--
 src/engine/imap-db/imap-db-message-row.vala        | 12 ++---
 .../replay-ops/imap-engine-fetch-email.vala        | 27 ++++++----
 src/engine/imap/message/imap-internal-date.vala    |  7 ---
 src/engine/meson.build                             |  1 -
 src/engine/rfc822/rfc822-message-data.vala         | 61 ++++++++++++----------
 src/engine/util/util-time.vala                     | 30 -----------
 test/engine/rfc822-message-data-test.vala          | 52 ++++++++++++++++++
 test/test-case.vala                                |  8 +++
 10 files changed, 121 insertions(+), 87 deletions(-)
---
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 92f7e71e..835ebb31 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -395,7 +395,6 @@ src/engine/util/util-scheduler.vala
 src/engine/util/util-stream.vala
 src/engine/util/util-string.vala
 src/engine/util/util-synchronization.vala
-src/engine/util/util-time.vala
 src/engine/util/util-timeout-manager.vala
 src/engine/util/util-trillian.vala
 src/mailer/main.vala
diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala
index 210800b3..63244b95 100644
--- a/src/engine/imap-db/imap-db-database.vala
+++ b/src/engine/imap-db/imap-db-database.vala
@@ -300,12 +300,15 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
                     string? internaldate = select.string_at(1);
 
                     try {
-                        time_t as_time_t = (internaldate != null ?
-                            Geary.Imap.InternalDate.decode(internaldate).to_time_t() : -1);
+                        int64 as_time_t = (
+                            internaldate != null
+                            ? Imap.InternalDate.decode(internaldate).value.to_unix()
+                            : -1
+                        );
 
                         Db.Statement update = cx.prepare(
                             "UPDATE MessageTable SET internaldate_time_t=? WHERE id=?");
-                        update.bind_int64(0, (int64) as_time_t);
+                        update.bind_int64(0, as_time_t);
                         update.bind_rowid(1, id);
                         update.exec();
                     } catch (Error e) {
diff --git a/src/engine/imap-db/imap-db-message-row.vala b/src/engine/imap-db/imap-db-message-row.vala
index 6ed964ab..36a4699b 100644
--- a/src/engine/imap-db/imap-db-message-row.vala
+++ b/src/engine/imap-db/imap-db-message-row.vala
@@ -9,7 +9,7 @@ private class Geary.ImapDB.MessageRow {
     public Geary.Email.Field fields { get; set; default = Geary.Email.Field.NONE; }
 
     public string? date { get; set; default = null; }
-    public time_t date_time_t { get; set; default = -1; }
+    public int64 date_time_t { get; set; default = -1; }
 
     public string? from { get; set; default = null; }
     public string? sender { get; set; default = null; }
@@ -33,7 +33,7 @@ private class Geary.ImapDB.MessageRow {
 
     public string? email_flags { get; set; default = null; }
     public string? internaldate { get; set; default = null; }
-    public time_t internaldate_time_t { get; set; default = -1; }
+    public int64 internaldate_time_t { get; set; default = -1; }
     public int64 rfc822_size { get; set; default = -1; }
 
     public MessageRow() {
@@ -54,7 +54,7 @@ private class Geary.ImapDB.MessageRow {
 
         if (fields.is_all_set(Geary.Email.Field.DATE)) {
             date = results.string_for("date_field");
-            date_time_t = (time_t) results.int64_for("date_time_t");
+            date_time_t = results.int64_for("date_time_t");
         }
 
         if (fields.is_all_set(Geary.Email.Field.ORIGINATORS)) {
@@ -92,7 +92,7 @@ private class Geary.ImapDB.MessageRow {
 
         if (fields.is_all_set(Geary.Email.Field.PROPERTIES)) {
             internaldate = results.string_for("internaldate");
-            internaldate_time_t = (time_t) results.int64_for("internaldate_time_t");
+            internaldate_time_t = results.int64_for("internaldate_time_t");
             rfc822_size = results.int64_for("rfc822_size");
         }
     }
@@ -190,7 +190,7 @@ private class Geary.ImapDB.MessageRow {
 
         if (email.fields.is_all_set(Geary.Email.Field.DATE)) {
             date = (email.date != null) ? email.date.original : null;
-            date_time_t = (email.date != null) ? email.date.to_time_t() : -1;
+            date_time_t = (email.date != null) ? email.date.value.to_unix() : -1;
 
             fields = fields.set(Geary.Email.Field.DATE);
         }
@@ -253,7 +253,7 @@ private class Geary.ImapDB.MessageRow {
         if (email.fields.is_all_set(Geary.Email.Field.PROPERTIES)) {
             Geary.Imap.EmailProperties? imap_properties = (Geary.Imap.EmailProperties) email.properties;
             internaldate = (imap_properties != null) ? imap_properties.internaldate.serialize() : null;
-            internaldate_time_t = (imap_properties != null) ? imap_properties.internaldate.to_time_t() : -1;
+            internaldate_time_t = (imap_properties != null) ? imap_properties.internaldate.value.to_unix() : 
-1;
             rfc822_size = (imap_properties != null) ? imap_properties.rfc822_size.value : -1;
 
             fields = fields.set(Geary.Email.Field.PROPERTIES);
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala 
b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala
index 3edf4347..79688792 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala
@@ -46,28 +46,33 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation
             this.uid = yield engine.local_folder.get_uid_async(
                 this.id, NONE, this.cancellable
             );
-            return CONTINUE;
+            return Status.CONTINUE;
         }
 
+        bool local_only = flags.is_all_set(Folder.ListFlags.LOCAL_ONLY);
         Geary.Email? email = null;
         try {
-            email = yield engine.local_folder.fetch_email_async(id, required_fields,
-                ImapDB.Folder.ListFlags.PARTIAL_OK, cancellable);
-        } catch (Error err) {
-            // If NOT_FOUND or INCOMPLETE_MESSAGE, then fall through, otherwise return to sender
-            if (!(err is Geary.EngineError.NOT_FOUND) && !(err is Geary.EngineError.INCOMPLETE_MESSAGE))
+            email = yield engine.local_folder.fetch_email_async(
+                id,
+                required_fields,
+                ImapDB.Folder.ListFlags.PARTIAL_OK,
+                cancellable
+            );
+        } catch (Geary.EngineError.NOT_FOUND err) {
+            if (local_only) {
                 throw err;
+            }
         }
 
         // If returned in full, done
-        if (email.fields.fulfills(required_fields)) {
+        if (email != null && email.fields.fulfills(required_fields)) {
             this.email = email;
             this.remaining_fields = Email.Field.NONE;
             return ReplayOperation.Status.COMPLETED;
-        }
-
-        // If local only, ensure the email has all required fields
-        if (flags.is_all_set(Folder.ListFlags.LOCAL_ONLY)) {
+        } else if (local_only) {
+            // Didn't have an email that fulfills the reqs, but the
+            // caller didn't want to go to the remote, so let them
+            // know
             throw new EngineError.INCOMPLETE_MESSAGE(
                 "Email %s with fields %Xh locally incomplete %s",
                 id.to_string(),
diff --git a/src/engine/imap/message/imap-internal-date.vala b/src/engine/imap/message/imap-internal-date.vala
index ad01e6d2..e90e23d0 100644
--- a/src/engine/imap/message/imap-internal-date.vala
+++ b/src/engine/imap/message/imap-internal-date.vala
@@ -91,13 +91,6 @@ public class Geary.Imap.InternalDate : Geary.MessageData.AbstractMessageData, Ge
         return new InternalDate(internaldate, datetime);
     }
 
-    /**
-     * Returns the value of the InternalDate as a time_t representation.
-     */
-    public time_t to_time_t () {
-        return Time.datetime_to_time_t(this.value);
-    }
-
     /**
      * Returns the {@link InternalDate} as a {@link Parameter}.
      */
diff --git a/src/engine/meson.build b/src/engine/meson.build
index 3f5bfad9..f78a47da 100644
--- a/src/engine/meson.build
+++ b/src/engine/meson.build
@@ -316,7 +316,6 @@ geary_engine_vala_sources = files(
   'util/util-stream.vala',
   'util/util-string.vala',
   'util/util-synchronization.vala',
-  'util/util-time.vala',
   'util/util-timeout-manager.vala',
   'util/util-trillian.vala',
 )
diff --git a/src/engine/rfc822/rfc822-message-data.vala b/src/engine/rfc822/rfc822-message-data.vala
index 97be9acd..e2fd790b 100644
--- a/src/engine/rfc822/rfc822-message-data.vala
+++ b/src/engine/rfc822/rfc822-message-data.vala
@@ -169,59 +169,64 @@ public class Geary.RFC822.MessageIDList : Geary.MessageData.AbstractMessageData,
 public class Geary.RFC822.Date : Geary.RFC822.MessageData, Geary.MessageData.AbstractMessageData,
     Gee.Hashable<Geary.RFC822.Date> {
 
-    private time_t as_time_t;
-
     public string? original { get; private set; }
     public DateTime value { get; private set; }
 
-    public Date(string iso8601) throws ImapError {
-        this.as_time_t = GMime.utils_header_decode_date(iso8601, null);
-        if (as_time_t == 0)
+    public Date(string rfc822) throws ImapError {
+        int offset = 0;
+        int64 time_t_utc = GMime.utils_header_decode_date(rfc822, out offset);
+        if (time_t_utc == 0)
             throw new ImapError.PARSE_ERROR(
-                "Unable to parse \"%s\": Not ISO-8601 date", iso8601
+                "Unable to parse \"%s\": Not ISO-8601 date", rfc822
             );
 
-        DateTime? value = new DateTime.from_unix_local(this.as_time_t);
+        DateTime? value = new DateTime.from_unix_utc(time_t_utc);
         if (value == null) {
             throw new ImapError.PARSE_ERROR(
-                "Unable to parse \"%s\": Outside supported range", iso8601
+                "Unable to parse \"%s\": Outside supported range", rfc822
             );
         }
         this.value = value;
-        this.original = iso8601;
-    }
 
-    public Date.from_date_time(DateTime datetime) {
-        original = null;
-        value = datetime;
-        this.as_time_t = Time.datetime_to_time_t(datetime);
+        if (offset != 0) {
+            this.value = value.to_timezone(
+                new GLib.TimeZone("%+05d".printf(offset))
+            );
+        }
+
+        this.original = rfc822;
     }
 
-    /**
-     * Returns the {@link Date} in ISO-8601 format.
-     */
-    public string to_iso_8601() {
-        // Although GMime documents its conversion methods as requiring the tz offset in hours,
-        // it appears the number is handed directly to the string (i.e. an offset of -7 becomes
-        // "-0007", whereas we want "-0700").
-        return GMime.utils_header_format_date(this.as_time_t,
-            (int) (value.get_utc_offset() / TimeSpan.HOUR) * 100);
+    public Date.from_date_time(DateTime datetime) {
+        this.original = null;
+        this.value = datetime;
     }
 
     /**
-     * Returns {@link Date} as a time_t representation.
+     * Returns the {@link Date} in RFC 822 format.
      */
-    public time_t to_time_t() {
-        return this.as_time_t;
+    public string to_rfc822_string() {
+        // Although GMime documents its conversion methods as
+        // requiring the tz offset in hours, it appears the number is
+        // handed directly to the string (i.e. an offset of -7:30 becomes
+        // "-0007", whereas we want "-0730").
+        int hours = (int) GLib.Math.floor(value.get_utc_offset() / TimeSpan.HOUR);
+        int minutes = (int) (
+            (value.get_utc_offset() % TimeSpan.HOUR) / (double) TimeSpan.HOUR * 60
+        );
+        return GMime.utils_header_format_date(
+            (time_t) this.value.to_utc().to_unix(),
+            (hours * 100) + minutes
+        );
     }
 
     /**
      * Returns {@link Date} for transmission.
      *
-     * @see to_iso_8601
+     * @see to_rfc822_string
      */
     public virtual string serialize() {
-        return to_iso_8601();
+        return to_rfc822_string();
     }
 
     public virtual bool equal_to(Geary.RFC822.Date other) {
diff --git a/test/engine/rfc822-message-data-test.vala b/test/engine/rfc822-message-data-test.vala
index 5251fd9d..2b849edb 100644
--- a/test/engine/rfc822-message-data-test.vala
+++ b/test/engine/rfc822-message-data-test.vala
@@ -9,6 +9,8 @@ class Geary.RFC822.MessageDataTest : TestCase {
 
     public MessageDataTest() {
         base("Geary.RFC822.MessageDataTest");
+        add_test("date_from_rfc822", date_from_rfc822);
+        add_test("date_to_rfc822", date_to_rfc822);
         add_test("PreviewText.with_header", preview_text_with_header);
     }
 
@@ -40,6 +42,56 @@ class Geary.RFC822.MessageDataTest : TestCase {
         assert_string(HTML_BODY2_EXPECTED, html_preview2.buffer.to_string());
     }
 
+    public void date_from_rfc822() throws GLib.Error {
+        const string FULL_HOUR_TZ = "Thu, 28 Feb 2019 00:00:00 -0100";
+        Date full_hour_tz = new Date(FULL_HOUR_TZ);
+        assert_int64(
+            ((int64) (-1 * 3600)) * 1000 * 1000,
+            full_hour_tz.value.get_utc_offset(),
+            "full_hour_tz.value.get_utc_offset"
+        );
+        assert_int(0, full_hour_tz.value.get_hour(), "full_hour_tz hour");
+        assert_int(0, full_hour_tz.value.get_minute(), "full_hour_tz minute");
+        assert_int(0, full_hour_tz.value.get_second(), "full_hour_tz second");
+        assert_int(28, full_hour_tz.value.get_day_of_month(), "full_hour_tz day");
+        assert_int(2, full_hour_tz.value.get_month(), "full_hour_tz month");
+        assert_int(2019, full_hour_tz.value.get_year(), "full_hour_tz year");
+
+        assert_int64(
+            full_hour_tz.value.to_utc().to_unix(),
+            full_hour_tz.value.to_unix(),
+            "to_unix not UTC"
+        );
+
+        const string HALF_HOUR_TZ = "Thu, 28 Feb 2019 00:00:00 +1030";
+        Date half_hour_tz = new Date(HALF_HOUR_TZ);
+        assert_int64(
+            ((int64) (10.5 * 3600)) * 1000 * 1000,
+            half_hour_tz.value.get_utc_offset()
+        );
+        assert_int(0, half_hour_tz.value.get_hour());
+        assert_int(0, half_hour_tz.value.get_minute());
+        assert_int(0, half_hour_tz.value.get_second());
+        assert_int(28, half_hour_tz.value.get_day_of_month());
+        assert_int(2, half_hour_tz.value.get_month());
+        assert_int(2019, half_hour_tz.value.get_year());
+    }
+
+    public void date_to_rfc822() throws GLib.Error {
+        const string FULL_HOUR_TZ = "Thu, 28 Feb 2019 00:00:00 -0100";
+        Date full_hour_tz = new Date(FULL_HOUR_TZ);
+        assert_string(FULL_HOUR_TZ, full_hour_tz.to_rfc822_string());
+
+        const string HALF_HOUR_TZ = "Thu, 28 Feb 2019 00:00:00 +1030";
+        Date half_hour_tz = new Date(HALF_HOUR_TZ);
+        assert_string(HALF_HOUR_TZ, half_hour_tz.to_rfc822_string());
+
+        const string NEG_HALF_HOUR_TZ = "Thu, 28 Feb 2019 00:00:00 -1030";
+        Date neg_half_hour_tz = new Date(NEG_HALF_HOUR_TZ);
+        assert_string(NEG_HALF_HOUR_TZ, neg_half_hour_tz.to_rfc822_string());
+    }
+
+
     public static string PLAIN_BODY1_HEADERS = "Content-Type: text/plain; 
charset=\"us-ascii\"\r\nContent-Transfer-Encoding: 7bit\r\n";
     public static string PLAIN_BODY1_ENCODED = "-----BEGIN PGP SIGNED MESSAGE-----\r\nHash: 
SHA512\r\n\r\n=============================================================================\r\nFreeBSD-EN-16:11.vmbus
                                          Errata Notice\r\n                                                   
       The FreeBSD Project\r\n\r\nTopic:          Avoid using spin locks for channel message 
locks\r\n\r\nCategory:       core\r\nModule:         vmbus\r\nAnnounced:      2016-08-12\r\nCredits:        
Microsoft OSTC\r\nAffects:        FreeBSD 10.3\r\nCorrected:      2016-06-15 09:52:01 UTC (stable/10, 
10.3-STABLE)\r\n                2016-08-12 04:01:16 UTC (releng/10.3, 10.3-RELEASE-p7)\r\n\r\nFor general 
information regarding FreeBSD Errata Notices and Security\r\nAdvisories, including descriptions of the fields 
above, security\r\nbranches, and the following sections, please 
visit\r\n<URL:https://security.FreeBSD.org/>.\r\n";
     public static string PLAIN_BODY1_EXPECTED = "FreeBSD-EN-16:11.vmbus Errata Notice The FreeBSD Project 
Topic: Avoid using spin locks for channel message locks Category: core Module: vmbus Announced: 2016-08-12 
Credits: Microsoft OSTC Affects: FreeBSD 10.3 Corrected: 2016-06-15 09:52:01 UTC (stable/10, 10.3-STABLE) 
2016-08-12 04:01:16 UTC (releng/10.3, 10.3-RELEASE-p7) For general information regarding FreeBSD Errata 
Notices and Security Advisories, including descriptions of the fields above, security branches, and the 
following sections, please visit <URL:https://security.FreeBSD.org/>.";
diff --git a/test/test-case.vala b/test/test-case.vala
index bb684967..44675942 100644
--- a/test/test-case.vala
+++ b/test/test-case.vala
@@ -82,6 +82,14 @@ public void assert_int(int expected, int actual, string? context = null)
     }
 }
 
+public void assert_int64(int64 expected, int64 actual, string? context = null)
+    throws Error {
+    if (expected != actual) {
+        print_assert("Expected: %lld, was: %lld".printf(expected, actual), context);
+        assert_not_reached();
+    }
+}
+
 public void assert_uint(uint expected, uint actual, string? context = null)
     throws GLib.Error {
     if (expected != actual) {


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