Skip to content

fix(sync): apply all InstanceId removes for a recurring series #97

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

Merged
ralflang merged 1 commit into from
Jul 28, 2026
Merged

fix(sync): apply all InstanceId removes for a recurring series #97

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
2 changes: 1 addition & 1 deletion lib/Horde/ActiveSync/Connector/Importer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ protected function _draftModifyStat(
* @param array $ids Server message uids to delete
* @param string $class The server collection class.
* @param boolean $instanceids If true, $ids is a hash of
* instanceids => uids. @since 2.31.0
* uid => instanceid. @since 2.31.0
*
* @return array An array containing ids of successfully deleted messages.
*/
Expand Down
128 changes: 121 additions & 7 deletions lib/Horde/ActiveSync/Request/Sync.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<?php

/**
Expand Down Expand Up @@ -1125,18 +1125,100 @@
*
* @param Horde_ActiveSync_Connector_Importer $importer The importer.
* @param array $collection The collection array.
* @param array $instanceidRemoves Hash of uid => instanceid.
* @param array $instanceidRemoves Hash of uid => instanceid, or
* uid => list of instanceids (when the
* client deletes several occurrences of
* the same series in one Sync).
*/
protected function _importInstanceIdRemoves(
$importer,
array &$collection,
array $instanceidRemoves
) {
foreach ($instanceidRemoves as $uid => $instanceid) {
$importer->importMessageDeletion([$uid => $instanceid], $collection['class'], true);
foreach ($instanceidRemoves as $uid => $instanceids) {
if (!is_array($instanceids)) {
$instanceids = [$instanceids];
}
foreach ($instanceids as $instanceid) {
$importer->importMessageDeletion(
[$uid => $instanceid],
$collection['class'],
true
);
}
}
}

/**
* Count instance-delete entries, including multiple InstanceIds per uid.
*
* @param array $instanceidRemoves Hash of uid => instanceid|instanceids.
*
* @return integer
*/
protected function _countInstanceIdRemoves(array $instanceidRemoves)
{
$count = 0;
foreach ($instanceidRemoves as $instanceids) {
$count += is_array($instanceids) ? count($instanceids) : 1;
}

return $count;
}

/**
* Resolve EAS 16 instance Removes that arrived without a ServerEntryId.
*
* iOS often pairs a Remove(InstanceId) that omits ServerEntryId with an
* Add of the same recurring series in one Commands block. After the Add
* has produced a server uid via clientids, attach the orphaned InstanceIds
* to that uid so the occurrence is deleted (EXDATE) in the same sync.
*
* @param array $collection The collection array, updated in place.
*/
protected function _resolveOrphanInstanceIdRemoves(array &$collection)
{
if (empty($collection['orphan_instanceid_removes'])) {
return;
}
$orphans = $collection['orphan_instanceid_removes'];
unset($collection['orphan_instanceid_removes']);

$newUids = [];
if (!empty($collection['clientids'])) {
foreach ($collection['clientids'] as $serverid) {
if ($serverid) {
$newUids[] = $serverid;
}
}
}
$newUids = array_values(array_unique($newUids));

if (count($newUids) !== 1) {
$this->_logger->warn(sprintf(
'Dropping %d instance Remove(s) with empty ServerEntryId; cannot resolve to a single co-batched Add (found %d).',
count($orphans),
count($newUids)
));
return;
}

$uid = $newUids[0];
if (!isset($collection['instanceid_removes'][$uid])
|| !is_array($collection['instanceid_removes'][$uid])) {
$collection['instanceid_removes'][$uid] = [];
}
foreach ($orphans as $instanceid) {
$collection['instanceid_removes'][$uid][] = $instanceid;
}
$this->_logger->info(sprintf(
'Resolved %d instance Remove(s) with empty ServerEntryId to ServerEntryId %s from co-batched Add.',
count($orphans),
$uid
));
}


/**
* Import client-sent Sync commands that were queued during request
* parsing (streaming only).
Expand Down Expand Up @@ -1218,12 +1300,26 @@
$count += count($deferred['removes']);
$keepAlives += $this->_emitKeepAlive();
}
// Orphans may have been queued without a ServerEntryId; resolve them
// against a single successful co-batched Add before importing.
if (!empty($deferred['orphan_instanceid_removes'])) {
$collection['orphan_instanceid_removes']
= $deferred['orphan_instanceid_removes'];
}
if (!empty($deferred['instanceid_removes'])) {
$collection['instanceid_removes'] = $deferred['instanceid_removes'];
}
$this->_resolveOrphanInstanceIdRemoves($collection);

if (!empty($collection['instanceid_removes'])) {
$instanceCount = $this->_countInstanceIdRemoves(
$collection['instanceid_removes']
);
try {
$this->_importInstanceIdRemoves(
$importer,
$collection,
$deferred['instanceid_removes']
$collection['instanceid_removes']
);
} catch (Horde_Exception $e) {
$this->_logger->err(sprintf(
Expand All @@ -1232,7 +1328,8 @@
$e->getMessage()
));
}
$count += count($deferred['instanceid_removes']);
unset($collection['instanceid_removes']);
$count += $instanceCount;
$keepAlives += $this->_emitKeepAlive();
}

Expand Down Expand Up @@ -1715,7 +1812,17 @@

case Horde_ActiveSync::SYNC_REMOVE:
if ($instanceid) {
$collection['instanceid_removes'][$serverid] = $instanceid;
if ($serverid) {
// Several occurrence deletes for one series
// share the same ServerEntryId; keep a list.
$collection['instanceid_removes'][$serverid][]
= $instanceid;
} else {
// iOS may omit ServerEntryId when pairing the
// Remove with an Add of the same series.
$collection['orphan_instanceid_removes'][]
= $instanceid;
}
} elseif ($serverid) {
// Work around broken clients that send empty $serverid.
$collection['removes'][] = $serverid;
Expand Down Expand Up @@ -1750,6 +1857,11 @@
= $collection['instanceid_removes'];
unset($collection['instanceid_removes']);
}
if (!empty($collection['orphan_instanceid_removes'])) {
$this->_deferredCommands[$collection['id']]['orphan_instanceid_removes']
= $collection['orphan_instanceid_removes'];
unset($collection['orphan_instanceid_removes']);
}
$this->_logger->info(sprintf(
'Queued %d incoming changes for deferred import (streaming).',
$nchanges
Expand All @@ -1766,7 +1878,9 @@
);
unset($collection['removes']);
}
// EAS 16.0 instance deletions.
// Resolve InstanceId Removes that omitted ServerEntryId against
// a co-batched Add, then import all EAS 16.0 instance deletions.
$this->_resolveOrphanInstanceIdRemoves($collection);
if (!empty($collection['instanceid_removes'])
&& !empty($collection['synckey'])) {
$this->_importInstanceIdRemoves(
Expand Down
190 changes: 190 additions & 0 deletions test/unit/Horde/ActiveSync/Request/SyncStreamingTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<?php

/**
Expand Down Expand Up @@ -403,6 +403,196 @@
$this->assertSame(['id' => 'F1'], $collection);
}

public function testImportInstanceIdRemovesAcceptsMultiplePerUid()
{
$sync = $this->_syncRequestWithoutConstructor();

$importer = $this->createMock(Horde_ActiveSync_Connector_Importer::class);
$importer->expects($this->exactly(3))
->method('importMessageDeletion')
->willReturnCallback(function ($ids, $class, $instanceids) {
$this->assertTrue($instanceids);
$this->assertSame(Horde_ActiveSync::CLASS_CALENDAR, $class);
$this->assertCount(1, $ids);
return $ids;
});

$collection = [
'id' => 'A1',
'class' => Horde_ActiveSync::CLASS_CALENDAR,
];

$this->_invokeSyncMethod(
$sync,
'_importInstanceIdRemoves',
[
$importer,
&$collection,
[
'series-uid' => [
'20250801T120000Z',
'20250808T120000Z',
],
// Legacy single-string form still supported.
'other-uid' => '20250901T120000Z',
],
]
);
}

public function testResolveOrphanInstanceIdRemovesMapsToSingleAdd()
{
$sync = $this->_syncRequestWithoutConstructor();
$ref = new ReflectionClass($sync);
$loggerProp = $ref->getProperty('_logger');
$loggerProp->setAccessible(true);
$loggerProp->setValue(
$sync,
new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null())
);

$collection = [
'orphan_instanceid_removes' => [
'20260803T110000Z',
'20260810T110000Z',
],
'clientids' => [
'client-1' => 'server-uid-42',
],
'instanceid_removes' => [
'server-uid-42' => ['20260817T110000Z'],
],
];

$this->_invokeSyncMethod(
$sync,
'_resolveOrphanInstanceIdRemoves',
[&$collection]
);

$this->assertArrayNotHasKey('orphan_instanceid_removes', $collection);
$this->assertSame(
[
'server-uid-42' => [
'20260817T110000Z',
'20260803T110000Z',
'20260810T110000Z',
],
],
$collection['instanceid_removes']
);
}

public function testResolveOrphanInstanceIdRemovesDropsWhenAmbiguous()
{
$sync = $this->_syncRequestWithoutConstructor();
$ref = new ReflectionClass($sync);
$loggerProp = $ref->getProperty('_logger');
$loggerProp->setAccessible(true);
$loggerProp->setValue(
$sync,
new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null())
);

$collection = [
'orphan_instanceid_removes' => ['20260803T110000Z'],
'clientids' => [
'client-1' => 'server-a',
'client-2' => 'server-b',
],
];

$this->_invokeSyncMethod(
$sync,
'_resolveOrphanInstanceIdRemoves',
[&$collection]
);

$this->assertArrayNotHasKey('orphan_instanceid_removes', $collection);
$this->assertArrayNotHasKey('instanceid_removes', $collection);
}

public function testRunDeferredSyncCommandsImportsAllInstanceRemoves()
{
$sync = $this->_syncRequestWithoutConstructor();
$ref = new ReflectionClass($sync);

$appdata = $this->createMock(Horde_ActiveSync_Message_Base::class);

$importer = $this->createMock(Horde_ActiveSync_Connector_Importer::class);
$importer->expects($this->once())->method('init');
$importer->expects($this->once())
->method('importMessageChange')
->willReturn(['id' => 'series-uid', 'mod' => 1]);
$importer->expects($this->exactly(3))
->method('importMessageDeletion')
->willReturnCallback(function ($ids, $class, $instanceids) {
$this->assertTrue($instanceids);
$this->assertSame(['series-uid'], array_keys($ids));
return $ids;
});

$as = $this->createMock(Horde_ActiveSync::class);
$as->method('getImporter')->willReturn($importer);

$main = fopen('php://memory', 'wb+');
$encoder = $this->_encoder($main);

foreach ([
'_activeSync' => $as,
'_device' => $this->createMock(\Horde_ActiveSync_Device::class),
'_state' => $this->createMock(Horde_ActiveSync_State_Base::class),
'_encoder' => $encoder,
'_logger' => new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null()),
'_keepAliveInterval' => 15,
'_deferredCommands' => [
'A1' => [
'commands' => [
[
'type' => Horde_ActiveSync::SYNC_ADD,
'serverid' => false,
'clientid' => 'client-cal',
'appdata' => $appdata,
],
],
// Multiple deletes for the known series uid.
'instanceid_removes' => [
'series-uid' => [
'20250801T120000Z',
'20250808T120000Z',
],
],
// Orphan Remove from iOS (empty ServerEntryId), resolved
// against the co-batched Add above.
'orphan_instanceid_removes' => [
'20250815T120000Z',
],
],
],
] as $property => $value) {
$prop = $ref->getProperty($property);
$prop->setAccessible(true);
$prop->setValue($sync, $value);
}

$collection = [
'id' => 'A1',
'class' => Horde_ActiveSync::CLASS_CALENDAR,
'synckey' => '{uuid}5',
'conflict' => Horde_ActiveSync::CONFLICT_OVERWRITE_PIM,
'clientids' => [],
];

$method = $ref->getMethod('_runDeferredSyncCommands');
$method->setAccessible(true);
$collectionArgs = [&$collection];
$method->invokeArgs($sync, $collectionArgs);

$this->assertSame(['client-cal' => 'series-uid'], $collection['clientids']);
$this->assertArrayNotHasKey('instanceid_removes', $collection);
$this->assertArrayNotHasKey('orphan_instanceid_removes', $collection);
}

protected function _encoder($stream)
{
$encoder = new Horde_ActiveSync_Wbxml_Encoder(
Expand Down
Loading