[geary/mjog/invert-folder-class-hierarchy: 46/72] Geary.Folder: Rename and update list_email_by_id_async to be local-only




commit aa11f210b799ae976780d11a9f0a3de04dae625e
Author: Michael Gratton <mike vee net>
Date:   Fri Feb 19 08:10:39 2021 +1100

    Geary.Folder: Rename and update list_email_by_id_async to be local-only
    
    Rename `list_email_by_id_async` to `list_email_range_by_id` to better
    indicate what it does. Update its signature to never return null, and
    update `ListFLags` to remove any flags that were relevant to
    remote-backed folders.
    
    Update its contract to only return email from local storage, not remote.
    Require results be returned in natural vector order (or reverse if
    `OLDEST_TO_NEWEST` is set). Return all email in the range all the time,
    and throw an error if any do not meet required fields (unless the new
    `ListFlags.INCLUDE_PARTIAL` flag is set).
    
    Update implementations to match. Move email search and vector expansion
    into MinimalFolder for now, so remaining list and search replay ops can
    be removed.

 po/POTFILES.in                                     |   3 -
 .../plugin/mail-merge/mail-merge-folder.vala       |  16 +-
 src/engine/api/geary-email-identifier.vala         |   2 +-
 src/engine/api/geary-folder.vala                   | 116 +++---
 src/engine/app/app-conversation-monitor.vala       |   6 +-
 src/engine/app/app-search-folder.vala              |  14 +-
 .../app-fill-window-operation.vala                 |  35 +-
 src/engine/imap-db/imap-db-folder.vala             |  32 +-
 .../imap-engine-account-synchronizer.vala          |  18 +-
 .../imap-engine/imap-engine-minimal-folder.vala    | 245 +++++++++----
 .../imap-engine-abstract-list-email.vala           | 393 ---------------------
 .../replay-ops/imap-engine-list-email-by-id.vala   | 169 ---------
 .../imap-engine-server-search-email.vala           |  91 -----
 src/engine/meson.build                             |   3 -
 src/engine/outbox/outbox-folder.vala               |  22 +-
 src/engine/smtp/smtp-client-service.vala           |  16 +-
 test/engine/app/app-conversation-monitor-test.vala |  16 +-
 test/mock/mock-folder.vala                         |  10 +-
 test/mock/mock-remote-folder.vala                  |  10 +-
 19 files changed, 329 insertions(+), 888 deletions(-)
---
diff --git a/po/POTFILES.in b/po/POTFILES.in
index b6fed0361..ff6f97d03 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -293,11 +293,9 @@ src/engine/imap-engine/other/imap-engine-other-folder.vala
 src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
 src/engine/imap-engine/outlook/imap-engine-outlook-drafts-folder.vala
 src/engine/imap-engine/outlook/imap-engine-outlook-folder.vala
-src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala
 src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala
 src/engine/imap-engine/replay-ops/imap-engine-create-email.vala
 src/engine/imap-engine/replay-ops/imap-engine-empty-folder.vala
-src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala
 src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala
 src/engine/imap-engine/replay-ops/imap-engine-move-email-commit.vala
 src/engine/imap-engine/replay-ops/imap-engine-move-email-prepare.vala
@@ -306,7 +304,6 @@ src/engine/imap-engine/replay-ops/imap-engine-remove-email.vala
 src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala
 src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
 src/engine/imap-engine/replay-ops/imap-engine-replay-update.vala
-src/engine/imap-engine/replay-ops/imap-engine-server-search-email.vala
 src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala
 src/engine/imap-engine/yahoo/imap-engine-yahoo-folder.vala
 src/engine/imap/message/imap-data-format.vala
diff --git a/src/client/plugin/mail-merge/mail-merge-folder.vala 
b/src/client/plugin/mail-merge/mail-merge-folder.vala
index c6ad228ec..aa7e0cc92 100644
--- a/src/client/plugin/mail-merge/mail-merge-folder.vala
+++ b/src/client/plugin/mail-merge/mail-merge-folder.vala
@@ -223,13 +223,13 @@ public class MailMerge.Folder : Geary.BaseObject,
         return results;
     }
 
