Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 939375 - [email] POP3: UX for when there are too many messages in the spool. #267

Merged
merged 1 commit into from Dec 4, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions data/lib/mailapi/composite/incoming.js
Expand Up @@ -77,6 +77,22 @@ function CompositeIncomingAccount(
* support hierarchies, but we just declare that those servers are not
* acceptable for use.
* }
* @param[overflowMap @dictof[
* @key[uidl String]
* @value[@dict[
* @key[size Number]
* ]]
* ]]{
* The list of messages that will NOT be downloaded by a sync
* automatically, but instead need to be fetched with a "Download
* more messages..." operation. (POP3 only.)
* }
* @param[uidlMap @dictof[
* @key[uidl String]
* @value[headerID String]
* ]]{
* A mapping of UIDLs to message header IDs. (POP3 only.)
* }
* ]{
* Meta-information about the account derived from probing the account.
* This information gets flushed on database upgrades.
Expand Down
2 changes: 1 addition & 1 deletion data/lib/mailapi/mailslice.js
Expand Up @@ -871,7 +871,7 @@ MailSlice.prototype = {
* but our driving UI doesn't need it right now.
* }
* @typedef[BodyBlock @dict[
* @key[ids @listof[ID]]}
* @key[ids @listof[ID]]{
* The issued-by-us id's of the messages; the order is parallel to the order
* of `bodies.`
* }
Expand Down
167 changes: 143 additions & 24 deletions data/lib/mailapi/pop3/sync.js
Expand Up @@ -5,6 +5,8 @@ function(log, util, module, require, exports,
mailchew, sync, date, jobmixins,
allback, pop3) {

var PASTWARDS = 1;

/**
* Manage the synchronization process for POP3 accounts. In IMAP and
* ActiveSync, the work of this class is split in two (a `folderConn`
Expand Down Expand Up @@ -75,7 +77,10 @@ function lazyWithConnection(getNew, cbIndex, fn) {

Pop3FolderSyncer.prototype = {
syncable: true,
canGrowSync: false, // not relevant for POP3
get canGrowSync() {
// Only the inbox can be grown in POP3.
return this.isInbox;
},

/**
* Given a list of messages, download snippets for those that don't
Expand Down Expand Up @@ -244,6 +249,17 @@ Pop3FolderSyncer.prototype = {
});
},

/**
* Return the folderMeta for the INBOX, upon which we store the
* uidlMap and overflowMap. Cache it for performance, since this
* function gets invoked frequently.
*/
get inboxMeta() {
// Override this getter to provide direct access in the future.
return (this.inboxMeta = this.account.getFolderMetaForFolderId(
this.account.getFirstFolderWithType('inbox').id));
},

/**
* Retrieve the message's id (header.id) given a server's UIDL.
*
Expand All @@ -255,21 +271,58 @@ Pop3FolderSyncer.prototype = {
if (uidl == null) {
return null;
}
var inboxMeta = this.account.getFolderMetaForFolderId(
this.account.getFirstFolderWithType('inbox').id);
inboxMeta.uidlMap = inboxMeta.uidlMap || {};
return inboxMeta.uidlMap[uidl];
this.inboxMeta.uidlMap = this.inboxMeta.uidlMap || {};
return this.inboxMeta.uidlMap[uidl];
},

/**
* Store the given message UIDL so that we know it has already been
* downloaded.
* downloaded. If the message was previously marked as overflow,
* remove it from the overflow map because we know about it now.
*/
storeMessageUidlForMessageId: function(uidl, headerId) {
var inboxMeta = this.account.getFolderMetaForFolderId(
this.account.getFirstFolderWithType('inbox').id);
inboxMeta.uidlMap = inboxMeta.uidlMap || {};
inboxMeta.uidlMap[uidl] = headerId;
this.inboxMeta.uidlMap = this.inboxMeta.uidlMap || {};
this.inboxMeta.uidlMap[uidl] = headerId;
if (this.inboxMeta.overflowMap) {
delete this.inboxMeta.overflowMap[uidl];
}
},

/**
* Mark the given message UIDL as being an "overflow message"; that
* is, it was NOT downloaded and should be made available to
* download during a "download more messages..." operation.
*
* This data is stored in INBOX's folderMeta like so:
*
* overflowMap: {
* "(message uidl)": { size: 0 },
* ...
* }
*/
storeOverflowMessageUidl: function(uidl, size) {
this.inboxMeta.overflowMap = this.inboxMeta.overflowMap || {};
this.inboxMeta.overflowMap[uidl] = { size: size };
},

/**
* Return true if there are overflow messages. (If so, we're NOT
* synced to the dawn of time.)
*/
hasOverflowMessages: function() {
if (!this.inboxMeta.overflowMap) { return false; }
for (var key in this.inboxMeta.overflowMap) {
return true; // if there's even a single key, we have some!
}
return false;
},

/**
* Return whether or not the given UIDL is in the overflow map.
*/
isUidlInOverflowMap: function(uidl) {
if (!this.inboxMeta.overflowMap) { return false; }
return !!this.inboxMeta.overflowMap[uidl];
},

/**
Expand All @@ -280,7 +333,7 @@ Pop3FolderSyncer.prototype = {
*/
initialSync: function(slice, initialDays, syncCb, doneCb, progressCb) {
syncCb('sync', false /* accumulateMode */, true /* ignoreHeaders */);
this.sync(true, slice, doneCb, progressCb);
this.sync('initial', slice, doneCb, progressCb);
},

/**
Expand All @@ -290,7 +343,7 @@ Pop3FolderSyncer.prototype = {
*/
refreshSync: function(
slice, dir, startTS, endTS, origStartTS, doneCb, progressCb) {
this.sync(false, slice, doneCb, progressCb);
this.sync('refresh', slice, doneCb, progressCb);
},

/**
Expand Down Expand Up @@ -324,11 +377,20 @@ Pop3FolderSyncer.prototype = {
},

/**
* Irrelevant for POP3.
* If we have overflow messages, fetch them here.
*/
growSync: function(slice, growthDirection, anchorTS, syncStepDays,
doneCallback, progressCallback) {
return false; // No need to invoke the callbacks.
if (growthDirection !== PASTWARDS || !this.hasOverflowMessages()) {
return false;
}

// For simplicity, we ignore anchorTS and syncStepDays, because
// POP3's limitations make it difficult to infer anything about
// the messages we're going to download now. All we can do here is
// download another batch of overflow messages.
this.sync('grow', slice, doneCallback, progressCallback);
return true;
},

allConsumersDead: function() {
Expand Down Expand Up @@ -357,17 +419,50 @@ Pop3FolderSyncer.prototype = {
* messages along with messages we've seen before. To ensure we only
* retrieve messages we don't know about, we keep track of message
* unique IDs (UIDLs) and only download new messages.
*
* OVERFLOW MESSAGE HANDLING:
*
* We don't want to overwhelm a sync with a ridiculous number of
* messages if the spool has a lot of new messagse. Instead of
* blindly downloading all headers right away, we store excess
* "overflow" messages for future "grow" syncs, e.g. when the user
* clicks "Get More Messages" in the message list.
*
* This works as follows:
*
* If we're syncing normally, mark any excess messages as overflow
* messages and don't download them. This is handled largely by
* Pop3Client by the maxMessages option to listMessages(). We ignore
* any messages already marked as overflow for the purposes of the
* sync filter.
*
* If this is a grow sync, i.e. we want to download some overflow
* messages, we set the download filter to _only_ include overflow
* UIDLs. We may still have _more_ overflow messages, but that's
* okay, because they'll just be stored in the overflowMap like
* normal, for a future "grow" sync. Any messages we do download are
* marked as stored and removed from the overflowMap (in
* `this.storeMessageUidlForMessageId`).
*/
sync: lazyWithConnection(/* getNew = */ true, /* cbIndex = */ 2,
function(conn, initialSync, slice, doneCallback, progressCallback) {
function(conn, syncType, slice, doneCallback, progressCallback) {
// if we could not establish a connection, abort the sync.
var self = this;
this._LOG.sync_begin();

// Only fetch info for messages we don't already know about.
var filterFunc = function(uidl) {
return self.getMessageIdForUidl(uidl) == null; // might be 0
};
var filterFunc;
if (syncType !== 'grow') {
// In a regular sync, download any message that we don't know
// about that isn't in the overflow map.
filterFunc = function(uidl) {
return self.getMessageIdForUidl(uidl) == null && // might be 0
!self.isUidlInOverflowMap(uidl);
};
} else /* (syncType === 'grow') */ {
// In a 'grow' sync, ONLY download overflow messages.
filterFunc = this.isUidlInOverflowMap.bind(this);
}

var bytesStored = 0;
var numMessagesSynced = 0;
Expand All @@ -385,6 +480,7 @@ Pop3FolderSyncer.prototype = {
conn.listMessages({
filter: filterFunc,
checkpointInterval: sync.POP3_SAVE_STATE_EVERY_N_MESSAGES,
maxMessages: sync.POP3_MAX_MESSAGES_PER_SYNC,
checkpoint: function(next) {
// Every N messages, wait for everything to be stored to
// disk and saved in the database. Then proceed.
Expand All @@ -405,7 +501,7 @@ Pop3FolderSyncer.prototype = {
messageCb();
});
}.bind(this),
}, function fetchDone(err, numSynced) {
}, function fetchDone(err, numSynced, overflowMessages) {
// Upon downloading all of the messages, we MUST issue a QUIT
// command. This will tear down the connection, however if we
// don't, we will never receive notifications of new messages.
Expand All @@ -420,6 +516,15 @@ Pop3FolderSyncer.prototype = {
doneCallback(err);
return;
}

// If there were excess messages, mark them for later download.
if (overflowMessages.length) {
overflowMessages.forEach(function(message) {
this.storeOverflowMessageUidl(message.uidl, message.size);
}, this);
this._LOG.overflowMessages(overflowMessages.length);
}

// When all of the messages have been persisted to disk, indicate
// that we've successfully synced. Refresh our view of the world.
this.storage.runAfterDeferredCalls(fetchDoneCb);
Expand All @@ -428,12 +533,24 @@ Pop3FolderSyncer.prototype = {

latch.then((function onSyncDone() {
this._LOG.sync_end();
// POP3 always syncs the entire time range available.

// Because POP3 has no concept of syncing discrete time ranges,
// we have to trick the storage into marking everything synced
// _except_ the dawn of time. This has to be slightly later than
// a value that would be interpreted as the dawn of time -- in
// this case, it has to be one day plus one. Ideally, this
// should be abstracted a little better; it's mostly IMAP that
// needs more involved logic.
this.storage.markSyncRange(
sync.OLDEST_SYNC_DATE, date.NOW(), 'XXX', date.NOW());
this.storage.markSyncedToDawnOfTime();
sync.OLDEST_SYNC_DATE + date.DAY_MILLIS + 1,
date.NOW(), 'XXX', date.NOW());

if (!this.hasOverflowMessages()) {
this.storage.markSyncedToDawnOfTime();
}

this.account.__checkpointSyncCompleted();
if (initialSync) {
if (syncType === 'initial') {
// If it's the first time we've synced, we've set
// ignoreHeaders to true, which means that slices don't know
// about new messages. We'll reset ignoreHeaders to false
Expand All @@ -449,7 +566,8 @@ Pop3FolderSyncer.prototype = {
this.storage._curSyncSlice.ignoreHeaders = false;
this.storage._curSyncSlice.waitingOnData = 'db';
this.storage.getMessagesInImapDateRange(
0, null, sync.INITIAL_FILL_SIZE, sync.INITIAL_FILL_SIZE,
sync.OLDEST_SYNC_DATE, null,
sync.INITIAL_FILL_SIZE, sync.INITIAL_FILL_SIZE,
// Don't trigger a refresh; we just synced. Accordingly,
// releaseMutex can be null.
this.storage.onFetchDBHeaders.bind(
Expand Down Expand Up @@ -479,6 +597,7 @@ var LOGFAB = exports.LOGFAB = log.register(module, {
events: {
savedAttachment: { storage: true, mimeType: true, size: true },
saveFailure: { storage: false, mimeType: false, error: false },
overflowMessages: { count: true },
},
TEST_ONLY_events: {
},
Expand Down