diff --git a/.dockerignore b/.dockerignore
index d515c268..d356cd8f 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -4,6 +4,9 @@
**/*.log
docker-compose.override.yml
compose.override.yaml
+compose.test.yaml
+# Local deployment file — contains real secrets, must never reach an image.
+truenas.compose.yaml.local
.env.local
.env.*.local
.env.local.php
@@ -14,6 +17,13 @@ public/bundles/
assets/vendor/
.phpunit.cache/
phpunit.xml
+# Test tooling: the image serves the app, it never runs the browser suite.
+# playwright/ holds .auth/user.json — a real authenticated session.
+node_modules/
+playwright/
+playwright-report/
+blob-report/
+test-results/
.dockerignore
Dockerfile
**/Thumbs.db
diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml
index 2b0a29e0..f3fddea8 100644
--- a/.github/workflows/e2e.yml
+++ b/.github/workflows/e2e.yml
@@ -1,132 +1,135 @@
-#too many changes for now
+name: E2E
-#name: E2E
-#
-#on:
-# push:
-# pull_request:
-#
-#jobs:
-# playwright:
-# name: Playwright
-# runs-on: ubuntu-latest
-#
-# env:
-# APP_ENV: test
-# APP_DEBUG: "1"
-# DATABASE_URL: "postgresql://app:app@127.0.0.1:5432/app?serverVersion=18&charset=utf8"
-# MESSENGER_TRANSPORT_DSN: "in-memory://"
-# MAILER_DSN: "null://null"
-# APP_DEV_USER_EMAIL: "e2e@plmail.test"
-# APP_DEV_USER_PASSWORD: "e2e-password-change-me"
-#
-# services:
-# postgres:
-# image: postgres:18
-# env:
-# POSTGRES_USER: app
-# POSTGRES_PASSWORD: app
-# POSTGRES_DB: app
-# ports:
-# - 5432:5432
-# options: >-
-# --health-cmd "pg_isready -U app"
-# --health-interval 10s
-# --health-timeout 5s
-# --health-retries 5
-#
-# steps:
-# - uses: actions/checkout@v4
-#
-# - name: Setup PHP extension cache
-# id: php-extension-cache
-# uses: shivammathur/cache-extensions@v1
-# with:
-# php-version: "8.4"
-# extensions: ctype, iconv, pcntl, pdo_pgsql, pgsql, intl, sodium, mbstring, xml, curl, zip
-# key: php-extensions-v1
-#
-# - name: Cache PHP extensions
-# uses: actions/cache@v4
-# with:
-# path: ${{ steps.php-extension-cache.outputs.dir }}
-# key: ${{ steps.php-extension-cache.outputs.key }}
-# restore-keys: ${{ steps.php-extension-cache.outputs.key }}
-#
-# - name: Setup PHP
-# uses: shivammathur/setup-php@v2
-# with:
-# php-version: "8.4"
-# extensions: ctype, iconv, pcntl, pdo_pgsql, pgsql, intl, sodium, mbstring, xml, curl, zip
-# coverage: none
-# tools: composer:v2
-#
-# - name: Get Composer cache directory
-# id: composer-cache
-# run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT"
-#
-# - name: Cache Composer dependencies
-# uses: actions/cache@v4
-# with:
-# path: ${{ steps.composer-cache.outputs.dir }}
-# key: composer-${{ hashFiles('**/composer.lock') }}
-# restore-keys: composer-
-#
-# - name: Install PHP dependencies
-# run: composer install --no-interaction --prefer-dist --no-progress
-#
-# - name: Install Symfony CLI
-# run: curl -sS https://get.symfony.com/cli/installer | bash && sudo mv ~/.symfony*/bin/symfony /usr/local/bin/symfony
-#
-# - name: Setup Node
-# uses: actions/setup-node@v4
-# with:
-# node-version: 20
-# cache: npm
-#
-# - name: Install Node dependencies
-# run: npm ci
-#
-# - name: Resolve Playwright version
-# id: playwright-version
-# run: echo "version=$(node -p "require('@playwright/test/package.json').version")" >> "$GITHUB_OUTPUT"
-#
-# - name: Cache Playwright browsers
-# id: playwright-cache
-# uses: actions/cache@v4
-# with:
-# path: ~/.cache/ms-playwright
-# key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }}
-#
-# - name: Install Playwright browsers
-# if: steps.playwright-cache.outputs.cache-hit != 'true'
-# run: npx playwright install --with-deps chromium
-#
-# # Browsers came from cache; the apt libs they link against did not.
-# - name: Install Playwright system dependencies
-# if: steps.playwright-cache.outputs.cache-hit == 'true'
-# run: npx playwright install-deps chromium
-#
-# - name: Build frontend assets
-# run: |
-# php bin/console importmap:install
-# php bin/console tailwind:build
-#
-# - name: Prepare database
-# run: |
-# php bin/console doctrine:database:create --if-not-exists --no-interaction
-# php bin/console doctrine:migrations:migrate --no-interaction --allow-no-migration
-#
-# - name: Seed E2E user
-# run: php bin/console app:test:seed-user
-#
-# - name: Run Playwright tests
-# run: npx playwright test
-#
-# - name: Upload Playwright report
-# if: ${{ !cancelled() }}
-# uses: actions/upload-artifact@v4
-# with:
-# name: playwright-report
-# path: playwright-report/
-# retention-days: 14
+on:
+ push:
+ pull_request:
+
+jobs:
+ playwright:
+ name: Playwright
+ runs-on: ubuntu-latest
+
+ env:
+ APP_ENV: test
+ APP_DEBUG: "1"
+ DATABASE_URL: "postgresql://app:app@127.0.0.1:5432/app?serverVersion=18&charset=utf8"
+ MESSENGER_TRANSPORT_DSN: "in-memory://"
+ MAILER_DSN: "null://null"
+ APP_DEV_USER_EMAIL: "e2e@plmail.test"
+ APP_DEV_USER_PASSWORD: "e2e-password-change-me"
+
+ services:
+ postgres:
+ image: postgres:18
+ env:
+ POSTGRES_USER: app
+ POSTGRES_PASSWORD: app
+ POSTGRES_DB: app
+ ports:
+ - 5432:5432
+ options: >-
+ --health-cmd "pg_isready -U app"
+ --health-interval 10s
+ --health-timeout 5s
+ --health-retries 5
+
+ steps:
+ - uses: actions/checkout@v4
+
+ - name: Setup PHP extension cache
+ id: php-extension-cache
+ uses: shivammathur/cache-extensions@v1
+ with:
+ php-version: "8.4"
+ extensions: ctype, iconv, pcntl, pdo_pgsql, pgsql, intl, sodium, mbstring, xml, curl, zip
+ key: php-extensions-v1
+
+ - name: Cache PHP extensions
+ uses: actions/cache@v4
+ with:
+ path: ${{ steps.php-extension-cache.outputs.dir }}
+ key: ${{ steps.php-extension-cache.outputs.key }}
+ restore-keys: ${{ steps.php-extension-cache.outputs.key }}
+
+ - name: Setup PHP
+ uses: shivammathur/setup-php@v2
+ with:
+ php-version: "8.4"
+ extensions: ctype, iconv, pcntl, pdo_pgsql, pgsql, intl, sodium, mbstring, xml, curl, zip
+ coverage: none
+ tools: composer:v2
+
+ - name: Get Composer cache directory
+ id: composer-cache
+ run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT"
+
+ - name: Cache Composer dependencies
+ uses: actions/cache@v4
+ with:
+ path: ${{ steps.composer-cache.outputs.dir }}
+ key: composer-${{ hashFiles('**/composer.lock') }}
+ restore-keys: composer-
+
+ - name: Install PHP dependencies
+ run: composer install --no-interaction --prefer-dist --no-progress
+
+ - name: Install Symfony CLI
+ run: curl -sS https://get.symfony.com/cli/installer | bash && sudo mv ~/.symfony*/bin/symfony /usr/local/bin/symfony
+
+ - name: Setup Node
+ uses: actions/setup-node@v4
+ with:
+ node-version: 20
+ cache: npm
+
+ - name: Install Node dependencies
+ run: npm ci
+
+ - name: Resolve Playwright version
+ id: playwright-version
+ run: echo "version=$(node -p "require('@playwright/test/package.json').version")" >> "$GITHUB_OUTPUT"
+
+ - name: Cache Playwright browsers
+ id: playwright-cache
+ uses: actions/cache@v4
+ with:
+ path: ~/.cache/ms-playwright
+ key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }}
+
+ - name: Install Playwright browsers
+ if: steps.playwright-cache.outputs.cache-hit != 'true'
+ run: npx playwright install --with-deps chromium
+
+ # Browsers came from cache; the apt libs they link against did not.
+ - name: Install Playwright system dependencies
+ if: steps.playwright-cache.outputs.cache-hit == 'true'
+ run: npx playwright install-deps chromium
+
+ - name: Build frontend assets
+ run: |
+ php bin/console importmap:install
+ php bin/console tailwind:build
+
+ - name: Prepare database
+ run: |
+ php bin/console doctrine:database:create --if-not-exists --no-interaction
+ php bin/console doctrine:migrations:migrate --no-interaction --allow-no-migration
+
+ - name: Seed E2E user
+ run: php bin/console app:test:seed-user
+
+ # Fast and dependency-free, so it runs before the browser suite:
+ # a failure here is far quicker to read than a Playwright trace.
+ - name: Run PHPUnit tests
+ run: composer test
+
+ - name: Run Playwright tests
+ run: npx playwright test
+
+ - name: Upload Playwright report
+ if: ${{ !cancelled() }}
+ uses: actions/upload-artifact@v4
+ with:
+ name: playwright-report
+ path: playwright-report/
+ retention-days: 14
diff --git a/.gitignore b/.gitignore
index 00bf4723..475af11b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,3 +31,12 @@ truenas.compose.yaml.local
###> lexik/jwt-authentication-bundle ###
/config/jwt/*.pem
###< lexik/jwt-authentication-bundle ###
+
+###> playwright ###
+/node_modules/
+/test-results/
+/playwright-report/
+/blob-report/
+# Saved storage state from auth.setup.ts — a real authenticated session.
+/playwright/.auth/
+###< playwright ###
diff --git a/.nvmrc b/.nvmrc
new file mode 100644
index 00000000..2bd5a0a9
--- /dev/null
+++ b/.nvmrc
@@ -0,0 +1 @@
+22
diff --git a/.php-version b/.php-version
new file mode 100644
index 00000000..c9dc0490
--- /dev/null
+++ b/.php-version
@@ -0,0 +1 @@
+8.4
diff --git a/README.md b/README.md
index bbc816fc..f62b406d 100644
--- a/README.md
+++ b/README.md
@@ -57,6 +57,36 @@ On first run, Doctrine migrations run automatically via the entrypoint. Create y
To connect Gmail accounts, set `GOOGLE_OAUTH_CLIENT_ID`, `GOOGLE_OAUTH_CLIENT_SECRET` and `GMAIL_PUBSUB_TOPIC` in `.env.local` first.
+## Running the Tests
+
+Two suites: PHPUnit for unit tests (`tests/`, mirroring `src/`) and Playwright for browser
+end-to-end tests (`tests/e2e/`).
+
+Both run against `compose.test.yaml` — a separate compose project with its own Postgres, so it
+never touches the dev stack or the database holding your mail. Playwright itself runs on the host
+(Node 18+; the repo ships an `.nvmrc`).
+
+```bash
+npm ci # once
+npx playwright install chromium
+
+npm run test:unit:docker # PHPUnit
+npm run test:e2e:docker # Playwright (boots the stack if needed)
+npm run test:e2e:docker:ui # same, in Playwright's watch UI
+```
+
+| Command | Description |
+|---|---|
+| `npm run test:env:up` | Start the test stack (migrates, builds assets, seeds the E2E user) |
+| `npm run test:env:down` | Stop it, keeping the database volume |
+| `npm run test:env:reset` | Stop it and delete its volumes — next run rebuilds from scratch |
+| `npm run test:env:logs` | Tail the test app's logs |
+
+The app is served at `http://127.0.0.1:8001` (override with `TEST_HTTP_PORT`). Individual specs
+reseed their own fixtures, so tests are independent and re-runnable.
+
+CI runs the same suites without Docker — see [.github/workflows/e2e.yml](.github/workflows/e2e.yml).
+
## Console Commands
| Command | Description |
diff --git a/compose.test.yaml b/compose.test.yaml
new file mode 100644
index 00000000..1d250907
--- /dev/null
+++ b/compose.test.yaml
@@ -0,0 +1,101 @@
+# Local test stack — PHPUnit and the Playwright E2E suite.
+#
+# npm run test:unit:docker
+# npm run test:e2e:docker
+#
+# Deliberately its OWN compose project (see `name:` below) with its OWN Postgres,
+# so it can never reconcile, recreate or restart the dev stack, and can never
+# reach the database holding real mail. `docker compose ...` without -f is
+# untouched by this file.
+#
+# Served by the app's own FrankenPHP image rather than PHP's built-in server:
+# variables_order is GPCS, so `php -S` never exposes APP_ENV to Symfony and the
+# app silently falls back to .env.local and the dev database. A server SAPI puts
+# the environment into $_SERVER, so the values below actually take effect.
+name: pl_mail_test
+
+x-test-app: &test-app
+ # The dev target, not the ghcr image: prod is built with `composer install
+ # --no-dev` (no PHPUnit) and ships a dump-env'd .env.local.php pinning prod.
+ build:
+ context: .
+ target: frankenphp_dev
+ volumes:
+ - ./:/app
+ # A dedicated var volume. Never share the dev stack's `app_var`: it is a
+ # named volume overlaying the bind mount, so sharing it would let the two
+ # environments write each other's caches and built assets.
+ - app_var_test:/app/var
+ environment: &test-env
+ APP_ENV: test
+ APP_DEBUG: "1"
+ # Host is `database`, unlike .env.test which targets 127.0.0.1 for CI.
+ # Doctrine's when@test dbname_suffix (config/packages/doctrine.yaml) turns
+ # this into `app_test` automatically.
+ DATABASE_URL: postgresql://app:app@database:5432/app?serverVersion=18&charset=utf8
+ MESSENGER_TRANSPORT_DSN: in-memory://
+ MAILER_DSN: null://null
+ APP_DEV_USER_EMAIL: e2e@plmail.test
+ APP_DEV_USER_PASSWORD: e2e-password-change-me
+ # Same-origin so a missing hub fails fast instead of raising a cross-origin
+ # TLS error against https://localhost. There is no Mercure hub here; the
+ # suite does not assert on live updates.
+ MERCURE_PUBLIC_URL: http://127.0.0.1:${TEST_HTTP_PORT:-8001}/.well-known/mercure
+ depends_on:
+ database:
+ condition: service_healthy
+
+services:
+ database:
+ image: postgres:18-alpine
+ environment:
+ POSTGRES_DB: app
+ POSTGRES_USER: app
+ POSTGRES_PASSWORD: app
+ healthcheck:
+ test: ["CMD", "pg_isready", "-d", "app", "-U", "app"]
+ interval: 5s
+ timeout: 5s
+ retries: 10
+ volumes:
+ - database_data_test:/var/lib/postgresql
+
+ # Owns schema, assets and the seed user, then exits. `app` waits for it.
+ app-init:
+ <<: *test-app
+ # The image entrypoint waits on the database and runs migrations *before*
+ # the database exists, so it cannot bootstrap an empty volume. Drop to the
+ # upstream PHP entrypoint and do the ordering here.
+ entrypoint: ["docker-php-entrypoint"]
+ restart: "no"
+ command:
+ - sh
+ - -c
+ - |
+ set -e
+ php bin/console doctrine:database:create --if-not-exists
+ php bin/console doctrine:migrations:migrate --no-interaction --allow-no-migration
+ php bin/console importmap:install
+ # Not optional: specs assert toBeVisible()/toBeHidden(), which depend on
+ # Tailwind's `.hidden { display: none }`. Building here also keeps this
+ # stack independent of the dev `tailwind` watcher.
+ php bin/console tailwind:build
+ php bin/console app:test:seed-user
+
+ app:
+ <<: *test-app
+ environment:
+ <<: *test-env
+ # Plain HTTP on 80 inside the container — no TLS, no cert prompts.
+ SERVER_NAME: ":80"
+ ports:
+ - "${TEST_HTTP_PORT:-8001}:80"
+ depends_on:
+ database:
+ condition: service_healthy
+ app-init:
+ condition: service_completed_successfully
+
+volumes:
+ app_var_test:
+ database_data_test:
diff --git a/composer.json b/composer.json
index 0ac9d70a..55f99c41 100644
--- a/composer.json
+++ b/composer.json
@@ -100,8 +100,14 @@
],
"post-update-cmd": [
"@auto-scripts"
+ ],
+ "test": [
+ "vendor/bin/phpunit"
]
},
+ "scripts-descriptions": {
+ "test": "Run the PHPUnit suite (unit tests). Browser end-to-end tests live in tests/e2e and run via `npm run test:e2e`."
+ },
"conflict": {
"symfony/symfony": "*"
},
diff --git a/migrations/Version20260728092413.php b/migrations/Version20260728092413.php
new file mode 100644
index 00000000..d4491dd3
--- /dev/null
+++ b/migrations/Version20260728092413.php
@@ -0,0 +1,40 @@
+addSql('ALTER TABLE message ADD provider_thread_key VARCHAR(255) DEFAULT NULL');
+ $this->addSql('CREATE INDEX idx_message_message_id ON message (message_id)');
+ $this->addSql('CREATE INDEX idx_message_provider_thread_key_account ON message (provider_thread_key, account_id)');
+ $this->addSql('ALTER TABLE message_thread ADD provider_thread_key VARCHAR(255) DEFAULT NULL');
+ $this->addSql('ALTER TABLE message_thread ALTER normalized_subject TYPE TEXT');
+ $this->addSql('CREATE INDEX idx_message_thread_account_normalized_subject ON message_thread (account_id, normalized_subject)');
+ $this->addSql('CREATE UNIQUE INDEX uniq_message_thread_provider_key_account ON message_thread (provider_thread_key, account_id)');
+ }
+
+ public function down(Schema $schema): void
+ {
+ // Narrowing normalized_subject back to VARCHAR(255) will fail if any row
+ // has since exceeded it — that is the bug this migration removes.
+ $this->addSql('DROP INDEX idx_message_message_id');
+ $this->addSql('DROP INDEX idx_message_provider_thread_key_account');
+ $this->addSql('ALTER TABLE message DROP provider_thread_key');
+ $this->addSql('DROP INDEX idx_message_thread_account_normalized_subject');
+ $this->addSql('DROP INDEX uniq_message_thread_provider_key_account');
+ $this->addSql('ALTER TABLE message_thread DROP provider_thread_key');
+ $this->addSql('ALTER TABLE message_thread ALTER normalized_subject TYPE VARCHAR(255)');
+ }
+}
diff --git a/package.json b/package.json
index 342e6195..1914b829 100644
--- a/package.json
+++ b/package.json
@@ -3,12 +3,22 @@
"private": true,
"description": "End-to-end tests for plMail (Playwright). Separate from the app's AssetMapper/importmap frontend.",
"scripts": {
+ "test:env:up": "docker compose -f compose.test.yaml up -d --wait app",
+ "test:env:down": "docker compose -f compose.test.yaml down",
+ "test:env:reset": "docker compose -f compose.test.yaml down -v",
+ "test:env:logs": "docker compose -f compose.test.yaml logs -f app",
+ "test:unit:docker": "docker compose -f compose.test.yaml run --rm --no-deps --entrypoint docker-php-entrypoint app php vendor/bin/phpunit",
+ "test:e2e:docker": "npm run test:env:up && E2E_DOCKER=1 playwright test",
+ "test:e2e:docker:ui": "npm run test:env:up && E2E_DOCKER=1 playwright test --ui",
"test:e2e": "playwright test",
"test:e2e:ui": "playwright test --ui",
"test:e2e:headed": "playwright test --headed",
"test:e2e:report": "playwright show-report",
"typecheck": "tsc --noEmit -p tsconfig.json"
},
+ "engines": {
+ "node": ">=18"
+ },
"devDependencies": {
"@playwright/test": "^1.55.0",
"@types/node": "^22.10.0",
diff --git a/phpunit.dist.xml b/phpunit.dist.xml
index 22bd8791..b66977b3 100644
--- a/phpunit.dist.xml
+++ b/phpunit.dist.xml
@@ -17,9 +17,14 @@
+
tests
+ tests/e2e
diff --git a/playwright.config.ts b/playwright.config.ts
index 62578c5b..89cbf85e 100644
--- a/playwright.config.ts
+++ b/playwright.config.ts
@@ -3,12 +3,28 @@ import { defineConfig, devices } from "@playwright/test";
/**
* plMail end-to-end configuration.
*
- * The app is booted via the Symfony local server in the `test` environment.
+ * Two ways to run:
+ *
+ * npm run test:e2e:docker — against the compose.test.yaml stack (local default)
+ * npm run test:e2e — against a Symfony local server this config starts (CI)
+ *
* A dedicated `setup` project logs in through the real UI once and saves the
* authenticated storage state, which the main `chromium` project reuses so
* every non-auth spec starts already signed in.
*/
+// Docker mode just supplies defaults for the two variables that already drive
+// everything, so the rest of the config stays single-path.
+if (process.env.E2E_DOCKER) {
+ const port = process.env.TEST_HTTP_PORT ?? "8001";
+ process.env.E2E_BASE_URL ??= `http://127.0.0.1:${port}`;
+ process.env.E2E_CONSOLE ??=
+ "docker compose -f compose.test.yaml exec -T app php bin/console";
+}
+
+/** An app we did not start — so this config must not try to start one. */
+const EXTERNAL_APP = !!process.env.E2E_BASE_URL;
+
const BASE_URL = process.env.E2E_BASE_URL ?? "http://127.0.0.1:8000";
const STORAGE_STATE = "playwright/.auth/user.json";
@@ -17,7 +33,9 @@ export default defineConfig({
fullyParallel: true,
forbidOnly: !!process.env.CI,
retries: process.env.CI ? 1 : 0,
- workers: process.env.CI ? 1 : undefined,
+ // Serialised whenever the specs own the database: several reseed shared
+ // fixtures in beforeEach, so concurrent workers would clobber each other.
+ workers: process.env.CI || EXTERNAL_APP ? 1 : undefined,
reporter: process.env.CI
? [["github"], ["html", { open: "never" }]]
: [["list"], ["html", { open: "never" }]],
@@ -44,14 +62,19 @@ export default defineConfig({
},
],
- webServer: {
- command: "symfony serve --port=8000 --no-tls --allow-http",
- url: `${BASE_URL}/login`,
- reuseExistingServer: !process.env.CI,
- timeout: 120_000,
- env: {
- APP_ENV: "test",
- APP_DEBUG: "1",
- },
- },
+ // Only when we are expected to boot the app ourselves. `symfony serve` does
+ // not exist in the container image, and pointing E2E_BASE_URL elsewhere while
+ // leaving this set would still race a second server onto the same fixtures.
+ webServer: EXTERNAL_APP
+ ? undefined
+ : {
+ command: "symfony serve --port=8000 --no-tls --allow-http",
+ url: `${BASE_URL}/login`,
+ reuseExistingServer: !process.env.CI,
+ timeout: 120_000,
+ env: {
+ APP_ENV: "test",
+ APP_DEBUG: "1",
+ },
+ },
});
diff --git a/src/Command/Backfill/RethreadBackfillTask.php b/src/Command/Backfill/RethreadBackfillTask.php
new file mode 100644
index 00000000..bb97f2e9
--- /dev/null
+++ b/src/Command/Backfill/RethreadBackfillTask.php
@@ -0,0 +1,366 @@
+accountRepository->findBy(['isActive' => true]);
+
+ if (count($accounts) === 0) {
+ $io->warning('No active accounts.');
+
+ return Command::SUCCESS;
+ }
+
+ $io->note('Threads are discarded and rebuilt. Per-thread starring, snoozing, category and labels are carried over; nothing else on the thread row survives.');
+
+ if (false === $io->confirm('Rebuild all threads now?', false)) {
+ return Command::SUCCESS;
+ }
+
+ foreach ($accounts as $account) {
+ $this->runForAccount($io, $account);
+ }
+
+ return Command::SUCCESS;
+ }
+
+ private function runForAccount(SymfonyStyle $io, Account $account): void
+ {
+ $accountId = (int) $account->getId();
+
+ $io->section(sprintf('Account #%d (%s)', $accountId, $account->getEmail()));
+
+ $normalised = $this->normaliseStoredIds($io, $accountId);
+ $io->text(sprintf('Normalised message-ids on %d message(s).', $normalised));
+
+ $snapshots = $this->snapshotThreadState($accountId);
+ $io->text(sprintf('Captured state for %d existing thread(s).', count($snapshots)));
+
+ $this->discardThreads($accountId);
+
+ $rebuilt = $this->rebuildThreads($io, $accountId);
+
+ $restored = $this->restoreThreadState($accountId, $snapshots);
+
+ $io->success(sprintf(
+ 'Rethreaded %d message(s); restored state on %d thread(s).',
+ $rebuilt,
+ $restored,
+ ));
+ }
+
+ /**
+ * Strips angle brackets from message_id and from every entry in the
+ * in_reply_to / thread_references arrays, so old rows compare equal to
+ * newly-synced ones.
+ */
+ private function normaliseStoredIds(SymfonyStyle $io, int $accountId): int
+ {
+ $touched = 0;
+
+ foreach ($this->idChunks($accountId) as $ids) {
+ foreach ($this->messagesByIds($ids) as $message) {
+ $messageId = $message->getMessageId();
+ $inReplyTo = MessageIdHelper::normaliseList($message->getInReplyTo());
+ $references = MessageIdHelper::normaliseList($message->getReferences());
+
+ $normalisedId = null === $messageId ? null : MessageIdHelper::normalise($messageId);
+
+ if (
+ $normalisedId === $messageId
+ && $inReplyTo === $message->getInReplyTo()
+ && $references === $message->getReferences()
+ ) {
+ continue;
+ }
+
+ $message
+ ->setMessageId($normalisedId)
+ ->setInReplyTo($inReplyTo)
+ ->setReferences($references);
+
+ ++$touched;
+ }
+
+ $this->em->flush();
+ $this->em->clear();
+ }
+
+ return $touched;
+ }
+
+ /**
+ * Per-thread state worth keeping, anchored to the thread's earliest message
+ * so it can be found again once threads have been rebuilt.
+ *
+ * @return list}>
+ */
+ private function snapshotThreadState(int $accountId): array
+ {
+ $connection = $this->em->getConnection();
+
+ $rows = $connection->fetchAllAssociative(
+ 'SELECT t.id, t.starred_at, t.snoozed_until, t.category, MIN(m.id) AS anchor
+ FROM message_thread t
+ INNER JOIN message m ON m.thread_id = t.id
+ WHERE t.account_id = :accountId
+ GROUP BY t.id',
+ ['accountId' => $accountId],
+ );
+
+ if (count($rows) === 0) {
+ return [];
+ }
+
+ $labelsByThread = [];
+
+ foreach ($connection->fetchAllAssociative(
+ 'SELECT message_thread_id, label_id FROM thread_label WHERE message_thread_id IN (:threadIds)',
+ ['threadIds' => array_map(static fn(array $row): int => (int) $row['id'], $rows)],
+ ['threadIds' => ArrayParameterType::INTEGER],
+ ) as $row) {
+ $labelsByThread[(int) $row['message_thread_id']][] = (int) $row['label_id'];
+ }
+
+ $snapshots = [];
+
+ foreach ($rows as $row) {
+ $threadId = (int) $row['id'];
+
+ $snapshots[] = [
+ 'anchor' => (int) $row['anchor'],
+ 'starredAt' => $row['starred_at'],
+ 'snoozedUntil' => $row['snoozed_until'],
+ 'category' => $row['category'],
+ 'labels' => $labelsByThread[$threadId] ?? [],
+ ];
+ }
+
+ return $snapshots;
+ }
+
+ /**
+ * Detaches messages first, then drops the thread rows.
+ *
+ * Deliberately DBAL, not the ORM: MessageThread cascades remove onto its
+ * messages, so removing threads through the EntityManager would delete the
+ * mail along with them.
+ */
+ private function discardThreads(int $accountId): void
+ {
+ $connection = $this->em->getConnection();
+
+ $connection->executeStatement(
+ 'UPDATE message SET thread_id = NULL WHERE account_id = :accountId',
+ ['accountId' => $accountId],
+ );
+
+ $connection->executeStatement(
+ 'DELETE FROM message_thread WHERE account_id = :accountId',
+ ['accountId' => $accountId],
+ );
+
+ $this->em->clear();
+ }
+
+ /**
+ * Replays every message through the threader in arrival order, so a parent
+ * always exists by the time its replies are threaded.
+ *
+ * Flushes per message rather than per batch: two messages in one batch can
+ * belong to the same new thread, and the threader can only find that thread
+ * once it is in the database.
+ */
+ private function rebuildThreads(SymfonyStyle $io, int $accountId): int
+ {
+ $total = (int) $this->messageRepository->createQueryBuilder('m')
+ ->select('COUNT(m.id)')
+ ->where('m.account = :accountId')
+ ->setParameter('accountId', $accountId)
+ ->getQuery()
+ ->getSingleScalarResult();
+
+ if (0 === $total) {
+ return 0;
+ }
+
+ $io->progressStart($total);
+
+ $processed = 0;
+
+ foreach ($this->idChunks($accountId, orderByArrival: true) as $ids) {
+ $account = $this->em->getReference(Account::class, $accountId);
+
+ foreach ($this->messagesByIds($ids, orderByArrival: true) as $message) {
+ $this->threader->assignThread($message, $account);
+
+ $this->em->flush();
+
+ ++$processed;
+ $io->progressAdvance();
+ }
+
+ $this->em->clear();
+ }
+
+ $io->progressFinish();
+
+ return $processed;
+ }
+
+ /**
+ * @param list}> $snapshots
+ */
+ private function restoreThreadState(int $accountId, array $snapshots): int
+ {
+ if (count($snapshots) === 0) {
+ return 0;
+ }
+
+ $connection = $this->em->getConnection();
+ $restored = 0;
+
+ foreach ($snapshots as $snapshot) {
+ $threadId = $connection->fetchOne(
+ 'SELECT thread_id FROM message WHERE id = :anchor',
+ ['anchor' => $snapshot['anchor']],
+ );
+
+ if (false === $threadId || null === $threadId) {
+ continue;
+ }
+
+ // COALESCE, not assignment: several old threads can collapse into one
+ // rebuilt thread, and a value already restored by an earlier snapshot
+ // must not be blanked by a later one that happened to be empty.
+ $connection->executeStatement(
+ 'UPDATE message_thread
+ SET starred_at = COALESCE(starred_at, :starredAt),
+ snoozed_until = COALESCE(snoozed_until, :snoozedUntil),
+ category = COALESCE(category, :category)
+ WHERE id = :threadId',
+ [
+ 'starredAt' => $snapshot['starredAt'],
+ 'snoozedUntil' => $snapshot['snoozedUntil'],
+ 'category' => $snapshot['category'],
+ 'threadId' => $threadId,
+ ],
+ );
+
+ foreach ($snapshot['labels'] as $labelId) {
+ $connection->executeStatement(
+ 'INSERT INTO thread_label (message_thread_id, label_id)
+ VALUES (:threadId, :labelId)
+ ON CONFLICT DO NOTHING',
+ ['threadId' => $threadId, 'labelId' => $labelId],
+ );
+ }
+
+ ++$restored;
+ }
+
+ return $restored;
+ }
+
+ /**
+ * Ids up front, entities in chunks: the rebuild clears the EntityManager on
+ * every batch, which would invalidate a cursor held across it.
+ *
+ * @return iterable>
+ */
+ private function idChunks(int $accountId, bool $orderByArrival = false): iterable
+ {
+ $qb = $this->messageRepository->createQueryBuilder('m')
+ ->select('m.id')
+ ->where('m.account = :accountId')
+ ->setParameter('accountId', $accountId);
+
+ if (true === $orderByArrival) {
+ $qb->orderBy('m.receivedAt', 'ASC');
+ }
+
+ $qb->addOrderBy('m.id', 'ASC');
+
+ $ids = array_map(
+ static fn(array $row): int => (int) $row['id'],
+ $qb->getQuery()->getArrayResult(),
+ );
+
+ foreach (array_chunk($ids, self::BATCH_SIZE) as $chunk) {
+ yield $chunk;
+ }
+ }
+
+ /**
+ * @param list $ids
+ *
+ * @return list
+ */
+ private function messagesByIds(array $ids, bool $orderByArrival = false): array
+ {
+ $qb = $this->messageRepository->createQueryBuilder('m')
+ ->where('m.id IN (:ids)')
+ ->setParameter('ids', $ids);
+
+ if (true === $orderByArrival) {
+ $qb->orderBy('m.receivedAt', 'ASC');
+ }
+
+ $qb->addOrderBy('m.id', 'ASC');
+
+ return $qb->getQuery()->getResult();
+ }
+}
diff --git a/src/Command/Test/SeedTestLabelCommand.php b/src/Command/Test/SeedTestLabelCommand.php
index bcc5640e..7b82fe25 100644
--- a/src/Command/Test/SeedTestLabelCommand.php
+++ b/src/Command/Test/SeedTestLabelCommand.php
@@ -85,10 +85,11 @@ final class SeedTestLabelCommand extends Command
return Command::SUCCESS;
}
- $label = new Label();
- $label->account = $account;
- $label->name = self::LABEL_NAME;
- $label->color = 'blue';
+ // Label's properties are private(set); the setters are the only way in.
+ $label = new Label()
+ ->setAccount($account)
+ ->setName(self::LABEL_NAME)
+ ->setColor('blue');
$this->entityManager->persist($label);
$this->entityManager->flush();
diff --git a/src/Domain/Enum/ThreadingMethod.php b/src/Domain/Enum/ThreadingMethod.php
index 90a4d3c8..75d0b781 100644
--- a/src/Domain/Enum/ThreadingMethod.php
+++ b/src/Domain/Enum/ThreadingMethod.php
@@ -4,6 +4,8 @@ namespace App\Domain\Enum;
enum ThreadingMethod: string
{
+ /** Grouped by the provider's own conversation id (Gmail threadId, Graph conversationId). */
+ case Provider = 'provider';
case References = 'references';
case SubjectFallback = 'subject_fallback';
}
diff --git a/src/Domain/Helper/MessageIdHelper.php b/src/Domain/Helper/MessageIdHelper.php
index c10ae034..2a0d79a9 100644
--- a/src/Domain/Helper/MessageIdHelper.php
+++ b/src/Domain/Helper/MessageIdHelper.php
@@ -18,4 +18,42 @@ final class MessageIdHelper
{
return trim(trim($raw), '<> ');
}
+
+ /**
+ * Splits a raw In-Reply-To / References header into canonical message-ids.
+ *
+ * Splits on any whitespace rather than a single space: these headers are
+ * routinely folded across lines, so a CRLF sits between ids as often as a
+ * space does. Empty results are dropped so callers never have to re-check.
+ *
+ * @param string|list|null $raw the raw header, or an already-split list
+ *
+ * @return list
+ */
+ public static function normaliseList(string|array|null $raw): array
+ {
+ if (null === $raw) {
+ return [];
+ }
+
+ $parts = is_array($raw)
+ ? $raw
+ : (preg_split('/\s+/', trim($raw)) ?: []);
+
+ $ids = [];
+
+ foreach ($parts as $part) {
+ if (false === is_string($part)) {
+ continue;
+ }
+
+ $id = self::normalise($part);
+
+ if ('' !== $id) {
+ $ids[] = $id;
+ }
+ }
+
+ return array_values(array_unique($ids));
+ }
}
diff --git a/src/Entity/Message.php b/src/Entity/Message.php
index d113cda7..c0f635fd 100644
--- a/src/Entity/Message.php
+++ b/src/Entity/Message.php
@@ -24,6 +24,12 @@ use Doctrine\ORM\Mapping as ORM;
// for one mailbox), and no query looks up a bare UID.
#[ORM\UniqueConstraint(name: 'uniq_message_mailbox_imap_uid', columns: ['mailbox_id', 'imap_uid'])]
#[ORM\Index(name: 'idx_message_search_vector', columns: ['search_vector'])]
+// Threading reads both of these on every synced message. message_id is looked up
+// by value across an account (parent lookup for References threading); the
+// provider key is looked up account-scoped, so account trails it the same way
+// the provider-id constraints above are ordered.
+#[ORM\Index(name: 'idx_message_message_id', columns: ['message_id'])]
+#[ORM\Index(name: 'idx_message_provider_thread_key_account', columns: ['provider_thread_key', 'account_id'])]
#[ORM\Entity(repositoryClass: MessageRepository::class)]
class Message extends MessageModel
{
@@ -55,6 +61,13 @@ class Message extends MessageModel
#[ORM\Column(length: 512, nullable: true)]
private ?string $graphId = null;
+ /**
+ * Gmail threadId / Graph conversationId, carried from the sync payload so the
+ * threader can group without a second API call. Null for IMAP.
+ */
+ #[ORM\Column(length: 255, nullable: true)]
+ private ?string $providerThreadKey = null;
+
/**
* @var list|null
*/
@@ -247,6 +260,18 @@ class Message extends MessageModel
return $this;
}
+ public function getProviderThreadKey(): ?string
+ {
+ return $this->providerThreadKey;
+ }
+
+ public function setProviderThreadKey(?string $providerThreadKey): static
+ {
+ $this->providerThreadKey = $providerThreadKey;
+
+ return $this;
+ }
+
/**
* @return list|null
*/
diff --git a/src/Entity/MessageThread.php b/src/Entity/MessageThread.php
index 6a7747c1..d42f3e07 100644
--- a/src/Entity/MessageThread.php
+++ b/src/Entity/MessageThread.php
@@ -10,6 +10,13 @@ use Doctrine\Common\Collections\Collection;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Mapping as ORM;
+// One thread per provider conversation per account: Gmail's threadId and Graph's
+// conversationId already express the grouping those backends show their own users,
+// so the key is what we match on first and the constraint is what stops a
+// concurrent batch from creating a second thread for the same conversation.
+#[ORM\UniqueConstraint(name: 'uniq_message_thread_provider_key_account', columns: ['provider_thread_key', 'account_id'])]
+// Serves the subject fallback lookup, which is always account-scoped.
+#[ORM\Index(name: 'idx_message_thread_account_normalized_subject', columns: ['account_id', 'normalized_subject'])]
#[ORM\Entity(repositoryClass: MessageThreadRepository::class)]
class MessageThread
{
@@ -64,9 +71,17 @@ class MessageThread
#[ORM\Column(enumType: ThreadingMethod::class)]
private ?ThreadingMethod $threadingMethod = null;
- #[ORM\Column(length: 255)]
+ // TEXT, not VARCHAR: this is derived from `subject`, which is itself TEXT, so
+ // any length cap here is a length cap that can reject a legitimate message.
+ #[ORM\Column(type: Types::TEXT)]
private ?string $normalizedSubject = null;
+ /**
+ * Gmail threadId / Graph conversationId. Null for IMAP, which has no such concept.
+ */
+ #[ORM\Column(length: 255, nullable: true)]
+ private ?string $providerThreadKey = null;
+
#[ORM\Column(nullable: true)]
private ?\DateTimeImmutable $snoozedUntil = null;
@@ -231,9 +246,16 @@ class MessageThread
return $this;
}
- public function isArchived(): bool
+ public function getProviderThreadKey(): ?string
{
- return $this->archivedAt !== null;
+ return $this->providerThreadKey;
+ }
+
+ public function setProviderThreadKey(?string $providerThreadKey): static
+ {
+ $this->providerThreadKey = $providerThreadKey;
+
+ return $this;
}
public function getSnoozedUntil(): ?\DateTimeImmutable
diff --git a/src/Repository/MessageRepository.php b/src/Repository/MessageRepository.php
index c46908e1..6fb8edcd 100644
--- a/src/Repository/MessageRepository.php
+++ b/src/Repository/MessageRepository.php
@@ -64,12 +64,36 @@ class MessageRepository extends ServiceEntityRepository
public function existsWithFromAddressInThread(string $fromAddress, MessageThread $thread): bool
{
+ return $this->existsWithAnyFromAddressInThread([$fromAddress], $thread);
+ }
+
+ /**
+ * Does the thread contain a message sent by any of these addresses?
+ *
+ * Threading passes the candidate's sender *and* its recipients: a reply from
+ * someone new to the conversation shares no sender with it, but its To/Cc
+ * almost always names someone who has already posted. Checking senders only
+ * would reject those replies.
+ *
+ * @param list $addresses
+ */
+ public function existsWithAnyFromAddressInThread(array $addresses, MessageThread $thread): bool
+ {
+ $normalized = array_values(array_unique(array_filter(
+ array_map(static fn(string $address): string => mb_strtolower(trim($address)), $addresses),
+ static fn(string $address): bool => '' !== $address,
+ )));
+
+ if (0 === count($normalized)) {
+ return false;
+ }
+
$result = $this->createQueryBuilder('m')
->select('1')
->where('m.thread = :thread')
- ->andWhere('LOWER(m.fromAddress) = :fromAddress')
+ ->andWhere('LOWER(m.fromAddress) IN (:addresses)')
->setParameter('thread', $thread)
- ->setParameter('fromAddress', mb_strtolower($fromAddress))
+ ->setParameter('addresses', $normalized)
->setMaxResults(1)
->getQuery()
->getOneOrNullResult();
diff --git a/src/Repository/MessageThreadRepository.php b/src/Repository/MessageThreadRepository.php
index 74b1f9aa..2069d585 100644
--- a/src/Repository/MessageThreadRepository.php
+++ b/src/Repository/MessageThreadRepository.php
@@ -20,13 +20,36 @@ class MessageThreadRepository extends ServiceEntityRepository
parent::__construct($registry, MessageThread::class);
}
- public function findMatchingNormalizedSubjectThreadForAccount(string $normalizedSubject, Account $account): ?MessageThread
+ public function findOneByProviderThreadKeyForAccount(string $providerThreadKey, Account $account): ?MessageThread
+ {
+ return $this->createQueryBuilder('thread')
+ ->where('thread.account = :account')
+ ->andWhere('thread.providerThreadKey = :providerThreadKey')
+ ->setParameter('account', $account)
+ ->setParameter('providerThreadKey', $providerThreadKey)
+ ->setMaxResults(1)
+ ->getQuery()
+ ->getOneOrNullResult();
+ }
+
+ /**
+ * Newest thread with this subject that is still recent enough to be a
+ * plausible parent. The $since bound is what stops a recurring subject
+ * ("Your order has shipped") from accreting into one endless thread.
+ */
+ public function findMatchingNormalizedSubjectThreadForAccount(
+ string $normalizedSubject,
+ Account $account,
+ \DateTimeImmutable $since,
+ ): ?MessageThread
{
return $this->createQueryBuilder('thread')
->where('thread.account = :account')
->andWhere('thread.normalizedSubject = :normalizedSubject')
+ ->andWhere('thread.lastMessageAt >= :since')
->setParameter('account', $account)
->setParameter('normalizedSubject', $normalizedSubject)
+ ->setParameter('since', $since)
->orderBy('thread.lastMessageAt', 'DESC')
->setMaxResults(1)
->getQuery()
diff --git a/src/Service/Gmail/GmailMessageBuilder.php b/src/Service/Gmail/GmailMessageBuilder.php
index 3d9aaef3..5be186af 100644
--- a/src/Service/Gmail/GmailMessageBuilder.php
+++ b/src/Service/Gmail/GmailMessageBuilder.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Service\Gmail;
+use App\Domain\Helper\MessageIdHelper;
use App\Entity\Account;
use App\Entity\Label;
use App\Entity\Message;
@@ -61,12 +62,17 @@ final class GmailMessageBuilder
$message->setGmailId($gmailId);
$message->setGmailLabelIds($labelIds);
+ // Gmail already grouped this conversation for the user; carrying its
+ // threadId over means our threads match what they see in Gmail itself.
+ $threadId = trim((string) ($payload['threadId'] ?? ''));
+ $message->setProviderThreadKey('' !== $threadId ? $threadId : null);
+
$this->applyTranslatedLabels($message, $labelIds, $account, $carrierAccount ?? $account);
// ── Headers ───────────────────────────────────────────────────────────
$headers = $this->indexHeaders($payload['payload']['headers'] ?? []);
- $rfcMessageId = $headers['message-id'] ?? '';
+ $rfcMessageId = MessageIdHelper::normalise($headers['message-id'] ?? '');
$message->setMessageId('' !== $rfcMessageId ? $rfcMessageId : $gmailId);
$message->setSubject($this->decodeMimeHeader($headers['subject'] ?? ''));
@@ -78,15 +84,8 @@ final class GmailMessageBuilder
$message->setCcAddresses($this->parseAddressList($headers['cc'] ?? ''));
$message->setBccAddresses($this->parseAddressList($headers['bcc'] ?? ''));
- $inReplyToRaw = trim($headers['in-reply-to'] ?? '');
- $referencesRaw = trim($headers['references'] ?? '');
-
- $message->setInReplyTo(
- '' !== $inReplyToRaw ? preg_split('/\s+/', $inReplyToRaw) : []
- );
- $message->setReferences(
- '' !== $referencesRaw ? preg_split('/\s+/', $referencesRaw) : []
- );
+ $message->setInReplyTo(MessageIdHelper::normaliseList($headers['in-reply-to'] ?? null));
+ $message->setReferences(MessageIdHelper::normaliseList($headers['references'] ?? null));
// ── Date ──────────────────────────────────────────────────────────────
$internalDateMs = (int)($payload['internalDate'] ?? 0);
diff --git a/src/Service/Graph/GraphMessageBuilder.php b/src/Service/Graph/GraphMessageBuilder.php
index da4181cd..a0eb4843 100644
--- a/src/Service/Graph/GraphMessageBuilder.php
+++ b/src/Service/Graph/GraphMessageBuilder.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Service\Graph;
+use App\Domain\Helper\MessageIdHelper;
use App\Entity\Account;
use App\Entity\Message;
use App\Entity\MessagePart;
@@ -39,8 +40,9 @@ use Doctrine\ORM\EntityManagerInterface;
*
* Note on internetMessageHeaders: Graph returns it only for messages that were
* received (not for drafts), and truncates it on very large header sets. It is
- * used for In-Reply-To/References, which fall back to conversationId-derived
- * threading when absent.
+ * used for In-Reply-To/References. When absent, threading falls back to
+ * conversationId, which is stored as the provider thread key below and is what
+ * the threader consults first anyway.
*/
final class GraphMessageBuilder
{
@@ -62,11 +64,17 @@ final class GraphMessageBuilder
$graphId = (string) ($payload['id'] ?? '');
$message->setGraphId($graphId);
+ // Graph's own conversation grouping, already selected by MESSAGE_SELECT.
+ // This is the only threading signal available for drafts and for messages
+ // whose internetMessageHeaders came back truncated.
+ $conversationId = trim((string) ($payload['conversationId'] ?? ''));
+ $message->setProviderThreadKey('' !== $conversationId ? $conversationId : null);
+
// ── Identity ──────────────────────────────────────────────────────────
// The RFC Message-ID is the dedup key, not the Graph id: Graph ids are
// locators and can rotate when a message moves. Fall back to the Graph
// id only when the mailbox genuinely has no Message-ID (drafts).
- $rfcMessageId = trim((string) ($payload['internetMessageId'] ?? ''));
+ $rfcMessageId = MessageIdHelper::normalise((string) ($payload['internetMessageId'] ?? ''));
$message->setMessageId('' !== $rfcMessageId ? $rfcMessageId : $graphId);
$message->setSubject((string) ($payload['subject'] ?? ''));
@@ -104,15 +112,8 @@ final class GraphMessageBuilder
$message->setHeaders($rawHeaders);
- $inReplyToRaw = trim($indexed['in-reply-to'] ?? '');
- $referencesRaw = trim($indexed['references'] ?? '');
-
- $message->setInReplyTo(
- '' !== $inReplyToRaw ? preg_split('/\s+/', $inReplyToRaw) : []
- );
- $message->setReferences(
- '' !== $referencesRaw ? preg_split('/\s+/', $referencesRaw) : []
- );
+ $message->setInReplyTo(MessageIdHelper::normaliseList($indexed['in-reply-to'] ?? null));
+ $message->setReferences(MessageIdHelper::normaliseList($indexed['references'] ?? null));
// ── Dates ─────────────────────────────────────────────────────────────
$receivedAt = $this->parseDate($payload['receivedDateTime'] ?? null) ?? new DateTimeImmutable();
diff --git a/src/Service/Imap/MessageSyncer.php b/src/Service/Imap/MessageSyncer.php
index d2dbf928..34b266be 100644
--- a/src/Service/Imap/MessageSyncer.php
+++ b/src/Service/Imap/MessageSyncer.php
@@ -271,7 +271,7 @@ class MessageSyncer
$message->addLabel($mailboxLabel);
}
$message->setImapUid($imapMessage->getUid());
- $message->setMessageId((string) $imapMessage->getMessageId());
+ $message->setMessageId(MessageIdHelper::normalise((string) $imapMessage->getMessageId()));
$message->setSubject(
$this->decodeMimeHeader((string) $imapMessage->getSubject())
);
@@ -312,10 +312,10 @@ class MessageSyncer
$references = $imapMessage->getReferences();
$message->setInReplyTo(
- $inReplyTo->exist() ? explode(' ', trim((string) $inReplyTo)) : []
+ $inReplyTo->exist() ? MessageIdHelper::normaliseList((string) $inReplyTo) : []
);
$message->setReferences(
- $references->exist() ? explode(' ', trim((string) $references)) : []
+ $references->exist() ? MessageIdHelper::normaliseList((string) $references) : []
);
// Headers
diff --git a/src/Service/Imap/MessageThreader.php b/src/Service/Imap/MessageThreader.php
index 89d3a1e2..a0310d4a 100644
--- a/src/Service/Imap/MessageThreader.php
+++ b/src/Service/Imap/MessageThreader.php
@@ -4,6 +4,7 @@ namespace App\Service\Imap;
use App\Domain\Enum\MessageCategory;
use App\Domain\Enum\ThreadingMethod;
+use App\Domain\Helper\MessageIdHelper;
use App\Entity\Account;
use App\Entity\Message;
use App\Entity\MessageThread;
@@ -13,6 +14,20 @@ use Doctrine\ORM\EntityManagerInterface;
final class MessageThreader
{
+ /**
+ * Reply/forward prefixes, in the languages this app is translated into plus
+ * the ones common European clients emit. Used both to strip the prefix for
+ * subject normalisation and to decide whether a message is a reply at all.
+ */
+ private const string REPLY_PREFIX_PATTERN = '/^\s*((re|fwd|fw|aw|wg|antw|sv|vs|res|rif|tr|doorst)\s*(\[\d+\])?\s*:\s*)+/i';
+
+ /**
+ * How far back the subject fallback may reach for a parent thread. Long
+ * enough to cover a slow-moving conversation, short enough that a recurring
+ * subject starts a fresh thread instead of extending a stale one.
+ */
+ private const string SUBJECT_FALLBACK_WINDOW = '-30 days';
+
public function __construct(
private readonly EntityManagerInterface $entityManager,
private readonly MessageRepository $messageRepository,
@@ -30,6 +45,22 @@ final class MessageThreader
*/
public function assignThread(Message $message, Account $account): void
{
+ // 1. The provider's own conversation id, when there is one. Gmail and
+ // Graph have already grouped the conversation the way the user sees
+ // it in their web client; nothing we derive can beat that.
+ $providerThreadKey = $message->getProviderThreadKey();
+
+ if (null !== $providerThreadKey && '' !== $providerThreadKey) {
+ $thread = $this->messageThreadRepository->findOneByProviderThreadKeyForAccount($providerThreadKey, $account)
+ ?? $this->createThread($message, $account, ThreadingMethod::Provider)
+ ->setProviderThreadKey($providerThreadKey);
+
+ $this->attachMessageToThread($message, $thread);
+
+ return;
+ }
+
+ // 2. RFC 5322 References / In-Reply-To.
$referenceIds = $this->collectReferenceIds($message);
if (count($referenceIds) > 0) {
@@ -50,19 +81,27 @@ final class MessageThreader
return;
}
- // No usable references at all — fall back to subject + participant matching.
- $normalizedSubject = $this->normalizeSubject($message->getSubject());
+ // 3. No usable references at all. Subject matching is the last resort and
+ // is deliberately narrow: it exists to rescue replies from clients that
+ // omit References, not to group unrelated mail that happens to share a
+ // subject line. A message that is not itself a reply always starts its
+ // own thread — otherwise every "Your order has shipped" notification
+ // ever received collapses into one endless conversation.
+ if (true === $this->hasReplyPrefix($message->getSubject())) {
+ $normalizedSubject = $this->normalizeSubject($message->getSubject());
- if ($normalizedSubject !== '') {
- $candidateThread = $this->messageThreadRepository->findMatchingNormalizedSubjectThreadForAccount(
- $normalizedSubject,
- $account,
- );
+ if ($normalizedSubject !== '') {
+ $candidateThread = $this->messageThreadRepository->findMatchingNormalizedSubjectThreadForAccount(
+ $normalizedSubject,
+ $account,
+ new \DateTimeImmutable(self::SUBJECT_FALLBACK_WINDOW),
+ );
- if ($candidateThread !== null && $this->participantsOverlap($message, $candidateThread)) {
- $this->attachMessageToThread($message, $candidateThread);
+ if ($candidateThread !== null && $this->participantsOverlap($message, $candidateThread)) {
+ $this->attachMessageToThread($message, $candidateThread);
- return;
+ return;
+ }
}
}
@@ -75,29 +114,28 @@ final class MessageThreader
*/
private function collectReferenceIds(Message $message): array
{
- $ids = [];
+ // Normalised again on read, not just on write: rows synced before the
+ // write paths were fixed still hold bracketed ids, and those have to keep
+ // matching until the rethread backfill has run.
+ return MessageIdHelper::normaliseList(array_merge(
+ MessageIdHelper::normaliseList($message->getInReplyTo()),
+ MessageIdHelper::normaliseList($message->getReferences()),
+ ));
+ }
- $inReplyTo = $message->getInReplyTo();
-
- if (is_array($inReplyTo)) {
- foreach ($inReplyTo as $id) {
- if (is_string($id) && $id !== '') {
- $ids[] = $id;
- }
- }
+ /**
+ * Is this message a reply or forward, as far as its subject line admits?
+ *
+ * This is the gate on subject-based threading. Automated notifications never
+ * carry a reply prefix, so they can never be merged into an existing thread.
+ */
+ public function hasReplyPrefix(?string $subject): bool
+ {
+ if (null === $subject) {
+ return false;
}
- $references = $message->getReferences();
-
- if (is_array($references)) {
- foreach ($references as $id) {
- if (is_string($id) && $id !== '') {
- $ids[] = $id;
- }
- }
- }
-
- return array_values(array_unique($ids));
+ return 1 === preg_match(self::REPLY_PREFIX_PATTERN, $subject);
}
public function normalizeSubject(?string $subject): string
@@ -108,18 +146,15 @@ final class MessageThreader
$normalized = trim($subject);
- // Strip repeated Re:/Fwd:/Fw: prefixes, including localized/spaced variants
- // like "RE :" or "Fwd:Fwd:".
- $pattern = '/^\s*((re|fwd|fw)\s*:\s*)+/i';
- $normalized = preg_replace($pattern, '', $normalized);
+ // Strip repeated Re:/Fwd:/AW:/… prefixes, including spaced and counted
+ // variants like "RE :", "Fwd:Fwd:" and "Re[2]:".
+ $normalized = preg_replace(self::REPLY_PREFIX_PATTERN, '', $normalized);
if ($normalized === null) {
$normalized = trim($subject);
}
- $normalized = mb_strtolower(trim($normalized));
-
- return $normalized;
+ return mb_strtolower(trim($normalized));
}
/**
* Keep a single-message draft thread's subject in step with the draft.
@@ -143,15 +178,39 @@ final class MessageThreader
->setNormalizedSubject($this->normalizeSubject($message->getSubject()));
}
+ /**
+ * Does this message share a participant with the thread?
+ *
+ * Sender *and* recipients are considered: a reply from someone who has not
+ * posted before shares no sender with the thread, but addresses someone who
+ * has. Comparing senders alone would reject exactly the replies the subject
+ * fallback exists to catch.
+ */
private function participantsOverlap(Message $message, MessageThread $thread): bool
{
+ $addresses = [];
+
$fromAddress = $message->getFromAddress();
- if ($fromAddress === null || $fromAddress === '') {
+ if (null !== $fromAddress && '' !== $fromAddress) {
+ $addresses[] = $fromAddress;
+ }
+
+ foreach ([$message->getToAddresses(), $message->getCcAddresses()] as $recipients) {
+ foreach ($recipients ?? [] as $recipient) {
+ $address = is_array($recipient) ? ($recipient['address'] ?? null) : $recipient;
+
+ if (is_string($address) && '' !== $address) {
+ $addresses[] = $address;
+ }
+ }
+ }
+
+ if (0 === count($addresses)) {
return false;
}
- return $this->messageRepository->existsWithFromAddressInThread($fromAddress, $thread);
+ return $this->messageRepository->existsWithAnyFromAddressInThread($addresses, $thread);
}
private function createThread(Message $message, Account $account, ThreadingMethod $threadingMethod): MessageThread
diff --git a/tests/Domain/Helper/MessageIdHelperTest.php b/tests/Domain/Helper/MessageIdHelperTest.php
new file mode 100644
index 00000000..fde74d00
--- /dev/null
+++ b/tests/Domain/Helper/MessageIdHelperTest.php
@@ -0,0 +1,72 @@
+
+ */
+ public static function normaliseCases(): iterable
+ {
+ yield 'bracketed' => ['', 'abc@host'];
+ yield 'bare' => ['abc@host', 'abc@host'];
+ yield 'padded' => [" \r\n", 'abc@host'];
+ yield 'empty' => ['', ''];
+ yield 'brackets only' => ['<>', ''];
+ }
+
+ /**
+ * The bug this helper exists for: IMAP stored ids unbracketed while the API
+ * backends stored them bracketed, so the two never compared equal.
+ */
+ public function testBracketedAndBareFormsNormaliseToTheSameId(): void
+ {
+ self::assertSame(
+ MessageIdHelper::normalise(''),
+ MessageIdHelper::normalise('CAF=abc@mail.gmail.com'),
+ );
+ }
+
+ public function testNormaliseListSplitsOnAnyWhitespace(): void
+ {
+ self::assertSame(
+ ['a@host', 'b@host', 'c@host'],
+ MessageIdHelper::normaliseList(" \r\n\t"),
+ );
+ }
+
+ public function testNormaliseListAcceptsAnAlreadySplitList(): void
+ {
+ self::assertSame(
+ ['a@host', 'b@host'],
+ MessageIdHelper::normaliseList(['', 'b@host']),
+ );
+ }
+
+ public function testNormaliseListDropsEmptiesAndDuplicates(): void
+ {
+ self::assertSame(
+ ['a@host'],
+ MessageIdHelper::normaliseList(['', '', ' ', 'a@host', '<>']),
+ );
+ }
+
+ public function testNormaliseListHandlesNullAndBlank(): void
+ {
+ self::assertSame([], MessageIdHelper::normaliseList(null));
+ self::assertSame([], MessageIdHelper::normaliseList(' '));
+ }
+}
diff --git a/tests/Service/Imap/MessageThreaderTest.php b/tests/Service/Imap/MessageThreaderTest.php
new file mode 100644
index 00000000..ce4f90a7
--- /dev/null
+++ b/tests/Service/Imap/MessageThreaderTest.php
@@ -0,0 +1,93 @@
+threader = new MessageThreader(
+ $this->createStub(EntityManagerInterface::class),
+ $this->createStub(MessageRepository::class),
+ $this->createStub(MessageThreadRepository::class),
+ );
+ }
+
+ #[DataProvider('normalizeSubjectCases')]
+ public function testNormalizeSubject(?string $subject, string $expected): void
+ {
+ self::assertSame($expected, $this->threader->normalizeSubject($subject));
+ }
+
+ /**
+ * @return iterable
+ */
+ public static function normalizeSubjectCases(): iterable
+ {
+ yield 'null' => [null, ''];
+ yield 'plain' => ['Project update', 'project update'];
+ yield 're' => ['Re: Project update', 'project update'];
+ yield 'fwd' => ['Fwd: Project update', 'project update'];
+ yield 'repeated' => ['Re: Re: Fwd: Project update', 'project update'];
+ yield 'no space' => ['Fwd:Fwd:Project update', 'project update'];
+ yield 'spaced colon' => ['RE : Project update', 'project update'];
+ yield 'counted' => ['Re[2]: Project update', 'project update'];
+ yield 'german reply' => ['AW: Projektstatus', 'projektstatus'];
+ yield 'german forward' => ['WG: Projektstatus', 'projektstatus'];
+ yield 'mixed locale' => ['AW: Re: Projektstatus', 'projektstatus'];
+ yield 'surrounding ws' => [' Re: Project update ', 'project update'];
+ yield 'prefix in body' => ['Rewriting the parser', 'rewriting the parser'];
+ }
+
+ /**
+ * The subject column is TEXT, so normalisation must not cap length —
+ * truncating here would silently merge two distinct long subjects.
+ */
+ public function testNormalizeSubjectDoesNotTruncateLongSubjects(): void
+ {
+ $subject = 'Re: ' . str_repeat('a', 4000);
+
+ self::assertSame(str_repeat('a', 4000), $this->threader->normalizeSubject($subject));
+ }
+
+ #[DataProvider('replyPrefixCases')]
+ public function testHasReplyPrefix(?string $subject, bool $expected): void
+ {
+ self::assertSame($expected, $this->threader->hasReplyPrefix($subject));
+ }
+
+ /**
+ * @return iterable
+ */
+ public static function replyPrefixCases(): iterable
+ {
+ yield 'reply' => ['Re: Project update', true];
+ yield 'lowercase reply' => ['re: project update', true];
+ yield 'forward' => ['Fwd: Project update', true];
+ yield 'german reply' => ['AW: Projektstatus', true];
+ yield 'german forward' => ['WG: Projektstatus', true];
+ yield 'counted reply' => ['Re[2]: Project update', true];
+ yield 'null' => [null, false];
+ yield 'empty' => ['', false];
+ yield 'plain' => ['Project update', false];
+ // The regression this gate exists for: notification subjects repeat
+ // verbatim for years and must never merge into one another.
+ yield 'amazon order' => ['Ihre Amazon.de Bestellung', false];
+ yield 'amazon shipped' => ['Ihre Bestellung wurde versandt', false];
+ yield 'prefix-like word' => ['Rewriting the parser', false];
+ yield 'colon elsewhere' => ['Reminder: standup at 9', false];
+ }
+}
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
new file mode 100644
index 00000000..27f9b209
--- /dev/null
+++ b/tests/bootstrap.php
@@ -0,0 +1,11 @@
+bootEnv(dirname(__DIR__) . '/.env');
+}
diff --git a/tests/e2e/account.spec.ts b/tests/e2e/account.spec.ts
index d21328f0..70c7d88f 100644
--- a/tests/e2e/account.spec.ts
+++ b/tests/e2e/account.spec.ts
@@ -45,7 +45,10 @@ test.describe("mail account creation", () => {
await expect(page.getByText("Account added successfully")).toBeVisible();
const list = page.locator("#settings-account-list");
- await expect(list).toContainText(label);
+ // The list renders the account's primary alias, and addresses are
+ // normalised to lowercase on save — so match case-insensitively rather
+ // than asserting the casing that happened to be typed.
+ await expect(list).toContainText(new RegExp(label, "i"));
await expect(list).toContainText(host);
});
diff --git a/tests/e2e/compose.spec.ts b/tests/e2e/compose.spec.ts
index a574a439..fe704605 100644
--- a/tests/e2e/compose.spec.ts
+++ b/tests/e2e/compose.spec.ts
@@ -1,6 +1,5 @@
import { test, expect } from "@playwright/test";
-import { execSync } from "node:child_process";
-import { readFileSync } from "node:fs";
+import { seed } from "./support/config";
/**
* Runs authenticated via the shared storage state from auth.setup.ts.
@@ -9,17 +8,14 @@ import { readFileSync } from "node:fs";
* initialised from the