diff --git a/README.md b/README.md index 5aafbf71..3be49e56 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,7 @@ Cross-cutting references, useful to all three: |-------|------| | Supported EAS protocol versions, negotiation, and what each version adds | [`doc/protocol-versions.md`](doc/protocol-versions.md) | | Streamed `Sync` response delivery — design, error model, tuning | [`doc/sync-streaming.md`](doc/sync-streaming.md) | +| Heartbeat polling and export performance — batched STATUS, read-only state loads, bulk export | [`doc/sync-performance.md`](doc/sync-performance.md) | | Open work and the Horde 6 roadmap | [`doc/todo.md`](doc/todo.md) | ## At a glance @@ -85,6 +86,10 @@ negotiation mechanics, and a per-version feature delta are in deferred up-sync import and WBXML keep-alives, so clients with ~30 s read timeouts survive large batches — see [`doc/sync-streaming.md`](doc/sync-streaming.md) +- **Efficiency:** heartbeat polling batches all mailbox status checks into + one IMAP round trip and reuses memoized, lock-free state reads; message + export prefetches email items in batches — see + [`doc/sync-performance.md`](doc/sync-performance.md) ## Package layout diff --git a/doc/architecture.md b/doc/architecture.md index e1d34c78..ecc61f7f 100644 --- a/doc/architecture.md +++ b/doc/architecture.md @@ -3,8 +3,9 @@ How the library works internally. Read this before changing protocol logic. Companion documents: [`protocol-versions.md`](protocol-versions.md) for per-version behaviour, [`sync-streaming.md`](sync-streaming.md) for the -streamed `Sync` delivery design, and [`todo.md`](todo.md) for the Horde 6 -refactor roadmap. +streamed `Sync` delivery design, [`sync-performance.md`](sync-performance.md) +for the heartbeat-polling and export batching design, and +[`todo.md`](todo.md) for the Horde 6 refactor roadmap. ## Component map @@ -77,7 +78,10 @@ is the heart of the library. A `Sync` request runs in two phases: `SyncCache`), enabling cheap looping sync. - `HeartbeatInterval` / `Wait` turn the request into a hanging sync: the handler polls for changes (`Collections::pollForChanges()`) until the - heartbeat expires or changes arrive. + heartbeat expires or changes arrive. Each poll iteration batches the + IMAP status checks for all pinged email folders into one round trip and + loads collection state through a lock-free, memoized read-only path — + see [`sync-performance.md`](sync-performance.md). ### Phase 2 — respond (encoder) @@ -92,8 +96,11 @@ For each collection: the streaming count cap) decides truncation, announced via `MoreAvailable` *before* the `Commands` block (MS-ASCMD ordering rule). 4. `Connector_Exporter_Sync` streams each change: fetches the item from the - driver, encodes it (`Message_*::encodeStream()`), and records it in the - change map so the client's echo is not mirrored back. + driver — prefetching upcoming email items in batches via + `getMessagesBulk()` where the driver supports it (see + [`sync-performance.md`](sync-performance.md)) — encodes it + (`Message_*::encodeStream()`), and records it in the change map so the + client's echo is not mirrored back. 5. Replies for client commands (`SyncReplies`: server ids for adds, status for failures, EAS 16 `modifiedids`) are encoded. 6. A **new sync key** is written with the resulting state. The client @@ -118,6 +125,10 @@ State backends (`State_Sql`, `State_Mongo`) persist, per device + user: - **SyncCache** (`horde_activesync_cache`): cross-request collection metadata (folder list, per-collection options, hierarchy sync key, pingable flags) enabling EAS ≥ 12.1 empty and looping Sync requests. + Saves are dirty-field aware on both backends: only properties marked + dirty are merged into the stored cache, so concurrent writers (a PING + heartbeat and a parallel SYNC) do not clobber each other's fields (see + [`sync-performance.md`](sync-performance.md)). Two sync keys per collection are kept alive so a lost response can be replayed; garbage collection trims older generations. When state and client @@ -161,7 +172,9 @@ Mail is the only class where the library itself contains substantial data logic (other classes convert in the backend): - `Imap_Adapter` wraps a `Horde_Imap_Client` factory and implements folder - stat/changes/fetch for the driver. + stat/changes/fetch for the driver, including the batched multi-mailbox + status prefetch consumed by `ping()` + ([`sync-performance.md`](sync-performance.md)). - Change detection strategies in `Imap_Strategy_*`: `Modseq` (CONDSTORE/QRESYNC servers), `Plain` (fallback polling), `Initial` (first sync). diff --git a/doc/integration.md b/doc/integration.md index 7aa1763e..c0b12d35 100644 --- a/doc/integration.md +++ b/doc/integration.md @@ -90,6 +90,22 @@ The authoritative list with full signatures and docblocks is `Horde_ActiveSync_Driver_Mock` (+ `MockConnector`) in this package is a minimal reference stack used by the unit and integration tests. +### Optional batching hooks + +Two non-abstract methods can be overridden for performance; both default +to "not supported" and the library falls back to the per-item calls, so +existing drivers need no changes: + +- `prefetchFolderStatus(array $folders)` — called once per heartbeat poll + iteration with all pinged email folder ids; batch your status lookups in + one backend round trip (the Horde driver issues a single IMAP + LIST-STATUS). +- `getMessagesBulk($folderid, array $ids, array $collection)` — fetch + several items at once during export; ids missing from the result are + fetched individually via `getMessage()`. + +Design and fallback semantics: [`sync-performance.md`](sync-performance.md). + ### Item conversion Sync items travel as `Horde_ActiveSync_Message_*` objects (typed, @@ -127,7 +143,10 @@ The state backend persists: device records and per-user device pairings, policy keys, sync state per collection + sync key, incoming-change maps (to avoid mirroring client changes back), and the `SyncCache`. If you implement your own, subclass `Horde_ActiveSync_State_Base` and keep its locking and -garbage-collection semantics — see [`architecture.md`](architecture.md). +garbage-collection semantics — including the read-only load path used by +the heartbeat poll loop (`loadState(..., ['readonly' => true])`, see +[`sync-performance.md`](sync-performance.md)) — details in +[`architecture.md`](architecture.md). ## Requirements and tests diff --git a/doc/sync-performance.md b/doc/sync-performance.md new file mode 100644 index 00000000..8be2dda0 --- /dev/null +++ b/doc/sync-performance.md @@ -0,0 +1,218 @@ +# Email sync and polling performance + +Design document for the heartbeat/PING polling and export performance work. +Internals context is in [`architecture.md`](architecture.md); the streamed +`Sync` delivery design is a separate document +([`sync-streaming.md`](sync-streaming.md)). + +Tracking issue: +[horde/ActiveSync#88](https://github.com/horde/ActiveSync/issues/88). + +## The problem + +EAS clients keep one hanging PING (or looping SYNC) request per account +open; the server polls all pinged collections in a loop until the heartbeat +expires or a change is found. Before this work, **every poll iteration** +performed, **for every pinged collection**: + +- a full sync-state load: collection lock (its own transaction), + state garbage collection, a `SELECT … FOR UPDATE` row lock, and blob + unserialization — followed by lock release; +- one IMAP `STATUS` round trip per email folder; +- a `SELECT count(*)` plus a full-blob `UPDATE` whenever the SyncCache was + saved, with the entire serialized cache written to the debug log. + +A device pinging a few dozen mail folders therefore generated hundreds of +SQL statements and IMAP round trips per minute in the steady state — with +**zero changes to report**. A plain IMAP client covers the same "anything +new?" question with one round trip per poll. Message export added an +`N+1` pattern on top: one IMAP fetch sequence per exported message. + +## Design principles + +Robustness and architectural clarity take precedence over raw speed: + +1. **Freshness invariant.** Every poll iteration compares against fresh + server data. Caches hold *our own state watermarks*, never the server's + answer: changes made by other clients (webmail, other devices, direct + IMAP) are always detected. Prefetched IMAP status is consumed exactly + once per iteration. +2. **Optimizations are hints with fallbacks.** Every new backend method is + optional and defaults to a no-op / empty result; every prefetch miss or + bulk failure falls back to the previous per-item code path with its + exact error semantics. Disabling any single optimization restores the + old behaviour. +3. **Explicit read-only vs. mutating paths.** State loads that only peek + (change polling) are now declared as such, instead of taking the same + locks as loads that precede a persisted mutation. + +## Optimization 1 — batched IMAP STATUS prefetch + +`Collections::pollForChanges()` collects the backend folder ids of all +pinged email collections once per iteration and calls +`Driver_Base::prefetchFolderStatus($folders)` — a **no-op by default**. +`Horde_Core_ActiveSync_Driver` implements it via +`Imap_Adapter::prefetchStatus()`, which issues **one** `STATUS` request for +all mailboxes (a single LIST-STATUS round trip, RFC 5819, on servers that +advertise it) with the same `FORCE_REFRESH` semantics as the per-mailbox +call. + +`Imap_Adapter::ping()` consumes the prefetched entry for its folder — +**consume-once**: the entry is removed on use, so the next iteration must +prefetch again and can never act on stale data. Missing or incomplete +entries (folder vanished, prefetch failure, no LIST-STATUS support) fall +back to the existing per-mailbox `STATUS` call, including its +`FolderGone` detection. + +Effect: N STATUS round trips per iteration become 1. + +## Optimization 2 — read-only state loads in the poll loop + +`State_Base::loadState()` accepts `['readonly' => true]`; +`Collections::initCollectionState()` passes it from the poll loop (and only +from there). A read-only load: + +- takes **no collection lock** — a polling PING can no longer block or be + blocked by a parallel SYNC of the same folder; +- runs **no state garbage collection** (GC belongs to mutating SYNC + loads); +- in `State_Sql`, replaces the `SELECT … FOR UPDATE` row lock with a plain + `SELECT`, and **memoizes** the decoded row per folder, keyed by synckey. + +Repeat iterations with an unchanged synckey are served from the memo with +**zero SQL**. The folder object is rehydrated from the raw serialized blob +on every load, so each iteration still starts from a pristine copy — +in-memory mutations of a previous iteration cannot leak (same semantics as +a database re-read). + +Memo invalidation: + +- any **mutating** load of the folder (a real SYNC in the same request), +- any state save for the folder (including the PING-positive + `save(['preservePending' => true])` checkpoint), +- a **synckey change** — a parallel SYNC advancing the state causes a memo + miss and a fresh database read (`Collections::updateCollectionsFromCache()` + refreshes synckeys every iteration, so the new key is seen promptly). + +Writes on the poll path remain safe without locks: the PING watermark +checkpoint runs in its own transaction (`_saveSyncStateRow()`), and +`updateSyncStamp()` uses an optimistic `WHERE sync_mod = ` update. +The worst interleaving with a parallel writer is a lost watermark advance, +which produces one redundant positive PING (an extra empty SYNC round +trip) — never a missed or duplicated change. + +`State_Mongo` inherits the base behaviour (no collection lock, no GC on +read-only loads) but does not add memoization; its plain document reads +were already lock-free. + +Effect: ~6–8 SQL statements per collection per iteration become 0 after +the first iteration. + +## Optimization 3 — dirty-field SyncCache saves (SQL) + +`State_Sql::saveSyncCache()` previously ignored the `$dirty` parameter, +ran `SELECT count(*)` before every save, overwrote the whole blob with the +caller's in-memory copy, and logged the entire serialized cache. Now it: + +- **honors `$dirty`** with the same semantics as the Mongo backend: only + dirty properties are merged into the *currently stored* cache + (`_mergeDirtySyncCache()`), with per-collection granularity for the + `collections` property (including removals). A PING heartbeat updating + `lasthbsyncstarted` no longer clobbers collection entries a parallel + SYNC wrote in the meantime — the SQL backend's last-write-wins race is + narrowed to genuinely conflicting fields; +- reads the stored blob in the same query that previously only counted + rows (no extra query), inserts the full cache when no row exists, and + **skips the write entirely** when nothing is dirty; +- logs the dirty property names and sizes instead of the full blob; +- falls back to a full replace when the stored blob is corrupt + (self-healing, as before). + +Dirty tracking in `SyncCache` is the contract this relies on; a missing +`bodypartprefs` dirty mark was fixed as part of this work. + +## Optimization 4 — batched message export + +`Connector_Exporter_Sync` previously fetched every exported message +individually via `Driver::getMessage()` — for mail, one IMAP +envelope/structure fetch sequence per message. Now it prefetches up to +`PREFETCH_BATCH_SIZE` (10) upcoming `CHANGE`/`DRAFT` ids (including the +bare-uid change lists of an initial sync) through +`Driver_Base::getMessagesBulk()`: + +- the default implementation returns an **empty array** (no bulk + support) — such backends keep the exact previous per-message behaviour; +- `Horde_Core_ActiveSync_Driver` implements it for email folders with a + single `Imap_Adapter::getMessages()` call (one combined + envelope/structure fetch for the batch; body fetches remain per message, + see *Deferred work*), applying the same post-processing as + `getMessage()` (`_postProcessMailMessage()`: last-verb lookup, draft + flag, iTip response import); +- any id missing from the bulk result — expunged messages, bulk errors, + unsupported backends — **falls back to a single `getMessage()` call**, + preserving the per-message error semantics the exporter's error handling + depends on (`Horde_Exception_NotFound` → drop and continue; + `TemporaryFailure` → abort request; other errors → keep batch in + `sync_pending`). Each prefetch window is attempted once; misses within a + window do not re-trigger the bulk call. + +The batch is bounded (10 messages, bodies truncated per the client's body +preferences), so peak memory stays small. + +## Backend contract summary + +Two optional driver methods were added to `Horde_ActiveSync_Driver_Base`; +both are pure optimization hints and both default to "not supported": + +| Method | Default | Purpose | +|--------|---------|---------| +| `prefetchFolderStatus(array $folders)` | no-op | About-to-poll hint; batch folder status in one backend round trip | +| `getMessagesBulk($folderid, array $ids, array $collection)` | `[]` | Fetch many messages at once; callers fall back per id | + +`State_Base::loadState()` gained the optional `$options` parameter +(`readonly`). `Imap_Adapter::getMessages()` gained the `uid_keys` option +(return array keyed by IMAP uid). + +## Observability + +In the per-device logs — meta (debug) level unless marked otherwise: + +- `Prefetched status for N of M mailboxes.` — batched STATUS worked; + N < M means some folders fell back per mailbox. +- `Unable to prefetch mailbox status, falling back to per-mailbox STATUS: …` + (notice) — LIST-STATUS/batch failure; polling continues on the old path. +- `STATE: Updating SYNC_CACHE fields [timestamp,collections] for user …` — + dirty-field save (replaces the former full-blob dump). +- `STATE: No dirty SYNC_CACHE fields …, skipping save.` — write avoided. +- `Bulk message prefetch failed, falling back to single fetches: …` + (notice) — exporter degraded to per-message fetches. + +## Testing + +- `test/unit/Horde/ActiveSync/ImapAdapterTest.php` — prefetch consumed by + `ping()` in one round trip, consume-once semantics, per-mailbox fallback + on batch failure. +- `test/unit/Horde/ActiveSync/StateTest/Sql/ReadonlyLoadStateTest.php` — + read-only loads take no locks, memo hit needs no SQL, mutating loads and + synckey changes invalidate the memo (SQLite-backed). +- `test/unit/Horde/ActiveSync/StateTest/Sql/SyncCacheSaveTest.php` — + dirty-field merge preserves concurrent writers' changes, per-collection + removal, whole-property replace, skip-when-clean (SQLite-backed). +- `test/unit/Horde/ActiveSync/Connector/ExporterPrefetchTest.php` — bulk + consumption, per-id fallback, no-bulk-support equivalence, initial-sync + bare-uid lists. + +## Deferred work + +Deliberately **not** done on FRAMEWORK_6_0; candidates for the Horde 6 +storage refactor ([`todo.md`](todo.md)): + +- Replacing the PHP `serialize()` blobs in `sync_data` / `cache_data` with + a versioned, cheaper format. +- Dedicated columns (or tables) for hot SyncCache fields (`timestamp`, + `lasthbsyncstarted`, per-collection synckeys) to make partial reads and + compare-and-swap writes natural instead of blob merges. +- Merging the per-message envelope/structure and body FETCHes in + `Imap_EasMessageBuilder` / `Imap_MessageBodyData` into one IMAP command + per batch — high risk in the message-building hot path, low benefit + until the fetches above dominate again. diff --git a/doc/todo.md b/doc/todo.md index 617bf97c..35af578e 100644 --- a/doc/todo.md +++ b/doc/todo.md @@ -1,6 +1,6 @@ # ActiveSync TODO -Last reviewed: 2026-07-14 +Last reviewed: 2026-07-24 This file tracks **remaining** work. For what the library already supports, see the doc index in the package `README.md` — in particular @@ -153,6 +153,15 @@ when a migration plan is written. - Fold `SyncCache` and per-collection state into the device object where practical. - Implement `Horde_ActiveSync_SyncKey` as a first-class type. +- Replace the PHP `serialize()` blobs in `sync_data` / `cache_data` with a + versioned, cheaper serialization format (deferred from + [#88](https://github.com/horde/ActiveSync/issues/88); see + [`sync-performance.md`](sync-performance.md) *Deferred work*). +- Dedicated columns (or tables) for hot SyncCache fields (`timestamp`, + `lasthbsyncstarted`, per-collection synckeys) so partial reads and + compare-and-swap writes replace blob merges — supersedes the interim + dirty-field merge from #88 and overlaps “Stronger SyncCache CAS / + partial-field save” above. ### Driver and backend shape @@ -197,6 +206,11 @@ when a migration plan is written. - Pass `FILTERTYPE_*` to the driver by constant, not precomputed cutoff timestamps (supersedes the near-term `INCOMPLETETASKS` fix style). - Split `getMessage()` into per-class methods with shared base logic. +- Merge the per-message envelope/structure and body FETCHes in + `Imap_EasMessageBuilder` / `Imap_MessageBodyData` into one IMAP command + per export batch (deferred from + [#88](https://github.com/horde/ActiveSync/issues/88) — high risk in the + message-building hot path for modest gain). - `Horde_ActiveSync_Change_Filter` (or equivalent) for client-specific workarounds. Some MOVEITEMS duplication issues were fixed in the past, but there is no general filter framework. @@ -310,6 +324,21 @@ active backlog; kept here so this file does not resurrect settled work. - PING `StateGone` recovery for stale collection synckeys during long poll. - **Not supported:** `POOMTASKS:Regenerate=1` (documented limitation). +### Email sync and polling performance ([#88](https://github.com/horde/ActiveSync/issues/88), 2026-07-24) + +- One batched IMAP `STATUS` (LIST-STATUS) round trip per PING/heartbeat + poll iteration instead of one per folder. +- Lock-free, memoized read-only state loads on the polling path: no + collection lock, no GC, no `SELECT … FOR UPDATE`; zero SQL on unchanged + synckeys. +- Dirty-field SyncCache saves on SQL (merge instead of blob overwrite, + skip when clean). +- Batched email fetches in `Connector_Exporter_Sync` via the optional + `Driver_Base::getMessagesBulk()`; driver implementation in `horde/core`. +- All optimizations are hints with explicit per-item fallbacks; + foreign-client change detection is unchanged. Design, invariants, and + observability: [`sync-performance.md`](sync-performance.md). + ### Stability (3.0.0-RC1 and related) - Deterministic EAS folder UIDs on cache rebuild (#81, 2026-07-09). diff --git a/lib/Horde/ActiveSync/Collections.php b/lib/Horde/ActiveSync/Collections.php index ec4d177f..d2fbadc3 100644 --- a/lib/Horde/ActiveSync/Collections.php +++ b/lib/Horde/ActiveSync/Collections.php @@ -17,6 +17,7 @@ * * @copyright 2010-2020 Horde LLC (http://www.horde.org) * @author Michael J Rubinsky + * @author Torben Dannhauer * @package ActiveSync * @internal Not intended for use outside of the ActiveSync library. */ @@ -1197,12 +1198,17 @@ public function save($preserve_folders = false) * @param array $collection The collection array. * @param boolean $requireSyncKey Require collection to have a synckey and * throw exception if it's not present. + * @param boolean $readonly Load the state for peeking only (heartbeat + * polling): no locks, no garbage collection, + * repeated loads of an unchanged synckey may + * be served from memory. + * @see Horde_ActiveSync_State_Base::loadState() * * @throws Horde_ActiveSync_Exception_InvalidRequest * @throws Horde_ActiveSync_Exception_FolderGone * @throws Horde_ActiveSync_Exception_StaleState */ - public function initCollectionState(array &$collection, $requireSyncKey = false) + public function initCollectionState(array &$collection, $requireSyncKey = false, $readonly = false) { // Clear the changes cache. $this->_changes = null; @@ -1246,7 +1252,8 @@ public function initCollectionState(array &$collection, $requireSyncKey = false) $collection, $collection['synckey'], Horde_ActiveSync::REQUEST_TYPE_SYNC, - $collection['id'] + $collection['id'], + ['readonly' => $readonly] ); } @@ -1356,12 +1363,46 @@ public function pollForChanges($heartbeat, $interval, array $options = []) } } + // Prefetch fresh IMAP status for all email folders polled below + // in a single round trip where the backend supports it + // (LIST-STATUS). Consumed by the backend's ping check; missing + // entries transparently fall back to per-mailbox STATUS. + $prefetch = []; + foreach ($this->_collections as $id => $collection) { + if (!empty($options['pingable']) + && !$this->_cache->collectionIsPingable($id)) { + continue; + } + if ($this->getCollectionClass($id) != Horde_ActiveSync::CLASS_EMAIL) { + continue; + } + if (empty($collection['serverid'])) { + try { + $serverid = $this->getBackendIdForFolderUid($id); + } catch (Horde_ActiveSync_Exception $e) { + // Leave error handling to the per-collection loop. + continue; + } + } else { + $serverid = $collection['serverid']; + } + if (!empty($serverid)) { + $prefetch[] = $serverid; + } + } + if (count($prefetch) > 1) { + $this->_as->driver->prefetchFolderStatus($prefetch); + } + // Check each collection we are interested in. foreach ($this->_collections as $id => $collection) { $stateLoaded = false; try { - $this->initCollectionState($this->_collections[$id], true); + // Read-only peek: the poll loop only detects changes; it + // never persists state under locks. Unchanged synckeys + // are rehydrated from memory instead of the database. + $this->initCollectionState($this->_collections[$id], true, true); $stateLoaded = true; } catch (Horde_ActiveSync_Exception_StaleState $e) { $this->_logger->notice( diff --git a/lib/Horde/ActiveSync/Connector/Exporter/Sync.php b/lib/Horde/ActiveSync/Connector/Exporter/Sync.php index ee3ac3df..41cd1dcd 100644 --- a/lib/Horde/ActiveSync/Connector/Exporter/Sync.php +++ b/lib/Horde/ActiveSync/Connector/Exporter/Sync.php @@ -23,6 +23,12 @@ */ class Horde_ActiveSync_Connector_Exporter_Sync extends Horde_ActiveSync_Connector_Exporter_Base { + /** + * Number of upcoming message changes to prefetch from the backend in a + * single bulk call. + */ + public const PREFETCH_BATCH_SIZE = 10; + /** * Local cache of object ids we have already dealt with. * @@ -30,6 +36,23 @@ class Horde_ActiveSync_Connector_Exporter_Sync extends Horde_ActiveSync_Connecto */ protected $_seenObjects = []; + /** + * Prefetched message objects keyed by server id, consumed by + * _sendNextChange(). @see self::_prefetchMessages() + * + * @var array + */ + protected $_prefetchedMessages = []; + + /** + * Highest step index already covered by a prefetch attempt. Prevents + * re-fetching ids the backend could not return (they fall back to a + * single-message fetch instead). + * + * @var integer + */ + protected $_prefetchUpToStep = -1; + /** * Server ids of SYNC_FETCH requests that failed with a NotFound error * during the last fetchIds() call. Used by the Sync handler to detect @@ -58,6 +81,8 @@ public function setChanges($changes, $collection = null) { parent::setChanges($changes, $collection); $this->_currentCollection = $collection; + $this->_prefetchedMessages = []; + $this->_prefetchUpToStep = -1; } /** @@ -531,11 +556,7 @@ protected function _sendNextChange() case Horde_ActiveSync::CHANGE_TYPE_CHANGE: case Horde_ActiveSync::CHANGE_TYPE_DRAFT: try { - $message = $this->_as->driver->getMessage( - $this->_currentCollection['serverid'], - $change['id'], - $this->_currentCollection - ); + $message = $this->_getChangeMessage($change); $message->flags = (isset($change['flags'])) ? $change['flags'] : false; $this->messageChange($change['id'], $message); } catch (Horde_Exception_NotFound $e) { @@ -630,4 +651,90 @@ protected function _sendNextChange() return true; } + /** + * Return the message object for an exported change, using a batched + * backend prefetch when supported. + * + * Any id not present in the prefetch cache transparently falls back to + * a single-message fetch, preserving the per-message error semantics + * (e.g. Horde_Exception_NotFound for expunged messages). + * + * @author Torben Dannhauer + * + * @param array $change The change array being exported. + * + * @return Horde_ActiveSync_Message_Base The message object. + * @throws Horde_ActiveSync_Exception, Horde_Exception_NotFound + */ + protected function _getChangeMessage(array $change) + { + $id = $change['id']; + if (!isset($this->_prefetchedMessages[$id]) + && $this->_step > $this->_prefetchUpToStep) { + $this->_prefetchMessages(); + } + if (isset($this->_prefetchedMessages[$id])) { + $message = $this->_prefetchedMessages[$id]; + unset($this->_prefetchedMessages[$id]); + return $message; + } + + return $this->_as->driver->getMessage( + $this->_currentCollection['serverid'], + $id, + $this->_currentCollection + ); + } + + /** + * Prefetch the next batch of exportable message changes in one bulk + * backend call. Ids the backend does not return (including all ids + * when the backend has no bulk support) are fetched individually by + * _getChangeMessage(). + */ + protected function _prefetchMessages() + { + $ids = []; + $total = count($this->_changes); + $last = $this->_step; + for ($i = $this->_step; $i < $total && count($ids) < self::PREFETCH_BATCH_SIZE; $i++) { + $last = $i; + $candidate = $this->_changes[$i]; + if (!is_array($candidate)) { + // Initial sync: a bare uid implies CHANGE_TYPE_CHANGE. + $ids[] = $candidate; + continue; + } + if (!empty($candidate['ignore']) || empty($candidate['id'])) { + continue; + } + if ($candidate['type'] == Horde_ActiveSync::CHANGE_TYPE_CHANGE + || $candidate['type'] == Horde_ActiveSync::CHANGE_TYPE_DRAFT) { + $ids[] = $candidate['id']; + } + } + $this->_prefetchUpToStep = $last; + $this->_prefetchedMessages = []; + + if (count($ids) < 2) { + return; + } + + try { + $this->_prefetchedMessages = $this->_as->driver->getMessagesBulk( + $this->_currentCollection['serverid'], + $ids, + $this->_currentCollection + ); + } catch (Horde_Exception $e) { + // Prefetching is an optimization only; the single-message path + // handles errors with full granularity. + $this->_logger->notice(sprintf( + 'Bulk message prefetch failed, falling back to single fetches: %s', + $e->getMessage() + )); + $this->_prefetchedMessages = []; + } + } + } diff --git a/lib/Horde/ActiveSync/Driver/Base.php b/lib/Horde/ActiveSync/Driver/Base.php index 9904ac14..003ed401 100644 --- a/lib/Horde/ActiveSync/Driver/Base.php +++ b/lib/Horde/ActiveSync/Driver/Base.php @@ -522,6 +522,22 @@ public function getSyncStamp($collection) return time(); } + /** + * Hint to the backend that the given email folders are about to be + * polled for changes, allowing implementations to prefetch status + * information for all folders in a single backend round trip. + * + * Purely an optimization hint: the default implementation is a no-op + * and backends remain fully functional without it. + * + * @author Torben Dannhauer + * + * @param array $folders An array of backend folder ids. + */ + public function prefetchFolderStatus(array $folders) + { + } + /** * Delete a folder on the server. * @@ -694,6 +710,29 @@ abstract public function statMessage($folderId, $id); */ abstract public function getMessage($folderid, $id, array $collection); + /** + * Obtain multiple ActiveSync messages from the backend in as few + * backend round trips as the driver supports. + * + * Purely an optimization hint for exporters: the returned array is + * keyed by message id and MAY omit any (or all) requested ids. Callers + * MUST fall back to self::getMessage() for missing ids to preserve + * per-message error semantics. The default implementation returns an + * empty array (no bulk support). + * + * @author Torben Dannhauer + * + * @param string $folderid The server's folder id the messages are in. + * @param array $ids The server's message ids. + * @param array $collection The collection data. @see self::getMessage() + * + * @return array Horde_ActiveSync_Message_Base objects keyed by id. + */ + public function getMessagesBulk($folderid, array $ids, array $collection) + { + return []; + } + /** * Delete a message * diff --git a/lib/Horde/ActiveSync/Imap/Adapter.php b/lib/Horde/ActiveSync/Imap/Adapter.php index dfc103dd..6271e844 100644 --- a/lib/Horde/ActiveSync/Imap/Adapter.php +++ b/lib/Horde/ActiveSync/Imap/Adapter.php @@ -56,6 +56,15 @@ class Horde_ActiveSync_Imap_Adapter */ protected $_options = []; + /** + * Prefetched mailbox status results for consumption by self::ping(), + * keyed by backend folder id (UTF-8 mailbox name). + * + * @see self::prefetchStatus() + * @var array + */ + protected $_prefetchedStatus = []; + /** * Cont'r * @@ -426,6 +435,9 @@ public function getMessageChanges( * DEFAULT: 0 (No MIME support) * - protocolversion: (float) The EAS protocol version to support. * DEFAULT: 2.5 + * - uid_keys: (boolean) If true, key the returned array by the IMAP + * message UID. Messages that could not be built are + * omitted. DEFAULT: false (numerically indexed). * * @return array An array of Horde_ActiveSync_Message_Mail objects. */ @@ -437,7 +449,12 @@ public function getMessages($folderid, array $messages, array $options = []) foreach ($results as $data) { if ($data->exists(Horde_Imap_Client::FETCH_STRUCTURE)) { try { - $ret[] = $this->_buildMailMessage($mbox, $data, $options); + $message = $this->_buildMailMessage($mbox, $data, $options); + if (!empty($options['uid_keys'])) { + $ret[$data->getUid()] = $message; + } else { + $ret[] = $message; + } } catch (Horde_Exception_NotFound $e) { $this->_logger->notice(sprintf( 'Unable to build message UID %s in %s: %s', @@ -541,32 +558,104 @@ public function moveMessage($folderid, array $ids, $newfolderid) } /** - * Ping a mailbox. This detects only if any new messages have arrived in - * the specified mailbox. + * Prefetch the IMAP status of multiple mailboxes in a single server + * round trip where possible (LIST-STATUS, RFC 5819) for later + * consumption by self::ping(). * - * @param Horde_ActiveSync_Folder_Imap $folder The folder object. + * Results are consumed exactly once: each ping() call removes its + * entry, so every poll iteration must prefetch again. This guarantees + * each iteration compares against fresh server data and changes made + * by other clients are always detected. * - * @return boolean True if changes were detected, otherwise false. - * @throws Horde_ActiveSync_Exception, Horde_ActiveSync_Exception_FolderGone + * Failures degrade gracefully: on any error the prefetch cache is left + * empty and ping() falls back to its per-mailbox STATUS call. + * + * @author Torben Dannhauer + * + * @param array $folders Backend folder ids (UTF-8 mailbox names). */ - public function ping(Horde_ActiveSync_Folder_Imap $folder) + public function prefetchStatus(array $folders) { - $mbox = new Horde_Imap_Client_Mailbox($folder->serverid()); + $this->_prefetchedStatus = []; + if (count($folders) < 2) { + // A single mailbox gains nothing over the ping() fallback. + return; + } + // Note: non-CONDSTORE servers will return a highestmodseq of 0 $status_flags = Horde_Imap_Client::STATUS_HIGHESTMODSEQ | Horde_Imap_Client::STATUS_UIDNEXT_FORCE | Horde_Imap_Client::STATUS_MESSAGES | Horde_Imap_Client::STATUS_FORCE_REFRESH; - // Get IMAP status. + $mboxes = []; + foreach ($folders as $folder) { + $mboxes[$folder] = new Horde_Imap_Client_Mailbox($folder); + } + try { - $status = $this->_getImapOb()->status($mbox, $status_flags); + $results = $this->_getImapOb()->status(array_values($mboxes), $status_flags); } catch (Horde_Imap_Client_Exception $e) { - // See if the folder disappeared. - if (!$this->_mailboxExists($mbox->utf8)) { - throw new Horde_ActiveSync_Exception_FolderGone(); + $this->_logger->notice(sprintf( + 'Unable to prefetch mailbox status, falling back to per-mailbox STATUS: %s', + $e->getMessage() + )); + return; + } + + foreach ($mboxes as $folder => $mbox) { + $key = strval($mbox); + // Only accept complete entries; anything else falls back to the + // per-mailbox STATUS in ping(). + if (isset($results[$key]['uidnext']) + && isset($results[$key]['messages']) + && isset($results[$key][Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ])) { + $this->_prefetchedStatus[$folder] = $results[$key]; + } + } + + $this->_logger->meta(sprintf( + 'Prefetched status for %d of %d mailboxes.', + count($this->_prefetchedStatus), + count($mboxes) + )); + } + + /** + * Ping a mailbox. This detects only if any new messages have arrived in + * the specified mailbox. + * + * @param Horde_ActiveSync_Folder_Imap $folder The folder object. + * + * @return boolean True if changes were detected, otherwise false. + * @throws Horde_ActiveSync_Exception, Horde_ActiveSync_Exception_FolderGone + */ + public function ping(Horde_ActiveSync_Folder_Imap $folder) + { + $mbox = new Horde_Imap_Client_Mailbox($folder->serverid()); + + if (isset($this->_prefetchedStatus[$folder->serverid()])) { + // Fresh status was prefetched for this poll iteration in a + // single round trip. Consume-once: see self::prefetchStatus(). + $status = $this->_prefetchedStatus[$folder->serverid()]; + unset($this->_prefetchedStatus[$folder->serverid()]); + } else { + // Note: non-CONDSTORE servers will return a highestmodseq of 0 + $status_flags = Horde_Imap_Client::STATUS_HIGHESTMODSEQ + | Horde_Imap_Client::STATUS_UIDNEXT_FORCE + | Horde_Imap_Client::STATUS_MESSAGES + | Horde_Imap_Client::STATUS_FORCE_REFRESH; + + // Get IMAP status. + try { + $status = $this->_getImapOb()->status($mbox, $status_flags); + } catch (Horde_Imap_Client_Exception $e) { + // See if the folder disappeared. + if (!$this->_mailboxExists($mbox->utf8)) { + throw new Horde_ActiveSync_Exception_FolderGone(); + } + throw new Horde_ActiveSync_Exception($e); } - throw new Horde_ActiveSync_Exception($e); } $this->_logger->meta( diff --git a/lib/Horde/ActiveSync/State/Base.php b/lib/Horde/ActiveSync/State/Base.php index a97e9472..c1dfebd1 100644 --- a/lib/Horde/ActiveSync/State/Base.php +++ b/lib/Horde/ActiveSync/State/Base.php @@ -16,6 +16,7 @@ * * @copyright 2009-2020 Horde LLC (http://www.horde.org) * @author Michael J Rubinsky + * @author Torben Dannhauer * @package ActiveSync */ abstract class Horde_ActiveSync_State_Base @@ -130,6 +131,15 @@ abstract class Horde_ActiveSync_State_Base */ protected $_syncPendingBlob; + /** + * True when the currently loaded state was requested read-only + * (PING/heartbeat polling peek). + * + * @see self::loadState() + * @var boolean + */ + protected $_loadReadonly = false; + /** * The type of request we are handling. * @@ -1173,18 +1183,26 @@ public static function RowCmp($a, $b) * Horde_ActiveSync::REQUEST_TYPE constant. * @param string $id The folder id this state represents. If empty * assumed to be a foldersync state. + * @param array $options Options: + * - readonly: (boolean) Load the state for peeking only (PING / + * heartbeat polling): no collection lock, no garbage + * collection, and concrete drivers may serve repeated + * loads of the same synckey from memory. Must not be + * used when the request will mutate and persist this + * state under locks. DEFAULT: false. * * @throws Horde_ActiveSync_Exception * @throws Horde_ActiveSync_Exception_StateGone * @throws Horde_ActiveSync_Exception_StaleState */ - public function loadState(array $collection, $syncKey, $type = null, $id = null) + public function loadState(array $collection, $syncKey, $type = null, $id = null, array $options = []) { // Initialize the local members. $this->_collection = $collection; $this->_changes = null; $this->_syncPendingBlob = null; $this->_type = $type; + $this->_loadReadonly = !empty($options['readonly']); // If this is a FOLDERSYNC, mock the device id. if ($type == Horde_ActiveSync::REQUEST_TYPE_FOLDERSYNC && empty($id)) { @@ -1239,6 +1257,13 @@ public function loadState(array $collection, $syncKey, $type = null, $id = null) } $this->_syncKey = $syncKey; + if ($this->_loadReadonly) { + // Read-only peek: no collection lock and no garbage collection. + // Parallel SYNC requests are never blocked by a polling PING. + $this->_loadState(); + return; + } + $this->_acquireCollectionLock(); try { // Cleanup older syncstates diff --git a/lib/Horde/ActiveSync/State/Sql.php b/lib/Horde/ActiveSync/State/Sql.php index 99426341..2263cc9e 100644 --- a/lib/Horde/ActiveSync/State/Sql.php +++ b/lib/Horde/ActiveSync/State/Sql.php @@ -164,6 +164,25 @@ class Horde_ActiveSync_State_Sql extends Horde_ActiveSync_State_Base */ protected $_collectionLockFolderId = null; + /** + * Memoized state rows for read-only (PING/heartbeat polling) loads, + * keyed by folder id. Each entry holds the synckey it was loaded for + * plus the already binary-decoded sync_mod/sync_data/sync_pending + * values. Invalidated on any mutating load or save for the folder. + * + * @see self::_loadStateReadonly() + * @var array + */ + protected $_readonlyStateMemo = []; + + /** + * Per-request cache of Horde_Db column metadata, keyed by table name. + * Schema does not change within a request. + * + * @var array + */ + protected $_columnsCache = []; + /** * Const'r * @@ -628,6 +647,14 @@ public function updateServerIdInState($uid, $serverid) */ protected function _loadState() { + if ($this->_loadReadonly) { + $this->_loadStateReadonly(); + return; + } + + // A mutating load may change the state; drop any read-only memo. + unset($this->_readonlyStateMemo[$this->_stateFolderId()]); + $this->_releaseStateRowLock(false); $results = $this->_acquireStateRowLock(); @@ -635,33 +662,148 @@ protected function _loadState() } /** - * Actually load the state data into the object from the query results. + * Load the state represented by $syncKey for peeking only (PING / + * heartbeat polling): a plain SELECT without row lock, transaction, or + * garbage collection. Repeated loads of the same folder and synckey + * within this request are served from memory without touching the + * database; the state objects are still rehydrated from the raw blobs + * on every call, so each poll iteration operates on a pristine copy. * - * @param array $results The results array from the state query. + * The memo is invalidated by any mutating load or save for the folder; + * a synckey advanced by a parallel SYNC misses the memo and is read + * fresh from the database. + * + * @author Torben Dannhauer + * + * @throws Horde_ActiveSync_Exception, Horde_ActiveSync_Exception_StateGone */ - protected function _loadStateFromResults($results) + protected function _loadStateReadonly() { - // Load the last known sync time for this collection - $this->_lastSyncStamp = !empty($results['sync_mod']) - ? $results['sync_mod'] - : 0; + $folderid = $this->_stateFolderId(); - // Pre-Populate the current sync timestamp in case this is only a - // Client -> Server sync. - $this->_thisSyncStamp = $this->_lastSyncStamp; + $memo = $this->_readonlyStateMemo[$folderid] ?? null; + if ($memo && $memo['synckey'] === $this->_syncKey) { + $this->_applyStateData($memo['sync_mod'], $memo['sync_data'], $memo['sync_pending']); + return; + } + unset($this->_readonlyStateMemo[$folderid]); + + $sql = 'SELECT sync_data, sync_devid, sync_mod, sync_pending FROM ' + . $this->_syncStateTable . ' WHERE sync_key = ?'; + $values = [$this->_syncKey]; + if (!empty($this->_collection['id'])) { + $sql .= ' AND sync_folderid = ?'; + $values[] = $this->_collection['id']; + } - // Restore any state or pending changes try { - $columns = $this->_db->columns($this->_syncStateTable); + $results = $this->_db->selectOne($sql, $values); } catch (Horde_Db_Exception $e) { $this->_logger->err($e->getMessage()); throw new Horde_ActiveSync_Exception($e); } + if (empty($results)) { + $this->_logger->warn( + sprintf( + 'STATE: Could not find state for synckey %s.', + $this->_syncKey + ) + ); + throw new Horde_ActiveSync_Exception_StateGone(); + } + + // Decode binary columns once; memoize the decoded strings so the + // memo stays valid on drivers returning stream resources. + $columns = $this->_columns($this->_syncStateTable); + $syncMod = !empty($results['sync_mod']) ? $results['sync_mod'] : 0; + $rawSyncData = $columns['sync_data']->binaryToString($results['sync_data']); + $rawSyncPending = !empty($results['sync_pending']) + ? $columns['sync_pending']->binaryToString($results['sync_pending']) + : ''; + + $this->_readonlyStateMemo[$folderid] = [ + 'synckey' => $this->_syncKey, + 'sync_mod' => $syncMod, + 'sync_data' => $rawSyncData, + 'sync_pending' => $rawSyncPending, + ]; + + $this->_applyStateData($syncMod, $rawSyncData, $rawSyncPending); + } + + /** + * Return the folder id identifying the current state row. + * + * @return string + */ + protected function _stateFolderId() + { + return !empty($this->_collection['id']) + ? $this->_collection['id'] + : Horde_ActiveSync::REQUEST_TYPE_FOLDERSYNC; + } + + /** + * Return (and cache per request) the column metadata for a table. + * + * @param string $table The table name. + * + * @return array The Horde_Db column objects, keyed by column name. + * @throws Horde_ActiveSync_Exception + */ + protected function _columns($table) + { + if (!isset($this->_columnsCache[$table])) { + try { + $this->_columnsCache[$table] = $this->_db->columns($table); + } catch (Horde_Db_Exception $e) { + $this->_logger->err($e->getMessage()); + throw new Horde_ActiveSync_Exception($e); + } + } + + return $this->_columnsCache[$table]; + } + + /** + * Actually load the state data into the object from the query results. + * + * @param array $results The results array from the state query. + */ + protected function _loadStateFromResults($results) + { + $columns = $this->_columns($this->_syncStateTable); $rawSyncData = $columns['sync_data']->binaryToString($results['sync_data']); - $data = $this->_unserializeState($rawSyncData); $rawSyncPending = !empty($results['sync_pending']) ? $columns['sync_pending']->binaryToString($results['sync_pending']) : ''; + + $this->_applyStateData( + !empty($results['sync_mod']) ? $results['sync_mod'] : 0, + $rawSyncData, + $rawSyncPending + ); + } + + /** + * Populate the state object from the raw (binary-decoded) state row + * values. + * + * @param integer|string $syncMod The sync_mod column value. + * @param string $rawSyncData The serialized sync_data blob. + * @param string $rawSyncPending The serialized sync_pending blob. + */ + protected function _applyStateData($syncMod, $rawSyncData, $rawSyncPending) + { + // Load the last known sync time for this collection + $this->_lastSyncStamp = !empty($syncMod) ? $syncMod : 0; + + // Pre-Populate the current sync timestamp in case this is only a + // Client -> Server sync. + $this->_thisSyncStamp = $this->_lastSyncStamp; + + // Restore any state or pending changes + $data = $this->_unserializeState($rawSyncData); $this->_syncPendingBlob = ($rawSyncPending !== '') ? $rawSyncPending : null; $pending = $this->_unserializeState($rawSyncPending); @@ -729,6 +871,10 @@ public function updateSyncStamp() $this->_releaseCollectionLock(false); throw new Horde_ActiveSync_Exception($e); } + if ($updated) { + // sync_mod changed; refresh any read-only memo. + unset($this->_readonlyStateMemo[$this->_stateFolderId()]); + } } if ($this->_stateRowLockHeld) { @@ -842,6 +988,9 @@ protected function _saveState(array $options = []) */ protected function _saveSyncStateRow(array $params) { + // State is changing; any read-only memo for this folder is stale. + unset($this->_readonlyStateMemo[$params['sync_folderid']]); + $where = [ 'sync_key = ? AND sync_folderid = ? AND sync_devid = ? AND sync_user = ?', [ @@ -2014,7 +2163,7 @@ public function getSyncCache($devid, $user, ?array $fields = null) . ' WHERE cache_devid = ? AND cache_user = ?'; try { $data = $this->_db->selectValue($sql, [$devid, $user]); - $columns = $this->_db->columns($this->_syncCacheTable); + $columns = $this->_columns($this->_syncCacheTable); $data = $columns['cache_data']->binaryToString($data); } catch (Horde_Db_Exception $e) { throw new Horde_ActiveSync_Exception($e); @@ -2054,56 +2203,129 @@ public function getSyncCache($devid, $user, ?array $fields = null) public function saveSyncCache(array $cache, $devid, $user, array $dirty = []) { $cache['timestamp'] = strval($cache['timestamp']); - $sql = 'SELECT count(*) FROM ' . $this->_syncCacheTable + + // Load the currently stored cache. Also tells us whether a row + // exists at all (replaces the former SELECT count(*)). + $sql = 'SELECT cache_data FROM ' . $this->_syncCacheTable . ' WHERE cache_devid = ? AND cache_user = ?'; try { - $have = $this->_db->selectValue($sql, [$devid, $user]); + $stored_raw = $this->_db->selectValue($sql, [$devid, $user]); } catch (Horde_Db_Exception $e) { $this->_logger->err($e->getMessage()); throw new Horde_ActiveSync_Exception($e); } - $cache = serialize($cache); - if ($have) { + + if (!$stored_raw) { + // No existing row; persist the full cache. $this->_logger->meta( sprintf( - 'STATE: Replacing SYNC_CACHE entry for user %s and device %s: %s', + 'STATE: Adding new SYNC_CACHE entry for user %s and device %s.', $user, - $devid, - $cache + $devid ) ); - $sql = 'UPDATE ' . $this->_syncCacheTable - . ' SET cache_data = ? WHERE cache_devid = ? AND cache_user = ?'; + $sql = 'INSERT INTO ' . $this->_syncCacheTable + . ' (cache_data, cache_devid, cache_user) VALUES (?, ?, ?)'; try { - $this->_db->update( + $this->_db->insert( $sql, - [$cache, $devid, $user] + [serialize($cache), $devid, $user] ); } catch (Horde_Db_Exception $e) { $this->_logger->err($e->getMessage()); throw new Horde_ActiveSync_Exception($e); } - } else { + return; + } + + if (empty($dirty)) { + // Nothing changed; avoid the write completely. $this->_logger->meta( sprintf( - 'STATE: Adding new SYNC_CACHE entry for user %s and device %s: %s', + 'STATE: No dirty SYNC_CACHE fields for user %s and device %s, skipping save.', $user, - $devid, - $cache + $devid ) ); - $sql = 'INSERT INTO ' . $this->_syncCacheTable - . ' (cache_data, cache_devid, cache_user) VALUES (?, ?, ?)'; - try { - $this->_db->insert( - $sql, - [$cache, $devid, $user] - ); - } catch (Horde_Db_Exception $e) { - $this->_logger->err($e->getMessage()); - throw new Horde_ActiveSync_Exception($e); + return; + } + + // Merge only the dirty properties into the stored data so parallel + // requests do not clobber each other's changes. Mirrors the + // field-level updates of the Mongo backend. A corrupt stored blob + // falls back to a full replace with the in-memory cache. + $columns = $this->_columns($this->_syncCacheTable); + $stored = $this->_unserializeState( + $columns['cache_data']->binaryToString($stored_raw) + ); + $data = is_array($stored) + ? $this->_mergeDirtySyncCache($stored, $cache, $dirty) + : $cache; + + $this->_logger->meta( + sprintf( + 'STATE: Updating SYNC_CACHE fields [%s] for user %s and device %s.', + implode(',', array_keys($dirty)), + $user, + $devid + ) + ); + $sql = 'UPDATE ' . $this->_syncCacheTable + . ' SET cache_data = ? WHERE cache_devid = ? AND cache_user = ?'; + try { + $this->_db->update( + $sql, + [serialize($data), $devid, $user] + ); + } catch (Horde_Db_Exception $e) { + $this->_logger->err($e->getMessage()); + throw new Horde_ActiveSync_Exception($e); + } + } + + /** + * Merge the dirty properties of an in-memory sync cache into the stored + * sync cache data. + * + * The 'collections' property supports per-collection granularity: when + * its dirty entry is an array, only the listed collection ids are + * replaced (or removed when no longer present in the in-memory cache). + * A dirty entry of true replaces the whole property, as for any other + * property. + * + * @author Torben Dannhauer + * + * @param array $stored The currently stored sync cache data. + * @param array $cache The in-memory sync cache data being saved. + * @param array $dirty The dirty property map. + * + * @return array The merged sync cache data. + */ + protected function _mergeDirtySyncCache(array $stored, array $cache, array $dirty) + { + foreach ($dirty as $property => $value) { + if ($property == 'collections' && is_array($value)) { + if (!isset($stored['collections']) || !is_array($stored['collections'])) { + $stored['collections'] = []; + } + foreach (array_keys($value) as $id) { + if (isset($cache['collections'][$id])) { + $stored['collections'][$id] = $cache['collections'][$id]; + } else { + // Collection was removed from the cache. + unset($stored['collections'][$id]); + } + } + continue; + } + if (array_key_exists($property, $cache)) { + $stored[$property] = $cache[$property]; + } else { + unset($stored[$property]); } } + + return $stored; } /** diff --git a/lib/Horde/ActiveSync/SyncCache.php b/lib/Horde/ActiveSync/SyncCache.php index 65cbafed..b54fe479 100644 --- a/lib/Horde/ActiveSync/SyncCache.php +++ b/lib/Horde/ActiveSync/SyncCache.php @@ -603,6 +603,7 @@ public function updateCollection(array $collection, array $options = []) } if (isset($collection['bodypartprefs'])) { $this->_data['collections'][$collection['id']]['bodypartprefs'] = $collection['bodypartprefs']; + $this->_markCollectionsDirty($collection['id']); } if (isset($collection['pingable'])) { $this->_data['collections'][$collection['id']]['pingable'] = $collection['pingable']; diff --git a/test/unit/Horde/ActiveSync/Connector/ExporterPrefetchTest.php b/test/unit/Horde/ActiveSync/Connector/ExporterPrefetchTest.php new file mode 100644 index 00000000..b4d4a826 --- /dev/null +++ b/test/unit/Horde/ActiveSync/Connector/ExporterPrefetchTest.php @@ -0,0 +1,171 @@ + + * @license http://www.horde.org/licenses/gpl GPLv2 + * @category Horde + * @package Horde_ActiveSync + * @subpackage UnitTests + */ + +namespace Horde\ActiveSync\Connector; + +use PHPUnit\Framework\TestCase; +use Horde_ActiveSync; +use Horde_ActiveSync_Connector_Exporter_Sync; +use Horde_ActiveSync_Driver_Base; +use Horde_ActiveSync_Log_Logger; +use Horde_ActiveSync_Message_Base; +use Horde_Log_Handler_Null; +use ReflectionClass; + +class ExporterPrefetchTest extends TestCase +{ + /** + * Messages returned by the bulk call must be consumed without any + * single-message fetches. + */ + public function testBulkPrefetchConsumedWithoutSingleFetches() + { + $msgA = $this->createMock(Horde_ActiveSync_Message_Base::class); + $msgB = $this->createMock(Horde_ActiveSync_Message_Base::class); + + $driver = $this->createMock(Horde_ActiveSync_Driver_Base::class); + $driver->expects($this->once()) + ->method('getMessagesBulk') + ->with('INBOX', [10, 11]) + ->willReturn([10 => $msgA, 11 => $msgB]); + $driver->expects($this->never())->method('getMessage'); + + $exporter = $this->_exporterFixture($driver, [ + ['id' => 10, 'type' => Horde_ActiveSync::CHANGE_TYPE_CHANGE], + ['id' => 11, 'type' => Horde_ActiveSync::CHANGE_TYPE_CHANGE], + ]); + + $this->assertSame($msgA, $this->_getChangeMessage($exporter, ['id' => 10], 0)); + $this->assertSame($msgB, $this->_getChangeMessage($exporter, ['id' => 11], 1)); + } + + /** + * Ids missing from the bulk result must fall back to a single fetch, + * without re-triggering the bulk call. + */ + public function testMissingIdsFallBackToSingleFetch() + { + $msgA = $this->createMock(Horde_ActiveSync_Message_Base::class); + $msgB = $this->createMock(Horde_ActiveSync_Message_Base::class); + + $driver = $this->createMock(Horde_ActiveSync_Driver_Base::class); + $driver->expects($this->once()) + ->method('getMessagesBulk') + ->willReturn([10 => $msgA]); + $driver->expects($this->once()) + ->method('getMessage') + ->with('INBOX', 11) + ->willReturn($msgB); + + $exporter = $this->_exporterFixture($driver, [ + ['id' => 10, 'type' => Horde_ActiveSync::CHANGE_TYPE_CHANGE], + ['id' => 11, 'type' => Horde_ActiveSync::CHANGE_TYPE_CHANGE], + ]); + + $this->assertSame($msgA, $this->_getChangeMessage($exporter, ['id' => 10], 0)); + $this->assertSame($msgB, $this->_getChangeMessage($exporter, ['id' => 11], 1)); + } + + /** + * Backends without bulk support (empty result) must behave exactly as + * before: one single fetch per change, one bulk attempt per window. + */ + public function testNoBulkSupportFallsBackPerMessage() + { + $msgA = $this->createMock(Horde_ActiveSync_Message_Base::class); + $msgB = $this->createMock(Horde_ActiveSync_Message_Base::class); + + $driver = $this->createMock(Horde_ActiveSync_Driver_Base::class); + $driver->expects($this->once()) + ->method('getMessagesBulk') + ->willReturn([]); + $driver->expects($this->exactly(2)) + ->method('getMessage') + ->willReturnOnConsecutiveCalls($msgA, $msgB); + + $exporter = $this->_exporterFixture($driver, [ + ['id' => 10, 'type' => Horde_ActiveSync::CHANGE_TYPE_CHANGE], + ['id' => 11, 'type' => Horde_ActiveSync::CHANGE_TYPE_CHANGE], + ]); + + $this->assertSame($msgA, $this->_getChangeMessage($exporter, ['id' => 10], 0)); + $this->assertSame($msgB, $this->_getChangeMessage($exporter, ['id' => 11], 1)); + } + + /** + * Initial sync change lists contain bare uids; they must be prefetched + * as CHANGE_TYPE_CHANGE entries. + */ + public function testInitialSyncBareUidsArePrefetched() + { + $msgA = $this->createMock(Horde_ActiveSync_Message_Base::class); + $msgB = $this->createMock(Horde_ActiveSync_Message_Base::class); + + $driver = $this->createMock(Horde_ActiveSync_Driver_Base::class); + $driver->expects($this->once()) + ->method('getMessagesBulk') + ->with('INBOX', [10, 11]) + ->willReturn([10 => $msgA, 11 => $msgB]); + $driver->expects($this->never())->method('getMessage'); + + $exporter = $this->_exporterFixture($driver, [10, 11]); + + $this->assertSame($msgA, $this->_getChangeMessage($exporter, ['id' => 10], 0)); + $this->assertSame($msgB, $this->_getChangeMessage($exporter, ['id' => 11], 1)); + } + + /** + * Build an exporter with stubbed dependencies, bypassing the + * constructor (only the prefetch path is under test). + */ + protected function _exporterFixture($driver, array $changes) + { + $reflection = new ReflectionClass(Horde_ActiveSync_Connector_Exporter_Sync::class); + $exporter = $reflection->newInstanceWithoutConstructor(); + + $as = $this->createMock(Horde_ActiveSync::class); + $as->driver = $driver; + + $this->_setProperty($exporter, '_as', $as); + $this->_setProperty( + $exporter, + '_logger', + new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null()) + ); + $this->_setProperty($exporter, '_changes', $changes); + $this->_setProperty($exporter, '_currentCollection', [ + 'id' => 'F1', + 'serverid' => 'INBOX', + 'class' => Horde_ActiveSync::CLASS_EMAIL, + ]); + + return $exporter; + } + + protected function _getChangeMessage($exporter, array $change, $step) + { + $this->_setProperty($exporter, '_step', $step); + $reflection = new ReflectionClass($exporter); + $method = $reflection->getMethod('_getChangeMessage'); + $method->setAccessible(true); + + return $method->invoke($exporter, $change); + } + + protected function _setProperty($object, $property, $value) + { + $reflection = new ReflectionClass($object); + $prop = $reflection->getProperty($property); + $prop->setAccessible(true); + $prop->setValue($object, $value); + } +} diff --git a/test/unit/Horde/ActiveSync/ImapAdapterTest.php b/test/unit/Horde/ActiveSync/ImapAdapterTest.php index f213f731..8c13b15d 100644 --- a/test/unit/Horde/ActiveSync/ImapAdapterTest.php +++ b/test/unit/Horde/ActiveSync/ImapAdapterTest.php @@ -17,6 +17,7 @@ use Horde_ActiveSync_Folder_Imap; use Horde_ActiveSync_Imap_Adapter; use Horde_ActiveSync_Interface_ImapFactory; +use Horde_Imap_Client_Exception; use Horde_Imap_Client_Socket; #[CoversNothing] @@ -114,6 +115,136 @@ public function testBug13711() ); } + /** + * A batched status prefetch must be consumed by subsequent ping() calls + * without any further per-mailbox STATUS round trips. + */ + public function testPrefetchStatusConsumedByPing() + { + $imap_client = $this->getMockBuilder(Horde_Imap_Client_Socket::class) + ->disableOriginalConstructor() + ->onlyMethods(['status']) + ->getMock(); + // Exactly ONE status call for both folders (the batched prefetch). + $imap_client->expects($this->once()) + ->method('status') + ->with($this->isType('array')) + ->willReturn([ + 'INBOX' => [ + Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ => 600, + 'uidnext' => 101, + 'messages' => 11, + ], + 'Sent' => [ + Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ => 500, + 'uidnext' => 100, + 'messages' => 10, + ], + ]); + + $adapter = new Horde_ActiveSync_Imap_Adapter([ + 'factory' => $this->_imapFactoryFixture($imap_client), + ]); + $adapter->prefetchStatus(['INBOX', 'Sent']); + + // INBOX: prefetched modseq/uidnext advanced -> changes. + $inbox = $this->_pingReadyFolder('INBOX'); + $this->assertTrue($adapter->ping($inbox)); + + // Sent: prefetched status identical to watermark -> no changes. + $sent = $this->_pingReadyFolder('Sent'); + $this->assertFalse($adapter->ping($sent)); + } + + /** + * Prefetched entries are consumed exactly once; a second ping() of the + * same folder must issue a fresh per-mailbox STATUS. + */ + public function testPrefetchStatusConsumeOnce() + { + $imap_client = $this->getMockBuilder(Horde_Imap_Client_Socket::class) + ->disableOriginalConstructor() + ->onlyMethods(['status']) + ->getMock(); + $quiet = [ + Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ => 500, + 'uidnext' => 100, + 'messages' => 10, + ]; + // One batched call, then one per-mailbox call for the second ping. + $imap_client->expects($this->exactly(2)) + ->method('status') + ->willReturnCallback(function ($mbox) use ($quiet) { + return is_array($mbox) + ? ['INBOX' => $quiet, 'Sent' => $quiet] + : $quiet; + }); + + $adapter = new Horde_ActiveSync_Imap_Adapter([ + 'factory' => $this->_imapFactoryFixture($imap_client), + ]); + $adapter->prefetchStatus(['INBOX', 'Sent']); + + $inbox = $this->_pingReadyFolder('INBOX'); + $this->assertFalse($adapter->ping($inbox)); + $this->assertFalse($adapter->ping($inbox)); + } + + /** + * A failing batched prefetch must degrade to the per-mailbox STATUS + * path without surfacing an error. + */ + public function testPrefetchStatusFallbackOnError() + { + $imap_client = $this->getMockBuilder(Horde_Imap_Client_Socket::class) + ->disableOriginalConstructor() + ->onlyMethods(['status']) + ->getMock(); + $imap_client->expects($this->exactly(2)) + ->method('status') + ->willReturnCallback(function ($mbox) { + if (is_array($mbox)) { + throw new Horde_Imap_Client_Exception('LIST-STATUS failed'); + } + return [ + Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ => 600, + 'uidnext' => 101, + 'messages' => 11, + ]; + }); + + $adapter = new Horde_ActiveSync_Imap_Adapter([ + 'factory' => $this->_imapFactoryFixture($imap_client), + ]); + $adapter->prefetchStatus(['INBOX', 'Sent']); + + $inbox = $this->_pingReadyFolder('INBOX'); + $this->assertTrue($adapter->ping($inbox)); + } + + /** + * Build a folder with SYNC state and PING watermark at modseq 500, + * uidnext 100, 10 messages. + */ + protected function _pingReadyFolder($serverid) + { + $folder = new Horde_ActiveSync_Folder_Imap($serverid, Horde_ActiveSync::CLASS_EMAIL); + $folder->setStatus([ + Horde_ActiveSync_Folder_Imap::UIDVALIDITY => 1, + Horde_ActiveSync_Folder_Imap::UIDNEXT => 100, + Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ => 500, + Horde_ActiveSync_Folder_Imap::MESSAGES => 10, + ]); + $folder->updateState(); + $folder->acknowledgePingStatus([ + Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ => 500, + 'uidnext' => 100, + 'messages' => 10, + ]); + + return $folder; + } + protected function _imapFactoryFixture($imap_client) { return new class ($imap_client) implements Horde_ActiveSync_Interface_ImapFactory { diff --git a/test/unit/Horde/ActiveSync/StateTest/Sql/ReadonlyLoadStateTest.php b/test/unit/Horde/ActiveSync/StateTest/Sql/ReadonlyLoadStateTest.php new file mode 100644 index 00000000..42514901 --- /dev/null +++ b/test/unit/Horde/ActiveSync/StateTest/Sql/ReadonlyLoadStateTest.php @@ -0,0 +1,222 @@ + + * @license http://www.horde.org/licenses/gpl GPLv2 + * @category Horde + * @package Horde_ActiveSync + * @subpackage UnitTests + */ + +namespace Horde\ActiveSync\StateTest\Sql; + +use Horde\ActiveSync\Test\Helpers\DbHelper; +use PHPUnit\Framework\TestCase; +use Horde_ActiveSync; +use Horde_ActiveSync_Exception_StateGone; +use Horde_ActiveSync_Folder_Imap; +use Horde_ActiveSync_Log_Logger; +use Horde_ActiveSync_State_Sql; +use Horde_Db_Value_Binary; +use Horde_Log_Handler_Null; +use ReflectionClass; + +class ReadonlyLoadStateTest extends TestCase +{ + private const SYNCKEY = '{6a13541c-a3bc-448b-853c-915b00000000}14'; + private const FOLDER_ID = 'F1'; + + /** + * A read-only load must not take the collection or state row lock, and + * a repeated load of the same synckey must be served from memory (no + * database access) with a freshly rehydrated folder object. + */ + public function testReadonlyLoadIsLockFreeAndMemoized() + { + [$state, $db] = $this->_stateWithRow(); + $collection = [ + 'id' => self::FOLDER_ID, + 'class' => Horde_ActiveSync::CLASS_EMAIL, + 'serverid' => 'INBOX', + ]; + + $state->loadState( + $collection, + self::SYNCKEY, + Horde_ActiveSync::REQUEST_TYPE_SYNC, + self::FOLDER_ID, + ['readonly' => true] + ); + + $this->assertFalse($this->_getProperty($state, '_collectionLockHeld')); + $this->assertFalse($this->_getProperty($state, '_stateRowLockHeld')); + $folderFirst = $this->_getProperty($state, '_folder'); + $this->assertInstanceOf(Horde_ActiveSync_Folder_Imap::class, $folderFirst); + $this->assertEquals('INBOX', $folderFirst->serverid()); + + // Remove the row: a successful repeat load proves the memo is used + // and no SQL was issued. + $db->delete( + 'DELETE FROM horde_activesync_state WHERE sync_key = ?', + [self::SYNCKEY] + ); + + $state->loadState( + $collection, + self::SYNCKEY, + Horde_ActiveSync::REQUEST_TYPE_SYNC, + self::FOLDER_ID, + ['readonly' => true] + ); + $folderSecond = $this->_getProperty($state, '_folder'); + $this->assertEquals('INBOX', $folderSecond->serverid()); + // Rehydrated from the raw blob: pristine copy, not the same object. + $this->assertNotSame($folderFirst, $folderSecond); + } + + /** + * A synckey change must bypass the memo and read fresh data. + */ + public function testReadonlyLoadMissesMemoOnSynckeyChange() + { + [$state, $db] = $this->_stateWithRow(); + $collection = [ + 'id' => self::FOLDER_ID, + 'class' => Horde_ActiveSync::CLASS_EMAIL, + 'serverid' => 'INBOX', + ]; + + $state->loadState( + $collection, + self::SYNCKEY, + Horde_ActiveSync::REQUEST_TYPE_SYNC, + self::FOLDER_ID, + ['readonly' => true] + ); + + // A parallel SYNC advanced the state; the old row is gone and the + // new synckey has no row yet in this fixture. + $db->delete( + 'DELETE FROM horde_activesync_state WHERE sync_key = ?', + [self::SYNCKEY] + ); + + $this->expectException(Horde_ActiveSync_Exception_StateGone::class); + $state->loadState( + $collection, + '{6a13541c-a3bc-448b-853c-915b00000000}15', + Horde_ActiveSync::REQUEST_TYPE_SYNC, + self::FOLDER_ID, + ['readonly' => true] + ); + } + + /** + * A mutating (non read-only) load must invalidate the memo and hit the + * database again. + */ + public function testMutatingLoadInvalidatesMemo() + { + [$state, $db] = $this->_stateWithRow(); + $collection = [ + 'id' => self::FOLDER_ID, + 'class' => Horde_ActiveSync::CLASS_EMAIL, + 'serverid' => 'INBOX', + ]; + + $state->loadState( + $collection, + self::SYNCKEY, + Horde_ActiveSync::REQUEST_TYPE_SYNC, + self::FOLDER_ID, + ['readonly' => true] + ); + + $db->delete( + 'DELETE FROM horde_activesync_state WHERE sync_key = ?', + [self::SYNCKEY] + ); + + // The mutating load must NOT be served from the memo. + $this->expectException(Horde_ActiveSync_Exception_StateGone::class); + $state->loadState( + $collection, + self::SYNCKEY, + Horde_ActiveSync::REQUEST_TYPE_SYNC, + self::FOLDER_ID + ); + } + + /** + * Create a state object backed by SQLite with one persisted sync state + * row for FOLDER_ID / SYNCKEY. + * + * @return array [Horde_ActiveSync_State_Sql, Horde_Db_Adapter] + */ + protected function _stateWithRow() + { + if (!extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite extension is not loaded'); + } + + $migrationDir = dirname(__DIR__, 6) . '/migration/Horde/ActiveSync'; + $db = DbHelper::createSqliteDb([ + 'migrations' => [[ + 'migrationsPath' => $migrationDir, + 'schemaTableName' => 'horde_activesync_schema_info', + ]], + ]); + + $folder = new Horde_ActiveSync_Folder_Imap('INBOX', Horde_ActiveSync::CLASS_EMAIL); + $folder->setStatus([ + Horde_ActiveSync_Folder_Imap::UIDVALIDITY => 1, + Horde_ActiveSync_Folder_Imap::UIDNEXT => 100, + Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ => 500, + Horde_ActiveSync_Folder_Imap::MESSAGES => 10, + ]); + $folder->updateState(); + + $db->insertBlob('horde_activesync_state', [ + 'sync_key' => self::SYNCKEY, + 'sync_data' => new Horde_Db_Value_Binary(serialize($folder)), + 'sync_devid' => 'device', + 'sync_mod' => 500, + 'sync_folderid' => self::FOLDER_ID, + 'sync_user' => 'user@example.com', + 'sync_pending' => '', + 'sync_timestamp' => time(), + ]); + + $state = new Horde_ActiveSync_State_Sql(['db' => $db]); + $this->_setProperty( + $state, + '_logger', + new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null()) + ); + $this->_setProperty($state, '_deviceInfo', (object) [ + 'id' => 'device', + 'user' => 'user@example.com', + ]); + + return [$state, $db]; + } + + protected function _getProperty($object, $property) + { + $reflection = new ReflectionClass($object); + $prop = $reflection->getProperty($property); + $prop->setAccessible(true); + + return $prop->getValue($object); + } + + protected function _setProperty($object, $property, $value) + { + $reflection = new ReflectionClass($object); + $prop = $reflection->getProperty($property); + $prop->setAccessible(true); + $prop->setValue($object, $value); + } +} diff --git a/test/unit/Horde/ActiveSync/StateTest/Sql/SyncCacheSaveTest.php b/test/unit/Horde/ActiveSync/StateTest/Sql/SyncCacheSaveTest.php new file mode 100644 index 00000000..e75555a0 --- /dev/null +++ b/test/unit/Horde/ActiveSync/StateTest/Sql/SyncCacheSaveTest.php @@ -0,0 +1,193 @@ + + * @license http://www.horde.org/licenses/gpl GPLv2 + * @category Horde + * @package Horde_ActiveSync + * @subpackage UnitTests + */ + +namespace Horde\ActiveSync\StateTest\Sql; + +use Horde\ActiveSync\Test\Helpers\DbHelper; +use PHPUnit\Framework\TestCase; +use Horde_ActiveSync_Log_Logger; +use Horde_ActiveSync_State_Sql; +use Horde_Log_Handler_Null; +use ReflectionClass; + +class SyncCacheSaveTest extends TestCase +{ + public function testInsertNewCacheRow() + { + $state = $this->_newState(); + $cache = $this->_cacheFixture(); + + $state->saveSyncCache($cache, 'device', 'user@example.com', ['timestamp' => true]); + + $stored = $state->getSyncCache('device', 'user@example.com'); + $this->assertEquals('100', $stored['timestamp']); + $this->assertEquals(['F1' => ['class' => 'Email', 'lastsynckey' => 'a']], $stored['collections']); + } + + /** + * A save must only write its dirty fields; fields changed by a parallel + * request in the meantime must survive. + */ + public function testDirtyFieldMergePreservesConcurrentChanges() + { + $state = $this->_newState(); + $initial = $this->_cacheFixture(); + $state->saveSyncCache($initial, 'device', 'user@example.com', ['timestamp' => true]); + + // Parallel request B advances collection F1. + $cacheB = $initial; + $cacheB['collections']['F1']['lastsynckey'] = 'b'; + $cacheB['timestamp'] = 200; + $state->saveSyncCache( + $cacheB, + 'device', + 'user@example.com', + ['collections' => ['F1' => true], 'timestamp' => true] + ); + + // Request A still holds the stale in-memory cache (F1 => 'a') but + // only marked lasthbsyncstarted dirty. Its save must NOT clobber + // B's F1 change. + $cacheA = $initial; + $cacheA['lasthbsyncstarted'] = 300; + $cacheA['timestamp'] = 300; + $state->saveSyncCache( + $cacheA, + 'device', + 'user@example.com', + ['lasthbsyncstarted' => true, 'timestamp' => true] + ); + + $stored = $state->getSyncCache('device', 'user@example.com'); + $this->assertEquals('b', $stored['collections']['F1']['lastsynckey']); + $this->assertEquals(300, $stored['lasthbsyncstarted']); + $this->assertEquals('300', $stored['timestamp']); + } + + /** + * A collection removed from the in-memory cache with a per-id dirty + * entry must be removed from storage. + */ + public function testCollectionRemovalViaPerIdDirty() + { + $state = $this->_newState(); + $initial = $this->_cacheFixture(); + $initial['collections']['F2'] = ['class' => 'Email', 'lastsynckey' => 'x']; + $state->saveSyncCache($initial, 'device', 'user@example.com', ['timestamp' => true]); + + $cache = $initial; + unset($cache['collections']['F2']); + $state->saveSyncCache( + $cache, + 'device', + 'user@example.com', + ['collections' => ['F2' => true], 'timestamp' => true] + ); + + $stored = $state->getSyncCache('device', 'user@example.com'); + $this->assertArrayNotHasKey('F2', $stored['collections']); + $this->assertArrayHasKey('F1', $stored['collections']); + } + + /** + * A boolean true dirty entry for collections replaces the whole + * property (clearCollections() semantics). + */ + public function testWholeCollectionsReplaceWhenDirtyTrue() + { + $state = $this->_newState(); + $initial = $this->_cacheFixture(); + $state->saveSyncCache($initial, 'device', 'user@example.com', ['timestamp' => true]); + + $cache = $initial; + $cache['collections'] = []; + $state->saveSyncCache( + $cache, + 'device', + 'user@example.com', + ['collections' => true, 'timestamp' => true] + ); + + $stored = $state->getSyncCache('device', 'user@example.com'); + $this->assertSame([], $stored['collections']); + } + + /** + * Nothing dirty on an existing row must not write at all. + */ + public function testSkipWriteWhenNothingDirty() + { + $state = $this->_newState(); + $initial = $this->_cacheFixture(); + $state->saveSyncCache($initial, 'device', 'user@example.com', ['timestamp' => true]); + + $cache = $initial; + $cache['timestamp'] = 999; + $cache['lasthbsyncstarted'] = 999; + $state->saveSyncCache($cache, 'device', 'user@example.com', []); + + $stored = $state->getSyncCache('device', 'user@example.com'); + $this->assertEquals('100', $stored['timestamp']); + $this->assertNotEquals(999, $stored['lasthbsyncstarted']); + } + + protected function _cacheFixture() + { + return [ + 'confirmed_synckeys' => [], + 'lasthbsyncstarted' => false, + 'lastsyncendnormal' => false, + 'timestamp' => 100, + 'wait' => false, + 'hbinterval' => false, + 'folders' => [], + 'foldermap' => [], + 'hierarchy' => false, + 'collections' => ['F1' => ['class' => 'Email', 'lastsynckey' => 'a']], + 'pingheartbeat' => false, + 'synckeycounter' => [], + ]; + } + + protected function _newState() + { + if (!extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite extension is not loaded'); + } + + $migrationDir = dirname(__DIR__, 6) . '/migration/Horde/ActiveSync'; + $db = DbHelper::createSqliteDb([ + 'migrations' => [[ + 'migrationsPath' => $migrationDir, + 'schemaTableName' => 'horde_activesync_schema_info', + ]], + ]); + + $state = new Horde_ActiveSync_State_Sql(['db' => $db]); + $this->_setProperty( + $state, + '_logger', + new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null()) + ); + + return $state; + } + + protected function _setProperty($object, $property, $value) + { + $reflection = new ReflectionClass($object); + $prop = $reflection->getProperty($property); + $prop->setAccessible(true); + $prop->setValue($object, $value); + } +}