Skip to content

feat(sync): idempotent email import so retries do not duplicate IMAP (#85) #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TDannhauer wants to merge 1 commit into
base: FRAMEWORK_6_0
Choose a base branch
from
+1,081 −40
Open

feat(sync): idempotent email import so retries do not duplicate IMAP (#85) #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/changelog.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
---
3.0.3:
api: 3.0.3
state:
release: stable
api: stable
date: 2026-07-17
license:
identifier: GPL-2.0-only
uri: https://spdx.org/licenses/GPL-2.0-only.html
notes: "feat(sync): idempotent email Add/Modify/Remove import so Sync retries do not duplicate IMAP messages (#85)\nfeat(state): record applied draft Modify and mail move mappings in Sql and Mongo map stores"
3.0.2:
api: 3.0.2
state:
Expand Down
237 changes: 209 additions & 28 deletions lib/Horde/ActiveSync/Connector/Importer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,53 @@ public function importMessageChange(
// Don't support SMS, but can't tell client that. Send back a phoney
// UID for any imported SMS objects.
if ($class == Horde_ActiveSync::CLASS_SMS
|| strpos($id, 'IGNORESMS_') === 0) {
|| strpos((string) $id, 'IGNORESMS_') === 0) {
return 'IGNORESMS_' . $clientid;
}

// Idempotent email Draft Modify: same SyncKey + ServerId must not
// append another IMAP message when the client retries a lost response.
if ($id
&& $synckey
&& ($message instanceof Horde_ActiveSync_Message_Mail)
&& !empty($message->airsyncbasebody)
&& ($applied = $this->_state->getAppliedPIMChange($id, $synckey))) {
$this->_logger->notice(
sprintf(
'Duplicate draft modify for %s under %s; returning %s',
$id,
$synckey,
$applied['id']
)
);
return $this->_draftModifyStat($applied['id'], $message, $synckey, $id);
}

// Idempotent email flag Modify under the same SyncKey.
if ($id
&& $synckey
&& ($message instanceof Horde_ActiveSync_Message_Mail)
&& empty($message->airsyncbasebody)
&& $this->_state->isMailMapChangeApplied(
$id,
Horde_ActiveSync::CHANGE_TYPE_FLAGS,
$synckey
)) {
$this->_logger->notice(
sprintf(
'Duplicate flag modify for %s under %s',
$id,
$synckey
)
);
return [
'id' => $id,
'mod' => 0,
'flags' => [],
'serverid' => $this->_folderId,
];
}

// Changing an existing object
if ($id && $this->_flags == Horde_ActiveSync::CONFLICT_OVERWRITE_PIM) {
// This is complicated by the fact that in EAS 16.0, clients
Expand Down Expand Up @@ -243,22 +286,43 @@ public function importMessageChange(
}
$stat['serverid'] = $this->_folderId;

// Record the state of the message.
// Email messages are only changed if they are Drafts or if we are
// updating flags.
// When CHANGING a draft message, we are actually deleting the old one
// and replacing it with a message (since we can't edit an existing
// IMAP message while keeping the UID the same). So, do not call
// updateState() for these messages since we don't want to ignore
// this as a PIM sourced change - the change will be caught during
// normal ping/sync cycle.
if ($message instanceof Horde_ActiveSync_Message_Mail) {
if (!empty($message->airsyncbasebody) && !empty($id)) {
// Changing an existing Draft mail.
return $stat;
// Email Draft Modify: IMAP append+delete creates a new UID. Record the
// applied mapping before mailmap so a lost response can be retried
// without another append, and suppress mirror Add/Delete export.
if ($message instanceof Horde_ActiveSync_Message_Mail
&& !empty($message->airsyncbasebody)
&& !empty($id)) {
if ($synckey) {
$this->_state->recordAppliedPIMChange($id, $stat, $synckey);
}
$user = $this->_as->driver->getUser();
$this->_state->updateState(
Horde_ActiveSync::CHANGE_TYPE_DRAFT,
$stat,
Horde_ActiveSync::CHANGE_ORIGIN_PIM,
$user
);
$this->_state->updateState(
Horde_ActiveSync::CHANGE_TYPE_DELETE,
[
'id' => $id,
'mod' => !empty($stat['mod']) ? $stat['mod'] : 0,
'serverid' => $this->_folderId,
],
Horde_ActiveSync::CHANGE_ORIGIN_PIM,
$user
);
return $this->_draftModifyStat(
$stat['id'],
$message,
$synckey ?: '',
$id,
$stat
);
}

// Either a flag change, or adding a new Draft mail.
if ($message instanceof Horde_ActiveSync_Message_Mail) {
// Flag change or new Draft mail.
$changeType = !empty($message->airsyncbasebody)
? Horde_ActiveSync::CHANGE_TYPE_DRAFT
: Horde_ActiveSync::CHANGE_TYPE_FLAGS;
Expand All @@ -274,9 +338,61 @@ public function importMessageChange(
$clientid
);

// Email Add: mailmap has no sync_clientid; dual-write to sync_map so
// isDuplicatePIMAddition() can catch retries after a lost response.
if (!$id && $clientid
&& ($message instanceof Horde_ActiveSync_Message_Mail)) {
$this->_state->recordPIMAddition(
$clientid,
$stat['id'],
$this->_folderId,
$synckey ?: null
);
}

return $stat;
}

/**
* Build a Draft Modify stat array suitable for Sync replies, including
* conversation fields so EAS 16 clients accept the response on retry.
*
* @param string|integer $newId
* @param Horde_ActiveSync_Message_Base $message
* @param string $synckey
* @param string|integer $oldId
* @param array $stat Optional driver stat to merge (atchash, etc.).
*
* @return array
*/
protected function _draftModifyStat(
$newId,
Horde_ActiveSync_Message_Base $message,
$synckey,
$oldId,
array $stat = []
) {
$out = array_merge(
[
'id' => $newId,
'mod' => 0,
'flags' => [],
],
$stat
);
$out['id'] = $newId;
$out['serverid'] = $this->_folderId;
// Stable conversation fields (not time()) so retries emit the same
// SyncReplies shape Gmail requires for Drafts up-sync.
if ($message instanceof Horde_ActiveSync_Message_Mail) {
$subject = (string) $message->subject;
$out['conversationid'] = bin2hex($subject);
$out['conversationindex'] = crc32($synckey . ':' . $oldId . ':' . $newId);
}

return $out;
}

/**
* Import message deletions. This may conflict if the local object has been
* modified.
Expand Down Expand Up @@ -318,27 +434,60 @@ public function importMessageDeletion(array $ids, $class, $instanceids = false)
return $ids;
}

// Ask the backend to delete the message.
$mod = $this->_as->driver->getSyncStamp($this->_folderId);
$ids = $this->_as->driver->deleteMessage($this->_folderId, $ids);
$already = [];
$toDelete = [];
foreach ($ids as $id) {
// Ignore SMS changes.
if (strpos($id, "IGNORESMS_") === 0) {
if (strpos((string) $id, 'IGNORESMS_') === 0) {
continue;
}
$change = [];
$change['id'] = $id;
$change['mod'] = $mod;
$change['serverid'] = $this->_folderId;
if ($this->_state->isMailMapChangeApplied(
$id,
Horde_ActiveSync::CHANGE_TYPE_DELETE
)) {
$already[] = $id;
} else {
$toDelete[] = $id;
}
}

$mod = $this->_as->driver->getSyncStamp($this->_folderId);
$deleted = $toDelete
? $this->_as->driver->deleteMessage($this->_folderId, $toDelete)
: [];
if (!is_array($deleted)) {
$deleted = [];
}

// UIDs the driver did not return are treated as already gone so a
// retried Remove after a lost response is not reported as missing.
$gone = array_diff($toDelete, $deleted);
foreach ($deleted as $id) {
$change = [
'id' => $id,
'mod' => $mod,
'serverid' => $this->_folderId,
];
$this->_state->updateState(
Horde_ActiveSync::CHANGE_TYPE_DELETE,
$change,
Horde_ActiveSync::CHANGE_ORIGIN_PIM,
$this->_as->driver->getUser()
);
}
foreach ($gone as $id) {
$this->_state->updateState(
Horde_ActiveSync::CHANGE_TYPE_DELETE,
[
'id' => $id,
'mod' => $mod,
'serverid' => $this->_folderId,
],
Horde_ActiveSync::CHANGE_ORIGIN_PIM,
$this->_as->driver->getUser()
);
}

return $ids;
return array_values(array_unique(array_merge($already, $deleted, $gone)));
}

/**
Expand Down Expand Up @@ -396,7 +545,31 @@ function ($e) {
$collectionClass = Horde_ActiveSync::CLASS_EMAIL;
}
$dst = $collections->getBackendIdForFolderUid($dst);
$results = $this->_as->driver->moveMessage($this->_folderId, $uids, $dst);
$synckey = $this->_state->getCurrentSyncKey();

$results = [];
$pending = [];
foreach ($uids as $uid) {
if ($synckey
&& ($prev = $this->_state->getAppliedMailMove($uid, $synckey))) {
$results[$uid] = $prev;
continue;
}
$pending[] = $uid;
}

if ($pending) {
$moved = $this->_as->driver->moveMessage(
$this->_folderId,
$pending,
$dst
);
if (is_array($moved)) {
foreach ($moved as $old => $new) {
$results[$old] = $new;
}
}
}

// Check for any missing (not found) source messages.
$missing = count($results) != count($uids)
Expand All @@ -408,7 +581,7 @@ function ($e) {
// sync, but some broken clients don't like this. Save the import
// in the map table in case we need it later.
$mod = $this->_as->driver->getSyncStamp($this->_folderId);
foreach ($uids as $uid) {
foreach ($pending as $uid) {
if (empty($results[$uid])) {
continue;
}
Expand All @@ -424,9 +597,17 @@ function ($e) {
Horde_ActiveSync::CHANGE_ORIGIN_PIM,
$this->_as->driver->getUser()
);
if ($synckey) {
$this->_state->recordAppliedMailMove(
$uid,
$results[$uid],
$synckey,
$dst
);
}
}

return ['results' => $results, 'missing' => $missing];
return ['results' => $results, 'missing' => array_values($missing)];
}

/**
Expand Down
Loading
Loading