Skip to content

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

Open
TDannhauer wants to merge 1 commit into
FRAMEWORK_6_0from
feat/sync-idempotent-import
Open

feat(sync): idempotent email import so retries do not duplicate IMAP (#85)#86
TDannhauer wants to merge 1 commit into
FRAMEWORK_6_0from
feat/sync-idempotent-import

Conversation

@TDannhauer

Copy link
Copy Markdown
Contributor

Summary

  • Make email Sync Add / Modify / Remove (including remove-as-move) idempotent under the same SyncKey so a lost or rejected Sync response cannot duplicate IMAP messages
  • Critical path: EAS 16 Drafts Modify (UID replace via append+delete) records old→new mapping and returns the same successful reply on retry
  • Sql and Mongo state backends get matching applied-command helpers; unit tests cover the importer short-circuits

Motivation

Tracked in #85 (observed with Gmail Android FullDraftsUpSync). Streaming / transport work (#84) reduces client read timeouts but does not fix the durability mismatch: client commands are committed to IMAP before the client acks the response. On retry under SyncKey N, a non-idempotent Draft Modify appends another generation of messages.

Complementary to #84not stacked on it; this PR targets FRAMEWORK_6_0 directly.

Changes

  • State_Base / State_Sql / State_Mongo: getAppliedPIMChange, recordAppliedPIMChange, recordPIMAddition, isMailMapChangeApplied, mail-move mapping; tighten isDuplicatePIMAddition; Mongo CHANGE_TYPE_DRAFT + isDuplicatePIMChange
  • Connector_Importer: short-circuit retries; Draft Modify dual-writes map then mailmap (sync_draft new + sync_deleted old); rebuild stable conversation fields for EAS 16 SyncReplies
  • Unit tests in ImporterIdempotentImportTest

Related Core PR: refuse Draft Modify append-as-new when the old IMAP UID is already missing (defense in depth).

Test plan

  • vendor/bin/phpunit … ImporterIdempotentImportTest.php (6 tests)
  • Simulate Drafts Modify batch, commit IMAP, abort response; retry same SyncKey — message count must not increase
  • Successful Drafts up-sync does not flood server→client Adds for just-replaced UIDs
  • Gmail Android FullDraftsUpSync against a small Drafts folder does not grow across failed/partial responses

@TDannhauer
TDannhauer requested a review from ralflang July 17, 2026 16:14
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

🔍 CI Results

Overall: ❌ 12/12 lanes failed

TL;DR: ❌ Quality issues: PHPUnit: 54 tests failed; PHPStan: 328 unique errors in 8 lanes; PHP-CS-Fixer: 23 files.

Summary by PHP Version

PHP dev stable
8.0
8.1
8.2
8.3
8.4
8.5

Quality Metrics

  • PHPUnit: 16 failures, 38 errors out of 229 tests in 8 lanes ❌
    Per-test detail in the table below. Raw JUnit XML and JSON are uploaded as workflow artifacts.

    Which tests failed
    Type Test Lanes Message
    ❌ failure Horde\ActiveSync\AppointmentTest::testSetRecurrenceEas16SetsFirstDayOfWeek php8.2-dev, php8.2-stable, php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [PHPUnit\Framework\ExpectationFailedException] Failed asserting that '' matches expected 0.
    ⚠️ error Horde\ActiveSync\FolderUidDeterministicTest::testFolderUidIsDeterministicAcrossDriverInstances php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\FolderUidDeterministicTest::testFolderUidIsDeterministicForRepeatedLookups php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\FolderUidDeterministicTest::testFolderUidUsesPersistedMapWhenPresent php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\Request\FindRequestTest::testMockDriverReturnsSuccessfulEmptyFindResults php8.2-dev, php8.2-stable, php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [InvalidArgumentException] Missing required state object
    ❌ failure Horde\ActiveSync\Request\FindRequestTest::testParseFindRangeHonorsClientWindow php8.2-dev, php8.2-stable, php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [PHPUnit\Framework\ExpectationFailedException] Failed asserting that 100 is identical to 101.
    ⚠️ error Horde\ActiveSync\SyncTimeBudgetTest::testGetSyncConfigDefaultsToEmptyArray php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\SyncTimeBudgetTest::testGetSyncConfigReturnsConfiguredValues php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
  • PHPStan (advisory): 328 unique errors in 8 lanes ⚠️

  • PHP-CS-Fixer: 23 unique files with issues (of 202 checked) ⚠️

❌ Failed Lanes

php8.0-dev

  • PHPUnit: ❌ Setup failed (No result file found)
  • PHPStan: ❌ Setup failed (No result file found)

php8.0-stable

  • PHPUnit: ❌ Setup failed (Composer install failed in /tmp/horde-ci/lanes/php8.0-stable/ActiveSync: Your requirements could not be resolved to an installable set of packages.)
  • PHPStan: ❌ Setup failed (Composer install failed in /tmp/horde-ci/lanes/php8.0-stable/ActiveSync: Your requirements could not be resolved to an installable set of packages.)

php8.1-dev

  • PHPUnit: ❌ Setup failed (No result file found)
  • PHPStan: ❌ Setup failed (No result file found)

php8.1-stable

  • PHPUnit: ❌ Setup failed (No result file found)
  • PHPStan: ❌ Setup failed (No result file found)

php8.2-dev

  • PHPUnit: 2 failures, 1 error

php8.2-stable

  • PHPUnit: 2 failures, 1 error

php8.3-dev

  • PHPUnit: 2 failures, 6 errors

php8.3-stable

  • PHPUnit: 2 failures, 6 errors

php8.4-dev

  • PHPUnit: 2 failures, 6 errors
  • PHP-CS-Fixer: 23 files with issues

php8.4-stable

  • PHPUnit: 2 failures, 6 errors

php8.5-dev

  • PHPUnit: 2 failures, 6 errors

php8.5-stable

  • PHPUnit: 2 failures, 6 errors

CI powered by horde-componentsView full results

Record applied Add/Modify/Remove (and remove-as-move) under the current
SyncKey so a lost Sync response can be retried without appending another
generation of Draft messages. Dual-write email Add clientids into the
sync map; map Draft Modify old→new UIDs and suppress mirror Add/Delete
export via mailmap.

Closes #85.
@ralflang
ralflang force-pushed the feat/sync-idempotent-import branch from 747a9b8 to b12f517 Compare July 18, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant