Repository navigation
feat(ops): add ActiveSync health evaluation - #107
Conversation
Add a pure state-derived health model for devices and collections, including heartbeat, FolderSync, backlog, wipe, and block signals. Document reason codes, tuning, operational limits, and the operator workflow.
🔍 CI ResultsOverall: ❌ 12/12 lanes failed TL;DR: ❌ Quality issues: PHP-CS-Fixer: 44 files. Summary by PHP Version
Quality Metrics
❌ Failed Lanesphp8.1-dev
php8.1-stable
php8.2-dev
php8.2-stable
php8.3-dev
php8.3-stable
php8.4-dev
php8.4-stable
php8.5-dev
php8.5-stable
php8.6-dev
php8.6-stable
CI powered by horde-components • View full results |
| final class CollectionFacts | ||
| { | ||
| public function __construct( | ||
| public readonly string $id, |
There was a problem hiding this comment.
Can you please investigate why the phpstan pipeline would complain here? It shouldn't.
There was a problem hiding this comment.
The annotation on this constructor is the PHP 8.0 lane parsing readonly. That keyword is valid from PHP 8.1, so the 8.0 parser reports unexpected T_STRING here. PHPStan on 8.1 through 8.5 does not report CollectionFacts.
The 8.0 lane was right by the declared constraint, though: composer.json still said ^7.4 || ^8, and this is the first code in the package that needs 8.1. The php8.0-stable lane could not resolve dependencies anyway. composer.json and .horde.yml now require ^8.1 and the 8.0 lane is dropped, matching horde/db and horde/horde.
The level-1 error the 8.1–8.5 lanes did report is Horde_Db_Migration_Base::announce() called with a second 'cli.message' argument. That call now passes only the message.
The same arity pattern exists in lib/, which CI does not scan: new Horde_ActiveSync_Request_Autodiscover($this, new Horde_ActiveSync_Device(...)). The request constructor takes the server only and reads the device from it. Autodiscover is now constructed the same way as the other commands.
| public readonly ?int $lasthbsyncstarted, | ||
| public readonly ?int $lastsyncendnormal, | ||
| public readonly int $foldersyncrequired, | ||
| public readonly array $collections |
There was a problem hiding this comment.
Can you further document the expected shape? Would it make sense to use a typed object with an ActiveSyncCollection interface [extends ArrayAccess] ?
There was a problem hiding this comment.
$collections is now a CollectionFactList: an immutable map of CollectionFacts, keyed by collection id. Array access returns the snapshot for that id.
Each snapshot copies class, serverid, lastsynckey, backlog, backlogpings, and pingable from the SyncCache row. Window size, filter type, truncation, body preferences, and conflict policy stay on the cache row. Those keys are documented on CollectionFacts and in doc/ops-monitor.md.
On the ActiveSyncCollection extends ArrayAccess idea: I put array access on the list rather than on each fact. A fact is a seven-field read-only snapshot for the evaluator, not a stand-in for the cache row, so $collection['windowsize'] on it would have to fail. A typed collection object that replaces the associative arrays in SyncCache and Sync.php is the item already noted in doc/todo.md; it makes sense, but as its own change with Horde_ActiveSync_Collections as the consumer. If you would rather have the facts themselves implement ArrayAccess for the copied keys, that is a small addition here.
There was a problem hiding this comment.
habe das Zielbild in der todo.md ergänzt.
Replace the raw collection array with an immutable id-keyed CollectionFactList. Construct Autodiscover with the server only, and call migration announce() with the message alone.
The Ops health classes use readonly properties, which need PHP 8.1. Declare that floor in composer.json and .horde.yml, and drop the 8.0 CI lane, matching horde/db and horde/horde.
Describe the four concerns mixed into the associative collection array, the target split into SyncCollection, Sync_Options, and the Changes object, the array-backed storage boundary, ArrayAccess as a removable migration shim, the conversion order, and how the Ops CollectionFacts projection folds into SyncCollection later.
2 participants
Summary
Motivation
Operators need shared, testable health semantics that can be reused by Core, CLI, and web consumers without changing protocol behavior.
Changes
Horde\ActiveSync\Opsclasses alongside the legacy treeTest plan
vendor/bin/phpunit -c vendor/horde/activesync/phpunit.xml.dist --bootstrap vendor/autoload.php vendor/horde/activesync/test/unit/Horde/ActiveSync/OpsCloses #100
Part of #99