-    public async Gee.List<Geary.Email>?
-        list_email_by_id_async(Geary.EmailIdentifier? initial_id,
-                               int count,
-                               Geary.Email.Field required_fields,
-                               Geary.Folder.ListFlags flags,
-                               GLib.Cancellable? cancellable = null)
-        throws GLib.Error {
+    public async Gee.List<Geary.Email> list_email_range_by_id(
+        Geary.EmailIdentifier? initial_id,
+        int count,
+        Geary.Email.Field required_fields,
+        Geary.Folder.ListFlags flags,
+        GLib.Cancellable? cancellable = null
+    ) throws GLib.Error {
         var initial = initial_id as EmailIdentifier;
         if (initial_id != null && initial == null) {
             throw new Geary.EngineError.BAD_PARAMETERS(
@@ -268,7 +268,7 @@ public class MailMerge.Folder : Geary.BaseObject,
             }
         }
 
-        return (list.size > 0) ? list : null;
+        return list;
     }
 
     /** {@inheritDoc} */
diff --git a/src/engine/api/geary-email-identifier.vala b/src/engine/api/geary-email-identifier.vala
index 3dda2f836..115a9522c 100644
--- a/src/engine/api/geary-email-identifier.vala
+++ b/src/engine/api/geary-email-identifier.vala
@@ -79,7 +79,7 @@ public abstract class Geary.EmailIdentifier :
      * EmailIdentifiers that cannot be compared against this one (i.e. of a different subclass)
      * should return 1 as well.  Generally this means they came from different Folders.
      *
-     * @see Folder.list_email_by_id_async
+     * @see Folder.list_email_range_by_id
      */
     public abstract int natural_sort_comparator(Geary.EmailIdentifier other);
 
diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala
index 1db1c9541..0aaa82207 100644
--- a/src/engine/api/geary-folder.vala
+++ b/src/engine/api/geary-folder.vala
@@ -494,31 +494,24 @@ public interface Geary.Folder : GLib.Object, Logging.Source {
     }
 
     /**
-     * Flags modifying how email is retrieved.
+     * Flags for modifying the ranges of email retrieved from the vector.
+     *
+     * @see list_email_range_by_id
      */
     [Flags]
     public enum ListFlags {
+
         NONE = 0,
-        /**
-         * Fetch from the local store only.
-         */
-        LOCAL_ONLY,
-        /**
-         * Fetch from remote store only (results merged into local store).
-         */
-        FORCE_UPDATE,
-        /**
-         * Include the provided EmailIdentifier (only respected by {@link list_email_by_id_async}.
-         */
+
+        /** Include the email with the given identifier in the range. */
         INCLUDING_ID,
-        /**
-         * Direction of list traversal (if not set, from newest to oldest).
-         */
-        OLDEST_TO_NEWEST,
-        /**
-         * Internal use only, prevents flag changes updating unread count.
-         */
-        NO_UNREAD_UPDATE;
+
+        /** Include email that only partially matches the requested fields. */
+        INCLUDING_PARTIAL,
+
+        /** Return email ordered oldest to newest, instead of the default. */
+        OLDEST_TO_NEWEST;
+
 
         public bool is_any_set(ListFlags flags) {
             return (this & flags) != 0;
@@ -528,27 +521,21 @@ public interface Geary.Folder : GLib.Object, Logging.Source {
             return (this & flags) == flags;
         }
 
-        public bool is_local_only() {
-            return is_all_set(LOCAL_ONLY);
-        }
-
-        public bool is_force_update() {
-            return is_all_set(FORCE_UPDATE);
-        }
-
         public bool is_including_id() {
             return is_all_set(INCLUDING_ID);
         }
 
+        public bool is_newest_to_oldest() {
+            return !is_oldest_to_newest();
+        }
+
         public bool is_oldest_to_newest() {
             return is_all_set(OLDEST_TO_NEWEST);
         }
 
-        public bool is_newest_to_oldest() {
-            return !is_oldest_to_newest();
-        }
     }
 
+
     /** The account that owns this folder. */
     public abstract Geary.Account account { get; }
 
@@ -728,51 +715,48 @@ public interface Geary.Folder : GLib.Object, Logging.Source {
      *
      * Emails in the folder are listed starting at a particular
      * location within the vector and moving either direction along
-     * it. For remote-backed folders, the remote server is contacted
-     * if any messages stored locally do not meet the requirements
-     * given by `required_fields`, or if `count` extends back past the
-     * low end of the vector.
-     *
-     * If the {@link EmailIdentifier} is null, it indicates the end of
-     * the vector, not the end of the remote.  Which end depends on
-     * the {@link ListFlags.OLDEST_TO_NEWEST} flag.  If not set, the
-     * default is to traverse from newest to oldest, with null being
-     * the newest email in the vector. If set, the direction is
-     * reversed and null indicates the oldest email in the vector, not
-     * the oldest in the mailbox.
+     * it.
+     *
+     * If the given identifier is null, it indicates the end of the
+     * vector (not the end of the remote for remote-backed folders).
+     * Which end depends on the {@link ListFlags.OLDEST_TO_NEWEST}
+     * flag. If not set, the default is to traverse from newest to
+     * oldest, with null being the newest email in the vector. If set,
+     * the direction is reversed and null indicates the oldest email
+     * in the vector, not the oldest in the mailbox.
      *
      * If not null, the EmailIdentifier ''must'' have originated from
-     * this Folder.
+     * this folder.
      *
      * To fetch all available messages in one call, use a count of
-     * `int.MAX`. If the {@link ListFlags.OLDEST_TO_NEWEST} flag is
-     * set then the listing will contain all messages in the vector,
-     * and no expansion will be performed. It may still access the
-     * remote however in case of any of the messages not meeting the
-     * given `required_fields`. If {@link ListFlags.OLDEST_TO_NEWEST}
-     * is not set, the call will cause the vector to be fully expanded
-     * and the listing will return all messages in the remote
-     * mailbox. Note that specifying `int.MAX` in either case may be a
-     * expensive operation (in terms of both computation and memory)
-     * if the number of messages in the folder or mailbox is large,
-     * hence should be avoided if possible.
+     * `int.MAX`. Note that this can be an extremely expensive
+     * operation.
+     *
+     * Note that for remote-backed folders, email may not have yet
+     * been fully downloaded and hence might exist incomplete in local
+     * storage. If the requested fields are not available for all
+     * email in the range, {@link EngineError.INCOMPLETE_MESSAGE} is
+     * thrown. Use {@link ListFlags.INCLUDING_PARTIAL} to allow email
+     * that does not meet the given criteria to be included in the
+     * results, and connect to the {@link Account.email_complete}
+     * signal to be notified of when those email are fully downloaded.
      *
      * Use {@link ListFlags.INCLUDING_ID} to include the {@link Email}
      * for the particular identifier in the results.  Otherwise, the
-     * specified email will not be included.  A null EmailIdentifier
-     * implies that the top most email is included in the result (i.e.
+     * specified email will not be included. A null identifier implies
+     * that the top most email is included in the result (i.e.
      * ListFlags.INCLUDING_ID is not required);
      *
-     * If the remote connection fails, this call will return
-     * locally-available Email without error.
-     *
-     * There's no guarantee of the returned messages' order.
-     *
-     * The Folder must be opened prior to attempting this operation.
+     * Email is returned listed by the vector's natural ordering, in
+     * the direction given by the given list flags.
      */
-    public abstract async Gee.List<Geary.Email>? list_email_by_id_async(Geary.EmailIdentifier? initial_id,
-        int count, Geary.Email.Field required_fields, ListFlags flags, Cancellable? cancellable = null)
-        throws Error;
+    public abstract async Gee.List<Email> list_email_range_by_id(
+        EmailIdentifier? initial_id,
+        int count,
+        Email.Field required_fields,
+        ListFlags flags,
+        GLib.Cancellable? cancellable = null
+    ) throws GLib.Error;
 
     /**
      * Sets whether this folder has a custom special use.
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index d1bc805cf..0fcaa92f8 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -463,13 +463,13 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         int load_count = 0;
         GLib.Error? scan_error = null;
         try {
-            Gee.Collection<Geary.Email>? messages =
-                yield this.base_folder.list_email_by_id_async(
+            Gee.Collection<Geary.Email> messages =
+                yield this.base_folder.list_email_range_by_id(
                     initial_id, count, required_fields, flags,
                     this.operation_cancellable
                 );
 
-            if (messages != null && !messages.is_empty) {
+            if (!messages.is_empty) {
                 load_count = messages.size;
 
                 foreach (Email email in messages) {
diff --git a/src/engine/app/app-search-folder.vala b/src/engine/app/app-search-folder.vala
index 4eb58d7ca..3ddd01028 100644
--- a/src/engine/app/app-search-folder.vala
+++ b/src/engine/app/app-search-folder.vala
@@ -239,12 +239,13 @@ public class Geary.App.SearchFolder : BaseObject,
         );
     }
 
-    public async Gee.List<Email>? list_email_by_id_async(
+    /** {@inheritDoc} */
+    public async Gee.List<Email> list_email_range_by_id(
         EmailIdentifier? initial_id,
         int count,
         Email.Field required_fields,
         Folder.ListFlags flags,
-        Cancellable? cancellable = null
+        GLib.Cancellable? cancellable = null
     ) throws GLib.Error {
         debug("Waiting to list email");
         int result_mutex_token = yield this.result_mutex.claim_async(cancellable);
@@ -336,7 +337,14 @@ public class Geary.App.SearchFolder : BaseObject,
 
         var list = new Gee.ArrayList<Email>();
         list.add_all(EmailIdentifier.sort_emails(results));
-        return list.is_empty ? null : list;
+        list.sort((a, b) => {
+                int cmp = a.id.natural_sort_comparator(b.id);
+                if (cmp == 0) {
+                    cmp = a.id.stable_sort_comparator(b.id);
+                }
+                return cmp;
+            });
+        return list;
     }
 
     public virtual async void remove_email_async(
diff --git a/src/engine/app/conversation-monitor/app-fill-window-operation.vala 
b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
index 53bc0fa96..f8fe08794 100644
--- a/src/engine/app/conversation-monitor/app-fill-window-operation.vala
+++ b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
@@ -41,7 +41,7 @@ private class Geary.App.FillWindowOperation : ConversationOperation {
 
         try {
             loaded = yield this.monitor.load_by_id_async(
-                this.monitor.window_lowest, num_to_load, LOCAL_ONLY
+                this.monitor.window_lowest, num_to_load, NONE
             );
         } catch (EngineError.NOT_FOUND err) {
             debug("Stale FillWindowOperation: %s", err.message);
@@ -55,39 +55,6 @@ private class Geary.App.FillWindowOperation : ConversationOperation {
             this.monitor.base_folder.email_total
         );
 
-        var remote = this.monitor.base_folder as RemoteFolder;
-        if (loaded < num_to_load &&
-            this.monitor.can_load_more &&
-            remote != null &&
-            remote.is_monitoring) {
-            // Not enough were loaded locally, but the remote seems to
-            // be online and it looks like there and there might be
-            // some more on the remote, so go see if there are any.
-            //
-            // XXX Ideally this would be performed as an explicit user
-            // action
-
-            // Load the max amount if going to the trouble of talking
-            // to the remote.
-            num_to_load = MAX_FILL_COUNT;
-            try {
-                loaded = yield this.monitor.load_by_id_async(
-                    this.monitor.window_lowest, num_to_load, FORCE_UPDATE
-                );
-            } catch (EngineError.NOT_FOUND err) {
-                debug("Stale FillWindowOperation: %s", err.message);
-                return;
-            }
-
-            debug(
-                "Filled %d of %d from the remote, window: %d, total: %d",
-                loaded, num_to_load,
-                this.monitor.conversations.size,
-                this.monitor.base_folder.email_total
-            );
-
-        }
-
         if (loaded == num_to_load) {
             // Loaded the maximum number of messages, so go see if
             // there are any more needed.
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index 486a21a27..17089f002 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -70,6 +70,9 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
             if (flags.is_all_set(Geary.Folder.ListFlags.INCLUDING_ID))
                 result |= INCLUDING_ID;
 
+            if (flags.is_all_set(Geary.Folder.ListFlags.INCLUDING_PARTIAL))
+                result |= PARTIAL_OK;
+
             if (flags.is_all_set(Geary.Folder.ListFlags.OLDEST_TO_NEWEST))
                 result |= OLDEST_TO_NEWEST;
 
@@ -427,11 +430,17 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
         return contained_ids.values;
     }
 
-    public async Gee.List<Geary.Email>? list_email_by_id_async(ImapDB.EmailIdentifier? initial_id,
-        int count, Geary.Email.Field required_fields, ListFlags flags, Cancellable? cancellable)
-        throws Error {
-        if (count <= 0)
-            return null;
+    public async Gee.List<Geary.Email> list_email_by_id_async(
+        ImapDB.EmailIdentifier? initial_id,
+        int count,
+        Email.Field required_fields,
+        ListFlags flags,
+        GLib.Cancellable? cancellable
+    ) throws GLib.Error {
+        var results = new Gee.LinkedList<Email>();
+        if (count <= 0) {
+            return results;
+        }
 
         bool including_id = flags.is_all_set(ListFlags.INCLUDING_ID);
         bool oldest_to_newest = flags.is_all_set(ListFlags.OLDEST_TO_NEWEST);
@@ -446,10 +455,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
             if (initial_id != null) {
                 // use INCLUDE_MARKED_FOR_REMOVE because this is a ranged list ...
                 // do_results_to_location() will deal with removing EmailIdentifiers if necessary
-                LocationIdentifier? location = do_get_location_for_id(cx, initial_id,
-                    ListFlags.INCLUDE_MARKED_FOR_REMOVE, cancellable);
-                if (location == null)
-                    return Db.TransactionOutcome.DONE;
+                LocationIdentifier? location = do_get_location_for_id(
+                    cx, initial_id, INCLUDE_MARKED_FOR_REMOVE, cancellable
+                );
+                if (location == null) {
+                    throw new EngineError.NOT_FOUND("Initial id not found");
+                }
 
                 start_uid = location.uid;
 
@@ -503,11 +514,10 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
         if (only_incomplete)
             locations = yield remove_complete_locations_in_chunks_async(locations, cancellable);
 
-        var results = new Gee.ArrayList<Email>();
         yield list_email_in_chunks_async(
             locations, results, required_fields, flags, true, cancellable
         );
-        return results.is_empty ? null : results;
+        return results;
     }
 
     // ListFlags.OLDEST_TO_NEWEST is ignored.  INCLUDING_ID means including *both* identifiers.
diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala 
b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
index 8ed96fd32..5da089171 100644
--- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
+++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
@@ -371,11 +371,13 @@ private class Geary.ImapEngine.FullFolderSync : RefreshFolderSync {
             "Unable to locate epoch messages on remote folder%s, fetching one past oldest...",
             (id != null) ? " earlier than oldest local" : ""
         );
-        yield this.folder.list_email_by_id_async(
-            id,
+        var minimal = (MinimalFolder) this.folder;
+        var remote = yield minimal.claim_remote_session(cancellable);
+        yield minimal.expand_vector_internal(
+            remote,
+            ((ImapDB.EmailIdentifier) id).uid,
             1,
-            Geary.Email.Field.NONE,
-            Geary.Folder.ListFlags.NONE,
+            OLDEST_TO_NEWEST,
             cancellable
         );
     }
@@ -395,11 +397,13 @@ private class Geary.ImapEngine.FullFolderSync : RefreshFolderSync {
         //
         // XXX This is expensive, but should only usually happen once
         // per folder - at the end of a full sync.
-        yield this.folder.list_email_by_id_async(
+        var minimal = (MinimalFolder) this.folder;
+        var remote = yield minimal.claim_remote_session(cancellable);
+        yield minimal.expand_vector_internal(
+            remote,
             null,
             int.MAX,
-            Geary.Email.Field.NONE,
-            Geary.Folder.ListFlags.NONE,
+            OLDEST_TO_NEWEST,
             cancellable
         );
     }
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index a58aa6ca0..443451668 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -909,28 +909,27 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
         );
     }
 
-    //
-    // list email variants
-    //
-
-    public async Gee.List<Geary.Email>? list_email_by_id_async(Geary.EmailIdentifier? initial_id,
-        int count, Geary.Email.Field required_fields, Folder.ListFlags flags,
-        Cancellable? cancellable = null) throws Error {
-        check_flags("list_email_by_id_async", flags);
-        if (initial_id != null)
+    /** {@inheritDoc} */
+    public async Gee.List<Email> list_email_range_by_id(
+        EmailIdentifier? initial_id,
+        int count,
+        Email.Field required_fields,
+        Folder.ListFlags flags,
+        GLib.Cancellable? cancellable = null
+    ) throws GLib.Error {
+        ImapDB.EmailIdentifier? imap_id = null;
+        if (initial_id != null) {
             check_id("list_email_by_id_async", initial_id);
+            imap_id = (ImapDB.EmailIdentifier) initial_id;
+        }
 
-        if (count == 0)
-            return null;
-
-        // Schedule list operation and wait for completion.
-        ListEmailByID op = new ListEmailByID(this, (ImapDB.EmailIdentifier) initial_id, count,
-            required_fields, flags, cancellable);
-        replay_queue.schedule(op);
-
-        yield op.wait_for_ready_async(cancellable);
-
-        return !op.accumulator.is_empty ? op.accumulator : null;
+        return yield this.local_folder.list_email_by_id_async(
+            imap_id,
+            count,
+            required_fields,
+            ImapDB.Folder.ListFlags.from_folder_flags(flags),
+            cancellable
+        );
     }
 
     // Helper function for child classes dealing with the
@@ -965,13 +964,6 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
         yield this._account.local.db.run_gc(NONE, null, cancellable);
     }
 
-    private void check_flags(string method, Folder.ListFlags flags) throws EngineError {
-        if (flags.is_all_set(Folder.ListFlags.LOCAL_ONLY) && 
flags.is_all_set(Folder.ListFlags.FORCE_UPDATE)) {
-            throw new EngineError.BAD_PARAMETERS("%s %s failed: LOCAL_ONLY and FORCE_UPDATE are mutually 
exclusive",
-                to_string(), method);
-        }
-    }
-
     private void check_id(string method, EmailIdentifier id) throws EngineError {
         if (!(id is ImapDB.EmailIdentifier))
             throw new EngineError.BAD_PARAMETERS("Email ID %s is not IMAP Email ID", id.to_string());
@@ -1129,24 +1121,161 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
                 new Imap.MessageSet.uid_range(new Imap.UID(Imap.UID.MIN), before_uid.previous(true))));
         }
 
-        ServerSearchEmail op = new ServerSearchEmail(this, criteria, Geary.Email.Field.NONE,
-            cancellable);
+        var remote = yield claim_remote_session(cancellable);
+        Gee.SortedSet<Imap.UID>? uids = yield remote.search_async(
+            criteria, cancellable
+        );
+        if (uids == null || uids.size == 0) {
+            return null;
+        }
 
-        replay_queue.schedule(op);
+        // if the earliest UID is not in the local store, then need to
+        // expand vector to it first
+        ImapDB.EmailIdentifier? first_id = yield this.local_folder.get_id_async(
+            uids.first(), NONE, cancellable
+        );
+        if (first_id == null) {
+            yield expand_vector_internal(
+                remote, uids.first(), 1, OLDEST_TO_NEWEST, cancellable
+            );
+            first_id = yield this.local_folder.get_id_async(
+                uids.first(), NONE, cancellable
+            );
+        }
 
-        yield op.wait_for_ready_async(cancellable);
+        return yield this.local_folder.fetch_email_async(
+            first_id, PROPERTIES, NONE, cancellable
+        );
+    }
 
-        // find earliest ID; because all Email comes from Folder, UID should always be present
-        Geary.Email? earliest = null;
-        ImapDB.EmailIdentifier? earliest_id = null;
-        foreach (Geary.Email email in op.accumulator) {
-            ImapDB.EmailIdentifier email_id = (ImapDB.EmailIdentifier) email.id;
-            if (earliest_id == null || email_id.uid.compare_to(earliest_id.uid) < 0) {
-                earliest = email;
-                earliest_id = email_id;
+    /**
+     * Expands the owning folder's vector.
+     *
+     * Lists on the remote messages needed to fulfill ImapDB's
+     * requirements from `initial_uid` (inclusive) forward to the
+     * start of the vector if the OLDEST_TO_NEWEST flag is set, else
+     * from `initial_uid` (inclusive) back at most by `count` number
+     * of messages. If `initial_uid` is null, the start or end of the
+     * vector is used, respectively.
+     *
+     * The returned UIDs are those added to the vector, which can then
+     * be examined and added to the messages to be fulfilled if
+     * needed.
+     */
+    internal async void expand_vector_internal(
+        Imap.FolderSession remote,
+        Imap.UID? initial_uid,
+        int count,
+        Folder.ListFlags flags,
+        GLib.Cancellable cancellable
+    ) throws GLib.Error {
+        debug("Expanding vector...");
+        int remote_count = remote.folder.properties.email_total;
+
+        // include marked for removed in the count in case this is being called while a removal
+        // is in process, in which case don't want to expand vector this moment because the
+        // vector is in flux
+        int local_count = yield this.local_folder.get_email_count_async(
+            INCLUDE_MARKED_FOR_REMOVE, cancellable
+        );
+
+        // watch out for attempts to expand vector when it's expanded as far as it will go
+        if (local_count >= remote_count) {
+            return;
+        }
+
+        // Determine low and high position for expansion. The vector
+        // start position is based on the assumption that the vector
+        // end is the same as the remote end.
+        int64 vector_start = (remote_count - local_count + 1);
+        int64 low_pos = -1;
+        int64 high_pos = -1;
+        int64 initial_pos = -1;
+
+        if (initial_uid != null) {
+            Gee.Map<Imap.UID, Imap.SequenceNumber>? map =
+            // this does an IMAP FETCH, but EXPUNGE responses to that
+            // are forbidden the returned UIDs are likely fine.
+            yield remote.uid_to_position_async(
+                new Imap.MessageSet.uid(initial_uid), cancellable
+            );
+            Imap.SequenceNumber? pos = map.get(initial_uid);
+            if (pos != null) {
+                initial_pos = pos.value;
             }
         }
-        return earliest;
+
+        // Determine low and high position for expansion
+        if (flags.is_oldest_to_newest()) {
+            low_pos = Imap.SequenceNumber.MIN;
+            if (initial_pos > Imap.SequenceNumber.MIN) {
+                low_pos = initial_pos;
+            }
+            high_pos = vector_start - 1;
+        } else {
+            // Newest to oldest.
+            if (initial_pos <= Imap.SequenceNumber.MIN) {
+                high_pos = remote_count;
+                low_pos = Numeric.int64_floor(
+                    high_pos - count + 1, Imap.SequenceNumber.MIN
+                );
+            } else {
+                high_pos = Numeric.int64_floor(
+                    initial_pos, vector_start - 1
+                );
+                low_pos = Numeric.int64_floor(
+                    initial_pos - (count - 1), Imap.SequenceNumber.MIN
+                );
+            }
+        }
+
+        if (low_pos > high_pos) {
+            debug("Aborting vector expansion, low_pos=%s > high_pos=%s",
+                  low_pos.to_string(), high_pos.to_string());
+            return;
+        }
+
+        Imap.MessageSet msg_set = new Imap.MessageSet.range_by_first_last(
+            new Imap.SequenceNumber(low_pos),
+            new Imap.SequenceNumber(high_pos)
+        );
+        int64 actual_count = (high_pos - low_pos) + 1;
+
+        debug("Performing vector expansion using %s for initial_uid=%s count=%d actual_count=%s 
local_count=%d remote_count=%d oldest_to_newest=%s",
+              msg_set.to_string(),
+              (initial_uid != null) ? initial_uid.to_string() : "(null)", count, actual_count.to_string(),
+              local_count, remote_count, flags.is_oldest_to_newest().to_string());
+
+
+        Gee.List<Geary.Email>? list = yield remote.list_email_async(
+            msg_set,
+            ImapDB.Folder.REQUIRED_FIELDS,
+            cancellable
+        );
+
+        var created_ids = new Gee.HashSet<EmailIdentifier>();
+        if (list != null) {
+            Gee.Map<Email, bool>? created_or_merged =
+                yield local_folder.create_or_merge_email_async(
+                    list, true, this.harvester, cancellable
+                );
+
+            foreach (Email email in created_or_merged.keys) {
+                if (created_or_merged.get(email)) {
+                    created_ids.add(email.id);
+                }
+            }
+
+            if (!created_ids.is_empty) {
+                if (flags.is_oldest_to_newest()) {
+                    email_inserted(created_ids);
+                } else {
+                    email_appended(created_ids);
+                }
+            }
+        }
+
+        debug("Vector expansion completed (%d new email)", created_ids.size);
     }
 
     protected async EmailIdentifier?
@@ -1167,9 +1296,7 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
             // locally possibly before the server notified that the
             // message exists. As such, fetch any missing parts from
             // the remote to ensure it is properly filled in.
-            yield list_email_by_id_async(
-                op.created_id, 1, ALL, INCLUDING_ID, cancellable
-            );
+            yield get_email_by_id(op.created_id, ALL, cancellable);
         } else {
             // The server didn't return a UID for the new email, so do
             // a sync now to ensure it shows up immediately.
@@ -1185,23 +1312,20 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
         // Update this to use CHANGEDSINCE FETCH when available, when
         // we support IMAP CONDSTORE (Bug 713117).
         int chunk_size = FLAG_UPDATE_START_CHUNK;
-        Geary.EmailIdentifier? lowest = null;
+        Geary.EmailIdentifier? oldest = null;
         while (true) {
-            Gee.List<Geary.Email>? local = yield list_email_by_id_async(
-                lowest, chunk_size,
-                Geary.Email.Field.FLAGS,
-                Geary.Folder.ListFlags.LOCAL_ONLY,
+            var local = yield list_email_range_by_id(
+                oldest,
+                chunk_size,
+                FLAGS,
+                NONE,
                 cancellable
             );
-            if (local == null ||
-                local.is_empty ||
-                this.remote_session == null) {
+            if (local.is_empty) {
                 break;
             }
 
-            // find the lowest for the next iteration
-            var sorted = EmailIdentifier.sort_emails(local);
-            lowest = sorted.first().id;
+            oldest = local.last().id;
 
             // Get all email identifiers in the local folder mapped to their EmailFlags
             var local_map = new Gee.HashMap<EmailIdentifier,EmailFlags>();
@@ -1210,20 +1334,21 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
             }
 
             debug("Fetching %d flags", local_map.keys.size);
-            Gee.List<Email>? remote =
-                yield this.remote_session.list_email_async(
+            var remote = yield claim_remote_session(cancellable);
+            Gee.List<Email>? remote_email =
+                yield remote.list_email_async(
                     new Imap.MessageSet.uid_range(
-                        ((ImapDB.EmailIdentifier) lowest).uid,
-                        ((ImapDB.EmailIdentifier) sorted.last().id).uid
+                        ((ImapDB.EmailIdentifier) oldest).uid,
+                        ((ImapDB.EmailIdentifier) local.first().id).uid
                     ),
                     FLAGS,
                     cancellable
                 );
 
-            if (remote != null && !remote.is_empty) {
+            if (remote_email != null && !remote_email.is_empty) {
                 var changed_set = new Gee.HashSet<Email>();
                 var changed_map = new Gee.HashMap<EmailIdentifier,EmailFlags>();
-                foreach (var email in remote) {
+                foreach (var email in remote_email) {
                     var local_flags = local_map.get(email.id);
                     var remote_flags = email.email_flags;
                     if (local_flags != null &&
diff --git a/src/engine/meson.build b/src/engine/meson.build
index 997675d85..e68de18b8 100644
--- a/src/engine/meson.build
+++ b/src/engine/meson.build
@@ -203,11 +203,9 @@ engine_vala_sources = files(
   'imap-engine/outlook/imap-engine-outlook-account.vala',
   'imap-engine/outlook/imap-engine-outlook-folder.vala',
   'imap-engine/outlook/imap-engine-outlook-drafts-folder.vala',
-  'imap-engine/replay-ops/imap-engine-abstract-list-email.vala',
   'imap-engine/replay-ops/imap-engine-copy-email.vala',
   'imap-engine/replay-ops/imap-engine-create-email.vala',
   'imap-engine/replay-ops/imap-engine-empty-folder.vala',
-  'imap-engine/replay-ops/imap-engine-list-email-by-id.vala',
   'imap-engine/replay-ops/imap-engine-mark-email.vala',
   'imap-engine/replay-ops/imap-engine-move-email-commit.vala',
   'imap-engine/replay-ops/imap-engine-move-email-prepare.vala',
@@ -216,7 +214,6 @@ engine_vala_sources = files(
   'imap-engine/replay-ops/imap-engine-replay-append.vala',
   'imap-engine/replay-ops/imap-engine-replay-removal.vala',
   'imap-engine/replay-ops/imap-engine-replay-update.vala',
-  'imap-engine/replay-ops/imap-engine-server-search-email.vala',
   'imap-engine/yahoo/imap-engine-yahoo-account.vala',
   'imap-engine/yahoo/imap-engine-yahoo-folder.vala',
 
diff --git a/src/engine/outbox/outbox-folder.vala b/src/engine/outbox/outbox-folder.vala
index a5b2147e7..9857a3964 100644
--- a/src/engine/outbox/outbox-folder.vala
+++ b/src/engine/outbox/outbox-folder.vala
@@ -264,21 +264,23 @@ public class Geary.Outbox.Folder : BaseObject,
         return email;
     }
 
-    public async Gee.List<Email>?
-        list_email_by_id_async(Geary.EmailIdentifier? _initial_id,
-                               int count,
-                               Geary.Email.Field required_fields,
-                               Geary.Folder.ListFlags flags,
-                               GLib.Cancellable? cancellable = null)
-        throws GLib.Error {
+    public async Gee.List<Email> list_email_range_by_id(
+        Geary.EmailIdentifier? _initial_id,
+        int count,
+        Email.Field required_fields,
+        Geary.Folder.ListFlags flags,
+        GLib.Cancellable? cancellable = null
+    ) throws GLib.Error {
         EmailIdentifier? initial_id = _initial_id as EmailIdentifier;
         if (_initial_id != null && initial_id == null) {
             throw new EngineError.BAD_PARAMETERS("EmailIdentifier %s not for Outbox",
                 initial_id.to_string());
         }
 
-        if (count <= 0)
-            return null;
+        var list = new Gee.LinkedList<Email>();
+        if (count <= 0)  {
+            return list;
+        }
 
         bool list_all = (required_fields != Email.Field.NONE);
 
@@ -287,7 +289,6 @@ public class Geary.Outbox.Folder : BaseObject,
             select = select + ", message, sent";
         }
 
-        Gee.List<Geary.Email>? list = null;
         yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
             string dir = flags.is_newest_to_oldest() ? "DESC" : "ASC";
 
@@ -317,7 +318,6 @@ public class Geary.Outbox.Folder : BaseObject,
             if (results.finished)
                 return Db.TransactionOutcome.DONE;
 
-            list = new Gee.ArrayList<Geary.Email>();
             int position = -1;
             do {
                 int64 ordering = results.int64_at(1);
diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala
index 841a1da0e..39c38b854 100644
--- a/src/engine/smtp/smtp-client-service.vala
+++ b/src/engine/smtp/smtp-client-service.vala
@@ -228,17 +228,15 @@ public class Geary.Smtp.ClientService : Geary.ClientService {
     private async void fill_outbox_queue(GLib.Cancellable cancellable) {
         debug("Filling queue");
         try {
-            Gee.List<Email>? queued = yield this.outbox.list_email_by_id_async(
+            Gee.List<Email> queued = yield this.outbox.list_email_range_by_id(
                 null,
                 int.MAX, // fetch all
-                Email.Field.NONE, // ids only
-                Folder.ListFlags.OLDEST_TO_NEWEST,
+                NONE, // ids only
+                OLDEST_TO_NEWEST,
                 cancellable
             );
-            if (queued != null) {
-                foreach (Email email in queued) {
-                    this.outbox_queue.send(email.id);
-                }
+            foreach (Email email in queued) {
+                this.outbox_queue.send(email.id);
             }
         } catch (Error err) {
             warning("Error filling queue: %s", err.message);
@@ -406,10 +404,10 @@ public class Geary.Smtp.ClientService : Geary.ClientService {
         if (id != null) {
             const int MAX_RETRIES = 3;
             for (int i = 0; i < MAX_RETRIES; i++) {
-                Gee.List<Email>? list = yield location.list_email_by_id_async(
+                Gee.List<Email> list = yield location.list_email_range_by_id(
                     null, 1, REFERENCES, NONE, cancellable
                 );
-                if (list != null && !list.is_empty) {
+                if (!list.is_empty) {
                     Email listed = Collection.first(list);
                     if (listed.message_id != null &&
                         listed.message_id.equal_to(id)) {
diff --git a/test/engine/app/app-conversation-monitor-test.vala 
b/test/engine/app/app-conversation-monitor-test.vala
index 40ac2d29c..d4ab4bd2f 100644
--- a/test/engine/app/app-conversation-monitor-test.vala
+++ b/test/engine/app/app-conversation-monitor-test.vala
@@ -94,7 +94,8 @@ class Geary.App.ConversationMonitorTest : TestCase {
         monitor.scan_completed.connect(() => { saw_scan_completed = true; });
 
         test_article.expect_call("start_monitoring");
-        test_article.expect_call("list_email_by_id_async");
+        test_article.expect_call("list_email_range_by_id")
+            .returns_object(new Gee.ArrayList<Email>());
         test_article.expect_call("stop_monitoring");
 
         monitor.start_monitoring.begin(
@@ -139,7 +140,8 @@ class Geary.App.ConversationMonitorTest : TestCase {
         monitor.scan_started.connect(() => { saw_scan_started = true; });
         monitor.scan_completed.connect(() => { saw_scan_completed = true; });
 
-        test_article.expect_call("list_email_by_id_async");
+        test_article.expect_call("list_email_range_by_id")
+            .returns_object(new Gee.ArrayList<Email>());
 
         monitor.start_monitoring.begin(
             test_cancellable, this.async_completion
@@ -180,7 +182,8 @@ class Geary.App.ConversationMonitorTest : TestCase {
         monitor.scan_started.connect(() => { saw_scan_started = true; });
         monitor.scan_completed.connect(() => { saw_scan_completed = true; });
 
-        test_article.expect_call("list_email_by_id_async");
+        test_article.expect_call("list_email_range_by_id")
+            .returns_object(new Gee.ArrayList<Email>());
 
         monitor.start_monitoring.begin(
             test_cancellable, this.async_completion
@@ -448,7 +451,8 @@ class Geary.App.ConversationMonitorTest : TestCase {
         );
 
         this.base_folder.expect_call("get_multiple_email_by_id");
-        this.base_folder.expect_call("list_email_by_id_async");
+        this.base_folder.expect_call("list_email_range_by_id")
+            .returns_object(new Gee.ArrayList<Email>());
 
         wait_for_signal(monitor, "email-flags-changed");
 
@@ -490,7 +494,7 @@ class Geary.App.ConversationMonitorTest : TestCase {
         /*
          * The process for loading messages looks roughly like this:
          * - load_by_id_async
-         *   - base_folder.list_email_by_id_async
+         *   - base_folder.list_email_range_by_id
          *   - process_email_async
          *     - gets all related messages from listing
          *     - expand_conversations_async
@@ -503,7 +507,7 @@ class Geary.App.ConversationMonitorTest : TestCase {
 
         this.base_folder.expect_call("start_monitoring");
         ValaUnit.ExpectedCall list_call = this.base_folder
-            .expect_call("list_email_by_id_async")
+            .expect_call("list_email_range_by_id")
             .returns_object(new Gee.ArrayList<Email>.wrap(base_folder_email));
 
         if (base_folder_email.length > 0) {
diff --git a/test/mock/mock-folder.vala b/test/mock/mock-folder.vala
index 0663b7d49..2aa371925 100644
--- a/test/mock/mock-folder.vala
+++ b/test/mock/mock-folder.vala
@@ -94,17 +94,17 @@ public class Mock.Folder : GLib.Object,
         );
     }
 
-    public async Gee.List<Geary.Email>?
-        list_email_by_id_async(Geary.EmailIdentifier? initial_id,
+    public async Gee.List<Geary.Email>
+        list_email_range_by_id(Geary.EmailIdentifier? initial_id,
                                int count,
                                Geary.Email.Field required_fields,
                                Geary.Folder.ListFlags flags,
                                GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        return yield object_call_async<Gee.List<Geary.Email>?>(
-            "list_email_by_id_async",
+        return object_or_throw_call<Gee.List<Geary.Email>>(
+            "list_email_range_by_id",
             {initial_id, int_arg(count), box_arg(required_fields), box_arg(flags), cancellable},
-            null
+            new Geary.EngineError.UNSUPPORTED("Mock method")
         );
     }
 
diff --git a/test/mock/mock-remote-folder.vala b/test/mock/mock-remote-folder.vala
index ef7717024..0f88098cb 100644
--- a/test/mock/mock-remote-folder.vala
+++ b/test/mock/mock-remote-folder.vala
@@ -142,17 +142,17 @@ public class Mock.RemoteFolder : GLib.Object,
         );
     }
 
-    public async Gee.List<Geary.Email>?
-        list_email_by_id_async(Geary.EmailIdentifier? initial_id,
+    public async Gee.List<Geary.Email>
+        list_email_range_by_id(Geary.EmailIdentifier? initial_id,
                                int count,
                                Geary.Email.Field required_fields,
                                Geary.Folder.ListFlags flags,
                                GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        return yield object_call_async<Gee.List<Geary.Email>?>(
-            "list_email_by_id_async",
+        return object_or_throw_call<Gee.List<Geary.Email>>(
+            "list_email_range_by_id",
             {initial_id, int_arg(count), box_arg(required_fields), box_arg(flags), cancellable},
-            null
+            new Geary.EngineError.UNSUPPORTED("Mock method")
         );
     }
 


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