Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ EAS code pages defined in `Wbxml.php`. Points worth knowing:
detection against the change map, `changeMessage()` / `deleteMessage()` /
`moveMessage()` calls, folder operations, and recording of client-origin
changes so they are not exported back.
A failed `changeMessage()` returns either `false` — a server error,
reported as `Status 5` so the client may retry — or
`['error' => <Status>]` naming the per-item status; backends use
`Status 6` (`STATUS_INVALID`) when they *rejected the client's data*
(e.g. a Turba attribute over its configured length), so the client stops
resending an item the server will never accept.
- `Connector_Exporter_Sync` walks the change list produced by
`getServerChanges()` and encodes each change into the response, updating
state as it goes.
Expand Down
21 changes: 14 additions & 7 deletions lib/Horde/ActiveSync/Connector/Importer.php
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ public function importMessageChange(
$uid,
$message,
$device
))) {
))
&& empty($stat['error'])) {
$stat['serverid'] = $this->_folderId;
$this->_state->updateState(
Horde_ActiveSync::CHANGE_TYPE_CHANGE,
Expand Down Expand Up @@ -322,16 +323,22 @@ public function importMessageChange(
}

// Tell the backend about the change
if (!$stat = $this->_as->driver->changeMessage($this->_folderId, $backendId, $message, $device)) {
$stat = $this->_as->driver->changeMessage($this->_folderId, $backendId, $message, $device);
if (!$stat || !empty($stat['error'])) {
// The backend may name the per-item status to report (e.g.
// STATUS_INVALID for rejected client data, which the client
// must not retry); a plain false is a retryable server error.
$status = !empty($stat['error'])
? $stat['error']
: Horde_ActiveSync_Request_Sync::STATUS_SERVERERROR;
$this->_logger->err(
sprintf(
'Change message failed when updating %s',
$id
'Change message failed when updating %s (status %d)',
$id,
$status
)
);
return $id
? [0 => $id, 'error' => Horde_ActiveSync_Request_Sync::STATUS_SERVERERROR]
: [0 => false, 'error' => Horde_ActiveSync_Request_Sync::STATUS_SERVERERROR];
return [0 => $id ?: false, 'error' => $status];
}
$stat['serverid'] = $this->_folderId;

Expand Down
9 changes: 8 additions & 1 deletion lib/Horde/ActiveSync/Driver/Base.php
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<?php

/**
Expand Down Expand Up @@ -762,7 +762,14 @@
* The activesync message
* @param Horde_ActiveSync_Device $device The device information
*
* @return array|boolean A stat array if successful, otherwise false.
* @return array|boolean A stat array if successful. On failure either
* false (server error, the client may retry
* the item) or an array with only an 'error'
* key holding the
* Horde_ActiveSync_Request_Sync::STATUS_* code
* to report for the item - STATUS_INVALID when
* the backend rejected the client's data, so
* the client stops resending it.
*/
abstract public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $message, $device);

Expand Down
137 changes: 137 additions & 0 deletions test/unit/Horde/ActiveSync/Connector/ImporterChangeStatusTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
<?php

declare(strict_types=1);

/**
* Copyright 2026 The Horde Project (http://www.horde.org/)
*
* See the enclosed file LICENSE for license information (GPL). If you
* did not receive this file, see http://www.horde.org/licenses/gpl.
*
* @author Torben Dannhauer <torben@dannhauer.de>
* @category Horde
* @copyright 2026 The Horde Project
* @license http://www.horde.org/licenses/gpl GPLv2
* @package ActiveSync
* @subpackage UnitTests
*/

use PHPUnit\Framework\TestCase;

/**
* Unit tests for the per-item status a backend may attach to a failed
* changeMessage(): rejected client data must surface as STATUS_INVALID so
* the client stops resending the item, while a plain false stays a
* retryable STATUS_SERVERERROR.
*/
class Horde_ActiveSync_Connector_ImporterChangeStatusTest extends TestCase
{
public function testBackendErrorStatusIsReportedForModify(): void
{
$driver = $this->createMock(Horde_ActiveSync_Driver_Base::class);
$driver->method('changeMessage')
->willReturn(['error' => Horde_ActiveSync_Request_Sync::STATUS_INVALID]);

$state = $this->createMock(Horde_ActiveSync_State_Base::class);
$state->method('isConflict')->willReturn(false);
$state->expects($this->never())->method('updateState');

$result = $this->_importer($state, $driver)->importMessageChange(
'contact-1',
new Horde_ActiveSync_Message_Contact(),
$this->_device(),
false,
Horde_ActiveSync::CLASS_CONTACTS
);

$this->assertSame('contact-1', $result[0]);
$this->assertSame(Horde_ActiveSync_Request_Sync::STATUS_INVALID, $result['error']);
}

public function testBackendErrorStatusIsReportedForAdd(): void
{
$driver = $this->createMock(Horde_ActiveSync_Driver_Base::class);
$driver->method('changeMessage')
->willReturn(['error' => Horde_ActiveSync_Request_Sync::STATUS_INVALID]);

$state = $this->createMock(Horde_ActiveSync_State_Base::class);
$state->method('isDuplicatePIMAddition')->willReturn(false);
$state->expects($this->never())->method('updateState');

$result = $this->_importer($state, $driver)->importMessageChange(
false,
new Horde_ActiveSync_Message_Contact(),
$this->_device(),
'client-1',
Horde_ActiveSync::CLASS_CONTACTS
);

$this->assertFalse($result[0]);
$this->assertSame(Horde_ActiveSync_Request_Sync::STATUS_INVALID, $result['error']);
}

public function testPlainFalseStaysServerError(): void
{
$driver = $this->createMock(Horde_ActiveSync_Driver_Base::class);
$driver->method('changeMessage')->willReturn(false);

$state = $this->createMock(Horde_ActiveSync_State_Base::class);
$state->method('isConflict')->willReturn(false);

$result = $this->_importer($state, $driver)->importMessageChange(
'contact-1',
new Horde_ActiveSync_Message_Contact(),
$this->_device(),
false,
Horde_ActiveSync::CLASS_CONTACTS
);

$this->assertSame('contact-1', $result[0]);
$this->assertSame(Horde_ActiveSync_Request_Sync::STATUS_SERVERERROR, $result['error']);
}

public function testSuccessfulChangeIsUnaffected(): void
{
$driver = $this->createMock(Horde_ActiveSync_Driver_Base::class);
$driver->method('changeMessage')
->willReturn(['id' => 'contact-1', 'mod' => 5, 'flags' => 1]);

$state = $this->createMock(Horde_ActiveSync_State_Base::class);
$state->method('isConflict')->willReturn(false);
$state->expects($this->once())->method('updateState');

$result = $this->_importer($state, $driver)->importMessageChange(
'contact-1',
new Horde_ActiveSync_Message_Contact(),
$this->_device(),
false,
Horde_ActiveSync::CLASS_CONTACTS
);

$this->assertSame('contact-1', $result['id']);
$this->assertArrayNotHasKey('error', $result);
}

protected function _device(): Horde_ActiveSync_Device
{
$device = $this->createMock(Horde_ActiveSync_Device::class);
$device->supported = [];
return $device;
}

protected function _importer($state, $driver): Horde_ActiveSync_Connector_Importer
{
$collections = $this->createMock(Horde_ActiveSync_Collections::class);
$collections->method('getBackendIdForFolderUid')->willReturn('@Contacts@');

$as = $this->createMock(Horde_ActiveSync::class);
$as->driver = $driver;
$as->method('getCollectionsObject')->willReturn($collections);

$importer = new Horde_ActiveSync_Connector_Importer($as);
$importer->setLogger(new Horde_Log_Logger(new Horde_Log_Handler_Null()));
$importer->init($state, 'folder-uid', 0);

return $importer;
}
}
Loading