mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 10:04:18 +00:00
Store a UIDVALIDITY above 2147483647 (#174)
mailbox.uid_validity becomes a BIGINT, so IMAP folders whose UIDVALIDITY is above the signed INT range sync again. Reversible migration; the mapping stays ?int (DBAL 4 hydrates a bigint as an int). Closes #171 Co-Authored-By: Philippe Delodder <philippe@delodder.be> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
5 files changed
+282
-1
No files matched your search
@@ -8,6 +8,16 @@ The published image tags: `latest` follows the most recent release below,
|
||||
|
||||
## Unreleased
|
||||
|
||||
**IMAP folders that never synced now do.** On some servers, Dovecot among them, a folder's
|
||||
UIDVALIDITY (the number that says whether its message numbers still mean what they did) is above
|
||||
2147483647, and plMail could not store a number that large: every sync of such a folder stopped
|
||||
with "value … is out of range for type integer" in the worker log, and its mail never arrived. Other
|
||||
folders of the same account were not affected. **Migration:** `mailbox.uid_validity` is widened
|
||||
from `INT` to `BIGINT`, which rewrites the small `mailbox` table once on boot; no stored value
|
||||
changes. Reversible: rolling back forgets a folder's UIDVALIDITY that only the wider column can
|
||||
hold, and the next sync records it again.
|
||||
[#174](https://github.com/karatektus/pl_mail/pull/174), by @phdelodder.
|
||||
|
||||
<!-- highlight screenshot=whats-new -->
|
||||
**Release notes after an update, and a feedback form.** The first time you open plMail
|
||||
after it has been updated, a dialog lists what changed since the version you last saw: the
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace DoctrineMigrations;
|
||||
|
||||
use Doctrine\DBAL\Schema\Schema;
|
||||
use Doctrine\Migrations\AbstractMigration;
|
||||
|
||||
/**
|
||||
* mailbox.uid_validity becomes a BIGINT (#171).
|
||||
*
|
||||
* RFC 3501 defines UIDVALIDITY as an unsigned 32-bit number, up to 4294967295,
|
||||
* and the column was a signed INT, which stops at 2147483647. Servers that
|
||||
* derive the value from a timestamp or a hash go past that routinely — Dovecot
|
||||
* does — and for such a folder the flush that records it was refused with
|
||||
* "value … is out of range for type integer", so the folder never synced.
|
||||
*
|
||||
* Widening keeps every stored value as it is. Postgres rewrites the table to do
|
||||
* it, which is harmless here: mailbox holds one row per folder, not per message.
|
||||
*
|
||||
* down() narrows it again and forgets any value an INT cannot hold rather than
|
||||
* failing on it. A null UIDVALIDITY is the state of a folder never listed, and
|
||||
* the next sweep adopts what the server says instead of treating it as a
|
||||
* rebuild (VanishedMessageReconciler::isRebuilt()) — which is the most an older
|
||||
* release could have done with such a folder anyway.
|
||||
*/
|
||||
final class Version20261016120000 extends AbstractMigration
|
||||
{
|
||||
public function getDescription(): string
|
||||
{
|
||||
return 'Widen mailbox.uid_validity to BIGINT, so a UIDVALIDITY above 2147483647 can be stored';
|
||||
}
|
||||
|
||||
public function up(Schema $schema): void
|
||||
{
|
||||
$this->addSql('ALTER TABLE mailbox ALTER COLUMN uid_validity TYPE BIGINT');
|
||||
}
|
||||
|
||||
public function down(Schema $schema): void
|
||||
{
|
||||
$this->addSql(<<<'SQL'
|
||||
ALTER TABLE mailbox ALTER COLUMN uid_validity TYPE INT
|
||||
USING CASE WHEN uid_validity > 2147483647 THEN NULL ELSE uid_validity END
|
||||
SQL);
|
||||
}
|
||||
}
|
||||
@@ -9,6 +9,7 @@ use App\Repository\Mail\MailboxRepository;
|
||||
use Doctrine\Common\Collections\ArrayCollection;
|
||||
use Doctrine\Common\Collections\Collection;
|
||||
use App\Domain\Trait\TimestampableTrait;
|
||||
use Doctrine\DBAL\Types\Types;
|
||||
use Doctrine\ORM\Mapping as ORM;
|
||||
|
||||
#[ORM\Entity(repositoryClass: MailboxRepository::class)]
|
||||
@@ -50,7 +51,21 @@ class Mailbox
|
||||
#[ORM\Column(length: 50, nullable: true)]
|
||||
public ?MailboxSpecialUse $specialUse = null;
|
||||
|
||||
#[ORM\Column(nullable: true)]
|
||||
/**
|
||||
* The folder's UIDVALIDITY as last seen, or null before the first listing.
|
||||
*
|
||||
* BIGINT, not INT: RFC 3501 makes it an unsigned 32-bit number, up to
|
||||
* 4294967295, and servers that derive it from a timestamp or a hash
|
||||
* (Dovecot among them) go past the 2147483647 a signed INT holds. Stored as
|
||||
* INT, the flush that records such a value was refused and the folder never
|
||||
* synced at all (#171).
|
||||
*
|
||||
* Still an int in PHP, and that is not an oversight: since DBAL 4 a bigint
|
||||
* that fits PHP's integer range is hydrated as an int, and on a 64-bit build
|
||||
* every bigint does. VanishedMessageReconciler::isRebuilt() compares it
|
||||
* with `!==`, so a string here would read every sweep as a rebuild.
|
||||
*/
|
||||
#[ORM\Column(type: Types::BIGINT, nullable: true)]
|
||||
public ?int $uidValidity = null;
|
||||
|
||||
#[ORM\Column(nullable: true)]
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Migrations;
|
||||
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\Mailbox;
|
||||
use App\Entity\User\User;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\DBAL\Schema\Schema;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use DoctrineMigrations\Version20261016120000;
|
||||
use Psr\Log\NullLogger;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
|
||||
/**
|
||||
* That mailbox.uid_validity is a BIGINT, and that the widening can be taken
|
||||
* back without failing on a value only the wider column could hold (#171).
|
||||
*
|
||||
* The forward half is read from the live catalogue rather than from the
|
||||
* migration's SQL, for the reason ReadReceiptColumnsMigrationTest gives: the
|
||||
* mapping validates and every in-memory test passes whether or not the
|
||||
* migration ran, and a deployed instance is the first to find out.
|
||||
*
|
||||
* The reverse half is run against rows. A plain `TYPE INT` is the obvious
|
||||
* down() and it refuses the whole statement as soon as one folder holds a
|
||||
* UIDVALIDITY above 2147483647 — which is exactly the folder this change is for.
|
||||
*/
|
||||
final class MailboxUidValidityMigrationTest extends KernelTestCase
|
||||
{
|
||||
private const int ABOVE_INT = 3064473417;
|
||||
private const int WITHIN_INT = 1700000000;
|
||||
|
||||
private EntityManagerInterface $em;
|
||||
private Connection $connection;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$this->em = self::getContainer()->get(EntityManagerInterface::class);
|
||||
$this->connection = self::getContainer()->get(Connection::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
public function testTheColumnIsABigint(): void
|
||||
{
|
||||
self::assertSame('bigint', $this->columnType(), 'mailbox.uid_validity must hold an unsigned 32-bit UIDVALIDITY');
|
||||
}
|
||||
|
||||
/**
|
||||
* Rolled back and re-applied inside this test's own transaction, so the
|
||||
* database it started with is the database it leaves.
|
||||
*/
|
||||
public function testTheMigrationCanBeRolledBackOverAValueAnIntCannotHold(): void
|
||||
{
|
||||
[$large, $small] = $this->seedMailboxes();
|
||||
|
||||
$this->migrate('down');
|
||||
|
||||
self::assertSame('integer', $this->columnType());
|
||||
self::assertNull($this->uidValidityOf($large), 'a value an INT cannot hold is forgotten rather than failing the rollback');
|
||||
self::assertSame(self::WITHIN_INT, $this->uidValidityOf($small), 'and one it can hold is kept');
|
||||
|
||||
$this->migrate('up');
|
||||
|
||||
self::assertSame('bigint', $this->columnType());
|
||||
self::assertSame(self::WITHIN_INT, $this->uidValidityOf($small));
|
||||
}
|
||||
|
||||
// ── helpers ───────────────────────────────────────────────────────────
|
||||
|
||||
private function migrate(string $direction): void
|
||||
{
|
||||
// The migrations directory is read by Doctrine's own loader, not by
|
||||
// Composer, so nothing has autoloaded the class by the time a test runs.
|
||||
require_once dirname(__DIR__, 2) . '/migrations/Version20261016120000.php';
|
||||
|
||||
$migration = new Version20261016120000($this->connection, new NullLogger());
|
||||
$migration->{$direction}(new Schema());
|
||||
|
||||
foreach ($migration->getSql() as $query) {
|
||||
$this->connection->prepare($query->getStatement())->executeStatement();
|
||||
}
|
||||
|
||||
$this->em->clear();
|
||||
}
|
||||
|
||||
private function columnType(): string
|
||||
{
|
||||
return (string) $this->connection->fetchOne(
|
||||
'SELECT data_type FROM information_schema.columns WHERE table_name = :table AND column_name = :column',
|
||||
['table' => 'mailbox', 'column' => 'uid_validity'],
|
||||
);
|
||||
}
|
||||
|
||||
private function uidValidityOf(Mailbox $mailbox): ?int
|
||||
{
|
||||
$value = $this->connection->fetchOne('SELECT uid_validity FROM mailbox WHERE id = :id', ['id' => $mailbox->id]);
|
||||
|
||||
return null === $value ? null : (int) $value;
|
||||
}
|
||||
|
||||
/**
|
||||
* One folder whose UIDVALIDITY only a BIGINT holds, and one an INT holds.
|
||||
*
|
||||
* @return array{Mailbox, Mailbox}
|
||||
*/
|
||||
private function seedMailboxes(): array
|
||||
{
|
||||
$user = new User();
|
||||
$user->email = 'uidvalidity-' . uniqid('', true) . '@example.test';
|
||||
$user->nameFirst = 'Uid';
|
||||
$user->nameLast = 'Validity';
|
||||
$user->roles = ['ROLE_USER'];
|
||||
$user->password = 'x';
|
||||
$this->em->persist($user);
|
||||
|
||||
$account = new Account();
|
||||
$account->usr = $user;
|
||||
$account->email = 'uidvalidity-' . uniqid('', true) . '@example.test';
|
||||
$account->username = $account->email;
|
||||
$account->imapHost = 'localhost';
|
||||
$account->imapPort = 993;
|
||||
$account->imapEncryption = 'ssl';
|
||||
$account->smtpHost = 'localhost';
|
||||
$account->smtpPort = 587;
|
||||
$account->smtpEncryption = 'starttls';
|
||||
$account->password = 'x';
|
||||
$account->authType = 'password';
|
||||
$account->isActive = true;
|
||||
$this->em->persist($account);
|
||||
|
||||
$large = $this->mailbox($account, 'INBOX', self::ABOVE_INT);
|
||||
$small = $this->mailbox($account, 'Archive', self::WITHIN_INT);
|
||||
|
||||
$this->em->flush();
|
||||
|
||||
return [$large, $small];
|
||||
}
|
||||
|
||||
private function mailbox(Account $account, string $path, int $uidValidity): Mailbox
|
||||
{
|
||||
$mailbox = new Mailbox();
|
||||
$mailbox->account = $account;
|
||||
$mailbox->name = $path;
|
||||
$mailbox->fullPath = $path;
|
||||
$mailbox->uidValidity = $uidValidity;
|
||||
$mailbox->isSyncEnabled = true;
|
||||
$mailbox->isIdleEnabled = false;
|
||||
$this->em->persist($mailbox);
|
||||
|
||||
return $mailbox;
|
||||
}
|
||||
}
|
||||
@@ -652,6 +652,49 @@ final class RemoteDeletionSyncTest extends KernelTestCase
|
||||
self::assertSame(333, $this->inbox->uidValidity);
|
||||
}
|
||||
|
||||
/**
|
||||
* UIDVALIDITY is an unsigned 32-bit number, and a server that derives it
|
||||
* from a timestamp or a hash goes past what a signed INT holds. The column
|
||||
* was one (#171): the flush that recorded such a value was refused, the
|
||||
* folder's sync was aborted, and the same thing happened on every round.
|
||||
*
|
||||
* Re-read from the database rather than asserted on the object the sweep
|
||||
* wrote, so the assertion is about what was stored and how it comes back.
|
||||
*/
|
||||
public function testAUidValidityAboveTheSignedIntRangeIsStored(): void
|
||||
{
|
||||
$this->incoming('Eins', $this->inbox, 6);
|
||||
|
||||
$believed = $this->sweep($this->inbox, [6], now: '2026-08-11 09:00:00', uidValidity: 3064473417);
|
||||
|
||||
self::assertTrue($believed);
|
||||
|
||||
$this->em->refresh($this->inbox);
|
||||
|
||||
self::assertSame(3064473417, $this->inbox->uidValidity, 'stored whole, and read back as an int');
|
||||
}
|
||||
|
||||
/**
|
||||
* The other half of storing it: the same large value, read back from the
|
||||
* database on the next sweep, is the same validity. isRebuilt() compares
|
||||
* with `!==`, so a value that came back as a string — which is what a
|
||||
* bigint was hydrated as before DBAL 4 — would void every stored UID of the
|
||||
* folder on every sweep.
|
||||
*/
|
||||
public function testAnUnchangedUidValidityAboveTheSignedIntRangeIsNotARebuild(): void
|
||||
{
|
||||
$message = $this->incoming('Eins', $this->inbox, 6);
|
||||
|
||||
$this->inbox->uidValidity = 3064473417;
|
||||
$this->em->flush();
|
||||
$this->em->refresh($this->inbox);
|
||||
|
||||
$believed = $this->sweep($this->inbox, [6], now: '2026-08-11 09:00:00', uidValidity: 3064473417);
|
||||
|
||||
self::assertTrue($believed, 'an unchanged validity is a comparison, not a rebuild');
|
||||
self::assertSame(6, $message->imapUid, 'and the address it had still holds');
|
||||
}
|
||||
|
||||
// ── cadence ──────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
|
||||
Reference in new issue
Block a user