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
35 changes: 27 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,15 @@ concurrency:

jobs:
unit-tests:
name: Unit tests (PHP ${{ matrix.php }})
name: Unit tests
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
php: ['8.1', '8.2', '8.3', '8.4']
steps:
- uses: actions/checkout@v4

- name: Set up PHP ${{ matrix.php }}
- name: Set up PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
php-version: '8.4'
coverage: none
tools: composer:v2
extensions: imap, intl, mbstring
Expand All @@ -44,6 +40,29 @@ jobs:
- name: Run unit tests
run: composer test:unit -- --colors=never

static-analysis:
name: Static analysis (Psalm)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Set up PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.4'
coverage: none
tools: composer:v2
extensions: imap, intl, mbstring

- name: Install dependencies
uses: ramsey/composer-install@v3

- name: Fetch Roundcube source for type resolution
run: composer dist:fetch

- name: Run Psalm
run: composer analyse

integration-tests:
name: Integration tests (Dovecot via Docker)
runs-on: ubuntu-latest
Expand All @@ -54,7 +73,7 @@ jobs:
- name: Set up PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.3'
php-version: '8.4'
coverage: none
tools: composer:v2
extensions: imap, intl, mbstring
Expand Down
12 changes: 8 additions & 4 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ future worker mode — must use an out-of-session store (file, DB, Roundcube cac
├── LICENSE # MIT
├── README.md # user-facing documentation
├── composer.json # plugin manifest (Roundcube plugin-installer type)
├── psalm.xml # Psalm static-analysis configuration (no baseline — every error must be fixed)
├── imapsync.php # plugin entry class (extends rcube_plugin)
├── imapsync.js # client-side: form handling, result rendering
├── config.inc.php.dist # default plugin config, copied by user to config.inc.php
Expand Down Expand Up @@ -91,7 +92,7 @@ future worker mode — must use an out-of-session store (file, DB, Roundcube cac
├── .gitattributes # export-ignore tests/, dist/, .github/, etc.
├── .github/
│ └── workflows/
│ ├── ci.yml # unit + integration tests, lint, docs-freshness
│ ├── ci.yml # unit + integration tests, Psalm, lint, docs-freshness
│ └── release.yml # tag → tarball + zip + checksums on GitHub Releases
└── dist/ # gitignored — local Roundcube source for reference (see below)
```
Expand Down Expand Up @@ -203,6 +204,8 @@ global rules win.
call.
- Run with `composer test:unit` (unit only) or `composer test:integration` (Docker, real
Dovecot) from the repo root. CI runs both.
- Run `composer analyse` for Psalm static analysis over `imapsync.php`, `lib/`, and `tests/`;
it uses the Roundcube source in `dist/` for type resolution.

### Security expectations

Expand Down Expand Up @@ -332,9 +335,9 @@ pre-commit check.

GitHub Actions (`.github/workflows/ci.yml`) runs the deterministic subset of these checks
(config-key consistency, locale-key consistency) on every push and PR as the `docs-freshness`
job, alongside `unit-tests`, `integration-tests`, and PHP linting. A failing freshness check
blocks the PR. CI is a safety net, not a substitute — run the checks locally before pushing so
you don't burn a round trip on something a 30-second grep would have caught.
job, alongside `unit-tests`, `static-analysis`, `integration-tests`, and PHP linting. A failing
freshness check blocks the PR. CI is a safety net, not a substitute — run the checks locally
before pushing so you don't burn a round trip on something a 30-second grep would have caught.

---

Expand Down Expand Up @@ -397,6 +400,7 @@ Test suite:

```bash
composer install
composer analyse # Psalm static analysis (uses dist/ for Roundcube symbols)
composer test:unit # unit tests (fast, no network, no Docker)
composer test:integration # integration tests (requires a running Docker daemon)
```
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ composer install
composer test:unit # in-memory unit tests, no network, no Docker
composer test:integration # Dovecot integration suite, requires a running Docker daemon
composer test # alias for test:unit
composer analyse # Psalm static analysis over plugin code and tests
```

The unit tests do not touch the network — the sync engine takes an injected IMAP-client
Expand Down
5 changes: 4 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,21 @@
},
"require-dev": {
"phpunit/phpunit": "^10.5",
"testcontainers/testcontainers": "^1.0"
"testcontainers/testcontainers": "^1.0",
"vimeo/psalm": "^6.0"
},
"scripts": {
"test": "@test:unit",
"test:unit": "phpunit",
"test:integration": "phpunit --configuration phpunit.integration.xml.dist",
"analyse": "psalm --no-progress",
"dist:fetch": "sh -c 'if [ -d dist/roundcubemail-1.7.0 ]; then echo \"dist/roundcubemail-1.7.0 already present, skipping.\"; exit 0; fi; curl -fLsS -o dist/roundcubemail-1.7.0-complete.tar.gz https://github.com/roundcube/roundcubemail/releases/download/1.7.0/roundcubemail-1.7.0-complete.tar.gz && tar -xzf dist/roundcubemail-1.7.0-complete.tar.gz -C dist && rm dist/roundcubemail-1.7.0-complete.tar.gz && echo \"dist/roundcubemail-1.7.0 ready.\"'"
},
"scripts-descriptions": {
"test": "Alias for test:unit.",
"test:unit": "Run the in-memory unit-test suite (fast, no Docker).",
"test:integration": "Run the Dovecot integration suite (requires a running Docker daemon).",
"analyse": "Run Psalm static analysis over imapsync.php, lib/, and tests/.",
"dist:fetch": "Download and extract Roundcube 1.7.0 source into dist/ for local reference. Idempotent."
},
"autoload": {
Expand Down
3 changes: 2 additions & 1 deletion imapsync.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class imapsync extends rcube_plugin

private rcmail $rc;

#[\Override]
public function init(): void
{
$this->rc = rcmail::get_instance();
Expand Down Expand Up @@ -278,7 +279,7 @@ public function action_status(): void
private function addFormRow(html_table $table, string $fieldId, string $labelKey, string $field): void
{
$table->add('title', html::label($fieldId, rcube::Q($this->gettext($labelKey))));
$table->add(null, $field);
$table->add('', $field);
}

private function createJobFromRequest(): RoundcubeImapSyncJob
Expand Down
18 changes: 17 additions & 1 deletion lib/RoundcubeImapSyncClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public function __construct(private readonly rcube_imap_generic $imap)
{
}

#[\Override]
public function connect(string $host, int $port, string $user, string $password, array $options): void
{
$options['port'] = $port;
Expand All @@ -44,11 +45,13 @@ public function connect(string $host, int $port, string $user, string $password,
}
}

#[\Override]
public function close(): void
{
$this->imap->closeConnection();
}

#[\Override]
public function listFolders(): array
{
$folders = $this->imap->listMailboxes('', '*');
Expand All @@ -59,18 +62,21 @@ public function listFolders(): array
return array_values(array_map('strval', $folders));
}

#[\Override]
public function getHierarchyDelimiter(): string
{
$delimiter = $this->imap->getHierarchyDelimiter();

return is_string($delimiter) && $delimiter !== '' ? $delimiter : '/';
}

#[\Override]
public function createFolder(string $folder): bool
{
return $this->imap->createFolder($folder);
}

#[\Override]
public function selectFolder(string $folder): int
{
$status = $this->imap->status($folder, ['MESSAGES']);
Expand All @@ -81,6 +87,7 @@ public function selectFolder(string $folder): int
return (int) ($status['MESSAGES'] ?? 0);
}

#[\Override]
public function getFolderSize(string $folder): int
{
if ($this->supportsStatusSize()) {
Expand Down Expand Up @@ -108,6 +115,7 @@ public function getFolderSize(string $folder): int
return $totalSize;
}

#[\Override]
public function getQuota(string $folder): ?array
{
$quota = $this->imap->getQuota($folder);
Expand All @@ -121,6 +129,7 @@ public function getQuota(string $folder): ?array
];
}

#[\Override]
public function supportsStatusSize(): bool
{
if ($this->statusSizeSupported === null) {
Expand All @@ -130,6 +139,7 @@ public function supportsStatusSize(): bool
return $this->statusSizeSupported;
}

#[\Override]
public function fetchMessageIdentities(string $folder): array
{
$totalMessages = $this->selectFolder($folder);
Expand Down Expand Up @@ -160,6 +170,7 @@ public function fetchMessageIdentities(string $folder): array
return $identities;
}

#[\Override]
public function fetchMessageRaw(string $folder, int $uid): ?array
{
$messages = $this->imap->fetch($folder, (string) $uid, true, ['UID', 'RFC822', 'FLAGS', 'INTERNALDATE']);
Expand All @@ -179,6 +190,7 @@ public function fetchMessageRaw(string $folder, int $uid): ?array
];
}

#[\Override]
public function appendMessage(string $folder, string $rawMessage, array $flags, ?string $internalDate): bool
{
$message = $rawMessage;
Expand All @@ -193,7 +205,11 @@ public function appendMessage(string $folder, string $rawMessage, array $flags,
throw new RoundcubeImapSyncException($errorMessage);
}

return $result;
// rcube_imap_generic::append returns mixed: false on failure, true on
// plain success, or the appended UID (string) when the server supports
// UIDPLUS. Our interface only contracts a bool, so collapse the
// success cases to true.
return (bool) $result;
}

/**
Expand Down
24 changes: 24 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?xml version="1.0"?>
<psalm
errorLevel="6"
resolveFromConfigFile="true"
findUnusedBaselineEntry="true"
findUnusedCode="false"
cacheDirectory="/tmp/psalm-cache"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
>
<projectFiles>
<file name="imapsync.php"/>
<directory name="lib"/>
<directory name="tests"/>
</projectFiles>

<extraFiles>
<directory name="dist/roundcubemail-1.7.0/program/lib/Roundcube"/>
</extraFiles>

<!-- If errors in scope are unavoidable (Roundcube interop quirks),
add <issueHandlers> here. Prefer baselining over suppressing. -->
</psalm>
17 changes: 16 additions & 1 deletion tests/Fakes/FakeImapClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,32 @@ public function seed(string $folder, array $messages): void
$this->folders[$folder] = $messages;
}

#[\Override]
public function connect(string $host, int $port, string $user, string $password, array $options): void
{
if ($this->connectShouldFail) {
throw new RoundcubeImapSyncException('Source connect failed.');
}
}

#[\Override]
public function close(): void
{
}

#[\Override]
public function listFolders(): array
{
return array_keys($this->folders);
}

#[\Override]
public function getHierarchyDelimiter(): string
{
return $this->delimiter;
}

#[\Override]
public function createFolder(string $folder): bool
{
if (isset($this->createFolderFailures[$folder])) {
Expand All @@ -59,6 +64,7 @@ public function createFolder(string $folder): bool
return false;
}

#[\Override]
public function selectFolder(string $folder): int
{
if (!isset($this->folders[$folder])) {
Expand All @@ -68,6 +74,7 @@ public function selectFolder(string $folder): int
return count($this->folders[$folder]);
}

#[\Override]
public function getFolderSize(string $folder): int
{
if ($this->getFolderSizeShouldFail) {
Expand All @@ -90,16 +97,19 @@ public function getFolderSize(string $folder): int
return $total;
}

#[\Override]
public function getQuota(string $folder): ?array
{
return $this->quotaResult;
}

#[\Override]
public function supportsStatusSize(): bool
{
return $this->statusSizeSupported;
}

#[\Override]
public function fetchMessageIdentities(string $folder): array
{
if (!isset($this->folders[$folder])) {
Expand All @@ -114,6 +124,7 @@ public function fetchMessageIdentities(string $folder): array
return $identities;
}

#[\Override]
public function fetchMessageRaw(string $folder, int $uid): ?array
{
if (!isset($this->folders[$folder][$uid])) {
Expand All @@ -127,6 +138,7 @@ public function fetchMessageRaw(string $folder, int $uid): ?array
];
}

#[\Override]
public function appendMessage(string $folder, string $rawMessage, array $flags, ?string $internalDate): bool
{
if (isset($this->appendQuotaFailures[$folder])) {
Expand Down Expand Up @@ -158,7 +170,10 @@ private function nextUid(string $folder): int
return 1;
}

return max(array_keys($this->folders[$folder])) + 1;
/** @var non-empty-array<int, mixed> $messages */
$messages = $this->folders[$folder];

return max(array_keys($messages)) + 1;
}

private function identityFromRaw(string $rawMessage): array
Expand Down
2 changes: 2 additions & 0 deletions tests/Integration/PreflightAndQuotaIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ final class PreflightAndQuotaIntegrationTest extends TestCase
private static ?DovecotContainer $sourceContainer = null;
private static ?DovecotContainer $destinationContainer = null;

#[\Override]
public static function setUpBeforeClass(): void
{
if (getenv('DOVECOT_INTEGRATION_SKIP') === '1') {
Expand All @@ -38,6 +39,7 @@ public static function setUpBeforeClass(): void
self::seedSourceContainer();
}

#[\Override]
public static function tearDownAfterClass(): void
{
try {
Expand Down
Loading
Loading