[Web][Rspamd] Keep the global archive switch in Redis only

data/conf/rspamd/dynmaps/archive.php

@@ -21,7 +21,7 @@ catch (PDOException $e) {
   http_response_code(501);
   exit;
 }
-// Init Redis, only to mirror the switch rspamd reads
+// Init Redis, it keeps the global switch; without it nothing is archived
 $redis = new Redis();
 try {
   $redis->connect('redis-mailcow', 6379);

data/conf/rspamd/lua/rspamd.local.lua

@@ -1072,13 +1072,12 @@ rspamd_config:register_symbol({
     end
 
     local function enabled_cb(err, data)
-      if not err and type(data) == 'string' and data == '0' then
+      if not err and type(data) == 'string' and data == '1' then
-        return
+        archive()
       end
-      archive()
     end
 
-    local ret = rspamd_redis_make_request(task,
+    rspamd_redis_make_request(task,
       redis_params, -- connect params
       'ARCHIVE_ENABLED', -- hash key
       false, -- is write
@@ -1086,8 +1085,5 @@ rspamd_config:register_symbol({
       'GET', -- command
       {'ARCHIVE_ENABLED'} -- arguments
     )
-    if not ret then
-      archive()
-    end
   end
 })

data/web/inc/lib/mailcow/Archive/ArchiveModule.php

@@ -51,7 +51,7 @@ class ArchiveModule
         $settings = new SettingsService($settingsRepo, $lookup, $state, $smtp, $context);
         $journal = new JournalService($settingsRepo, new OwnerResolver($lookup), new HeaderBuilder(), $smtp, $state, $context);
         $status = new StatusService($settingsRepo, $state, new LegacyQueue(), $context);
-        $mailboxes = new MailboxListService($lookup, $settingsRepo, $context);
+        $mailboxes = new MailboxListService($lookup, $state, $context);
         $api = new ArchiveApiController($settings, $status, $journal, $mailboxes, new ApiResponse(self::SECRET_KEYS));
         return new self($settings, $journal, $status, $mailboxes, $api);
     }

data/web/inc/lib/mailcow/Archive/Redis/ArchiveState.php

@@ -5,10 +5,11 @@ declare(strict_types=1);
 namespace Mailcow\Archive\Redis;
 
 use Mailcow\Archive\Model\Settings;
+use RuntimeException;
 
 /**
  * Redis side of the archive:
- * - ARCHIVE_ENABLED: the switch rspamd reads before it asks for owners, mirrored from the settings.
+ * - ARCHIVE_ENABLED: the global switch, kept here only; rspamd reads it before it asks for owners.
  * - ARCHIVE_DROPPED / ARCHIVE_DROPPED_LOG: copies that could not be handed to Postfix (rspamd.local.lua, release
  *   hooks), counted and the last 100 listed.
  * - POSTFIX_MAILLOG: the Postfix log, read for the archive log: deliveries of the archive_smtp service and expired
@@ -33,6 +34,16 @@ class ArchiveState
         $this->redis?->set(self::ENABLED, $enabled ? '1' : '0');
     }
 
+    /** The global switch: on only while ARCHIVE_ENABLED is '1', as rspamd reads it (missing or unreadable is off) */
+    public function enabled(): bool
+    {
+        try {
+            return $this->redis?->get(self::ENABLED) === '1';
+        } catch (RuntimeException) {
+            return false;
+        }
+    }
+
     /** A mail was not archived: its copy could not be handed to Postfix */
     public function dropped(string $qid, string $reason, string $messageId = ''): void
     {

data/web/inc/lib/mailcow/Archive/Repository/SettingsRepository.php

@@ -19,7 +19,6 @@ class SettingsRepository extends Repository
     {
         $rows = $this->query("SELECT `name`, `value` FROM `archive_settings`")->fetchAll(PDO::FETCH_KEY_PAIR);
         $settings = new Settings();
-        $settings->enabled = ($rows['enabled'] ?? '0') === '1';
         $settings->address = (string)($rows['address'] ?? '');
         $settings->host = (string)($rows['host'] ?? '');
         $settings->port = (int)($rows['port'] ?? $settings->port);
@@ -40,7 +39,6 @@ class SettingsRepository extends Repository
     public function save(Settings $settings): void
     {
         $values = [
-            'enabled' => $settings->enabled ? '1' : '0',
             'address' => $settings->address,
             'host' => $settings->host,
             'port' => (string)$settings->port,

data/web/inc/lib/mailcow/Archive/Service/JournalService.php

@@ -48,8 +48,7 @@ class JournalService
     public function prepare(array $request): array
     {
         $settings = $this->settings->load();
-        $this->state->setEnabled($settings->enabled);
+        if (!$this->state->enabled() || !$settings->canDeliver()) {
-        if (!$settings->enabled || !$settings->canDeliver()) {
             return ['archive' => false];
         }
         $from = self::text($request['from'] ?? '');
@@ -75,7 +74,7 @@ class JournalService
     {
         try {
             $settings = $this->settings->load();
-            if (!$settings->enabled || !$settings->canDeliver()) {
+            if (!$this->state->enabled() || !$settings->canDeliver()) {
                 return false;
             }
             $resolved = $this->resolver->resolve($from, $rcpts, '', false);

data/web/inc/lib/mailcow/Archive/Service/MailboxListService.php

@@ -5,8 +5,8 @@ declare(strict_types=1);
 namespace Mailcow\Archive\Service;
 
 use Mailcow\Archive\Model\Mailbox;
+use Mailcow\Archive\Redis\ArchiveState;
 use Mailcow\Archive\Repository\LookupRepository;
-use Mailcow\Archive\Repository\SettingsRepository;
 use Mailcow\Core\AccessDenied;
 use Mailcow\Core\Context;
 
@@ -16,7 +16,7 @@ use Mailcow\Core\Context;
  */
 class MailboxListService
 {
-    public function __construct(private LookupRepository $lookup, private SettingsRepository $settings, private Context $context)
+    public function __construct(private LookupRepository $lookup, private ArchiveState $state, private Context $context)
     {
     }
 
@@ -29,7 +29,7 @@ class MailboxListService
     public function archived(): array
     {
         $this->context->requireAdmin(false);
-        $enabled = $this->settings->load()->enabled;
+        $enabled = $this->state->enabled();
         return [
             'enabled' => (int)$enabled,
             'mailboxes' => $enabled ? array_map(fn(Mailbox $mailbox): array => $mailbox->toArray(), $this->lookup->archivedMailboxes()) : [],

data/web/inc/lib/mailcow/Archive/Service/SettingsService.php

@@ -14,7 +14,6 @@ use Mailcow\Core\AccessDenied;
 use Mailcow\Core\Context;
 use Mailcow\Core\Input;
 use Mailcow\Core\UserError;
-use PDOException;
 
 /**
  * Admin use cases of the archive target. Read-only API keys may read the settings (never password or key).
@@ -34,17 +33,15 @@ class SettingsService
     public function get(): Settings
     {
         $this->context->requireAdmin(false);
-        return $this->repo->load();
+        $settings = $this->repo->load();
+        $settings->enabled = $this->state->enabled();
+        return $settings;
     }
 
-    /** Global switch for internal callers (page templates, mailbox details), no permission check; off while the table is missing */
+    /** Global switch for internal callers (page templates, mailbox details), no permission check */
     public function enabled(): bool
     {
-        try {
+        return $this->state->enabled();
-            return $this->repo->load()->enabled;
-        } catch (PDOException) {
-            return false;
-        }
     }
 
     /**
@@ -57,6 +54,7 @@ class SettingsService
     {
         $this->context->requireAdmin();
         $settings = $this->repo->load();
+        $settings->enabled = $this->state->enabled();
         if (array_key_exists('address', $input)) {
             $address = strtolower((string)Input::text($input['address']));
             if (!is_scalar($input['address']) || ($address !== '' && !filter_var($address, FILTER_VALIDATE_EMAIL))) {

data/web/inc/lib/mailcow/Archive/Service/StatusService.php

@@ -58,7 +58,7 @@ class StatusService
             }
         }
         return [
-            'enabled' => (int)$settings->enabled,
+            'enabled' => (int)$this->state->enabled(),
             'configured' => (int)$settings->canDeliver(),
             'queued' => count($copies),
             'queued_bytes' => $bytes,

tests/php/Archive/Model/MailboxTest.php

@@ -5,13 +5,13 @@ declare(strict_types=1);
 namespace Mailcow\Tests\Archive\Model;
 
 use Mailcow\Archive\Model\Mailbox;
+use Mailcow\Archive\Redis\ArchiveState;
 use Mailcow\Archive\Service\MailboxListService;
-use Mailcow\Archive\Repository\SettingsRepository;
-use Mailcow\Archive\Model\Settings;
 use Mailcow\Core\Context;
 use Mailcow\Core\UserError;
 use Mailcow\Tests\Support\FakeArchiveLookup;
 use Mailcow\Tests\Support\FakeContext;
+use Mailcow\Tests\Support\FakeRedis;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
@@ -43,20 +43,18 @@ class MailboxTest extends TestCase
     {
         $lookup = new FakeArchiveLookup();
         $lookup->mailboxes = ['b@firma.de' => ['inherit', 1], 'a@other.de' => ['1', 0], 'c@firma.de' => ['0', 1]];
-        $settings = new Settings();
+        $redis = new FakeRedis();
-        $repo = $this->createMock(SettingsRepository::class);
-        $repo->method('load')->willReturnCallback(fn(): Settings => $settings);
         $readOnly = new Context('admin', 'API', [], false, 'ro');
 
-        $this->assertSame(['enabled' => 0, 'mailboxes' => []], (new MailboxListService($lookup, $repo, $readOnly))->archived());
+        $this->assertSame(['enabled' => 0, 'mailboxes' => []], (new MailboxListService($lookup, new ArchiveState($redis), $readOnly))->archived());
 
-        $settings->enabled = true;
+        $redis->strings['ARCHIVE_ENABLED'] = '1';
         $this->assertSame(['enabled' => 1, 'mailboxes' => [
             ['username' => 'b@firma.de', 'domain' => 'firma.de', 'name' => '', 'active' => 1, 'source' => 'domain'],
             ['username' => 'a@other.de', 'domain' => 'other.de', 'name' => '', 'active' => 1, 'source' => 'mailbox'],
-        ]], (new MailboxListService($lookup, $repo, $readOnly))->archived());
+        ]], (new MailboxListService($lookup, new ArchiveState($redis), $readOnly))->archived());
 
         $this->expectException(UserError::class);
-        (new MailboxListService($lookup, $repo, FakeContext::domainAdmin()))->archived();
+        (new MailboxListService($lookup, new ArchiveState($redis), FakeContext::domainAdmin()))->archived();
     }
 }

tests/php/Archive/Redis/ArchiveStateTest.php

@@ -25,12 +25,25 @@ class ArchiveStateTest extends TestCase
         $this->redis->lPush('POSTFIX_MAILLOG', json_encode(['time' => $time, 'program' => $program, 'message' => $message]));
     }
 
-    public function testSwitchIsMirroredForRspamd(): void
+    public function testGlobalSwitchIsKeptHere(): void
     {
+        // no key yet: off
+        $this->assertFalse($this->state->enabled());
         $this->state->setEnabled(true);
         $this->assertSame('1', $this->redis->strings['ARCHIVE_ENABLED']);
+        $this->assertTrue($this->state->enabled());
         $this->state->setEnabled(false);
         $this->assertSame('0', $this->redis->strings['ARCHIVE_ENABLED']);
+        $this->assertFalse($this->state->enabled());
+        // no connection or a failing one: off, rspamd cannot read the switch either
+        $this->assertFalse((new ArchiveState(null))->enabled());
+        $failing = new class {
+            public function get(string $key): string
+            {
+                throw new \RuntimeException('connection lost');
+            }
+        };
+        $this->assertFalse((new ArchiveState($failing))->enabled());
     }
 
     public function testDroppedCopiesAreCountedAndTheLast100Listed(): void

tests/php/Archive/Service/JournalServiceTest.php

@@ -29,7 +29,6 @@ class JournalServiceTest extends TestCase
     protected function setUp(): void
     {
         $this->stored = new Settings();
-        $this->stored->enabled = true;
         $this->stored->address = 'journal@archive.example';
         $this->stored->signingKey = 'key';
         $this->lookup = new FakeArchiveLookup();
@@ -38,6 +37,7 @@ class JournalServiceTest extends TestCase
             $this->lookup->aliases[$username] = $username;
         }
         $this->redis = new FakeRedis();
+        $this->redis->strings['ARCHIVE_ENABLED'] = '1';
         $this->smtp = new FakeSmtpClient();
     }
 
@@ -87,9 +87,8 @@ class JournalServiceTest extends TestCase
         $this->stored->signingKey = '';
         $this->assertSame(['archive' => false], $this->service()->prepare(['from' => 'x@extern.example', 'rcpt' => ['anna@firma.de']]));
 
-        $this->stored->enabled = false;
+        $this->redis->strings['ARCHIVE_ENABLED'] = '0';
         $this->assertSame(['archive' => false], $this->service()->prepare(['from' => 'x@extern.example', 'rcpt' => ['anna@firma.de']]));
-        $this->assertSame('0', $this->redis->strings['ARCHIVE_ENABLED']);
         $this->assertSame([], $this->smtp->submitted);
     }
 
@@ -140,7 +139,7 @@ class JournalServiceTest extends TestCase
 
     public function testTestMail(): void
     {
-        $this->stored->enabled = false;
+        $this->redis->strings['ARCHIVE_ENABLED'] = '0';
 
         $this->assertSame('Q1', $this->service(FakeContext::admin())->testMail());
 

tests/php/Archive/Service/MailboxListServiceTest.php

@@ -4,21 +4,23 @@ declare(strict_types=1);
 
 namespace Mailcow\Tests\Archive\Service;
 
-use Mailcow\Archive\Model\Settings;
+use Mailcow\Archive\Redis\ArchiveState;
-use Mailcow\Archive\Repository\SettingsRepository;
 use Mailcow\Archive\Service\MailboxListService;
 use Mailcow\Core\AccessDenied;
 use Mailcow\Core\Context;
 use Mailcow\Tests\Support\FakeArchiveLookup;
 use Mailcow\Tests\Support\FakeContext;
+use Mailcow\Tests\Support\FakeRedis;
 use PHPUnit\Framework\TestCase;
 
 class MailboxListServiceTest extends TestCase
 {
     private FakeArchiveLookup $lookup;
+    private FakeRedis $redis;
 
     protected function setUp(): void
     {
+        $this->redis = new FakeRedis();
         $this->lookup = new FakeArchiveLookup();
         // username => [mailbox setting, domain default]
         $this->lookup->mailboxes = [
@@ -31,11 +33,8 @@ class MailboxListServiceTest extends TestCase
 
     private function service(bool $enabled, ?Context $context = null): MailboxListService
     {
-        $settings = new Settings();
+        $this->redis->strings['ARCHIVE_ENABLED'] = $enabled ? '1' : '0';
-        $settings->enabled = $enabled;
+        return new MailboxListService($this->lookup, new ArchiveState($this->redis), $context ?? FakeContext::admin());
-        $repo = $this->createStub(SettingsRepository::class);
-        $repo->method('load')->willReturn($settings);
-        return new MailboxListService($this->lookup, $repo, $context ?? FakeContext::admin());
     }
 
     public function testArchivedMailboxesWithTheirSource(): void

tests/php/Archive/Service/SettingsServiceTest.php

@@ -15,7 +15,6 @@ use Mailcow\Tests\Support\FakeArchiveLookup;
 use Mailcow\Tests\Support\FakeContext;
 use Mailcow\Tests\Support\FakeRedis;
 use Mailcow\Tests\Support\FakeSmtpClient;
-use PDOException;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -74,12 +73,19 @@ class SettingsServiceTest extends TestCase
         $this->assertSame('', $this->service()->save(['password' => '', 'password_remove' => '1'])->password);
     }
 
-    public function testEnabledIsOffWithoutTheTable(): void
+    public function testSwitchIsKeptInRedisOnly(): void
     {
-        $repo = $this->createMock(SettingsRepository::class);
+        $this->assertFalse($this->service()->enabled());
-        $repo->method('load')->willThrowException(new PDOException('no such table'));
+        $this->assertSame(0, $this->service()->get()->toArray()['enabled']);
-
+        $this->service()->save(['enabled' => '1']);
-        $this->assertFalse((new SettingsService($repo, $this->lookup, new ArchiveState($this->redis), $this->smtp, FakeContext::admin()))->enabled());
+        $this->assertSame('1', $this->redis->strings['ARCHIVE_ENABLED']);
+        $this->assertSame(1, $this->service()->get()->toArray()['enabled']);
+        // saving other settings keeps the switch
+        $this->service()->save(['port' => '2525']);
+        $this->assertTrue($this->service()->enabled());
+        // a lost Redis key shows as off, as rspamd sees it
+        unset($this->redis->strings['ARCHIVE_ENABLED']);
+        $this->assertFalse($this->service()->enabled());
     }
 
     public function testAddressOutsideTheMailcowDomains(): void

tests/php/Archive/Service/StatusServiceTest.php

@@ -24,12 +24,12 @@ class StatusServiceTest extends TestCase
     protected function setUp(): void
     {
         $this->redis = new FakeRedis();
+        $this->redis->strings['ARCHIVE_ENABLED'] = '1';
     }
 
     private function service(?Context $context = null): StatusService
     {
         $settings = new Settings();
-        $settings->enabled = true;
         $settings->address = 'journal@archive.example';
         $settings->profile = 'none';
         $repo = $this->createStub(SettingsRepository::class);
@@ -70,6 +70,12 @@ class StatusServiceTest extends TestCase
         ], $this->service()->status());
     }
 
+    public function testSwitchFromRedis(): void
+    {
+        $this->redis->strings['ARCHIVE_ENABLED'] = '0';
+        $this->assertSame(0, $this->service()->status()['enabled']);
+    }
+
     public function testLogWithMessageIds(): void
     {
         (new ArchiveState($this->redis))->dropped('4XyZ1', 'owner lookup failed', 'm1@firma.de');