From ac87c0e2d61b38763c888f89d583153e23d05a77 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 May 2026 18:02:12 +1200 Subject: [PATCH] test(notifications): add regression and happy-path coverage for review-fix critical gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Locks the bug-fix invariants from PR #12195's last review pass and rounds out worker channel coverage: C1 testEmailSendFailureDoesNotPersistAlert — SMTP throw must NOT leave a dedup row behind, retry must deliver and persist exactly once. C2 testNotificationEventResetClearsAllState — reset() drops every state-bearing field including preview (regression: missed in original reset body). C3 testWebhookSendAlertResetsBetweenCalls — Webhooks::sendAlert must reset() the DI-shared Notification event so two paused-webhook alerts in one worker pass do not bleed recipients/subject/body into each other. C4 testConsoleAdapterTreatsDuplicateAsDelivered — Duplicate on createDocument must surface as a successful idempotent send, not a per-recipient error. M7 testTrackingPixelRejectsJwtWithoutPurposeClaim — Track endpoint silently ignores JWTs missing or with the wrong purpose claim (defends against replaying session/reset JWTs to mark alerts read). Worker happy-path tests: testEmailChannelHappyPath, testConsoleChannelHappyPath, testWebhookChannelHappyPath cover the full per-channel dispatch contract end to end, including HMAC signing for webhooks and the tracking pixel injection + post-send persistence for email. Also extracts CapturingWebhook into its own PSR-4 file so reused across tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Notifications/NotificationsBase.php | 80 ++++ .../Platform/Workers/NotificationsTest.php | 441 ++++++++++++++++++ .../Messaging/Adapter/CapturingWebhook.php | 34 ++ .../Utopia/Messaging/Adapter/ConsoleTest.php | 50 ++ .../Utopia/Messaging/Adapter/WebhookTest.php | 29 -- 5 files changed, 605 insertions(+), 29 deletions(-) create mode 100644 tests/unit/Utopia/Messaging/Adapter/CapturingWebhook.php diff --git a/tests/e2e/Services/Notifications/NotificationsBase.php b/tests/e2e/Services/Notifications/NotificationsBase.php index 19a64411ac..5423e4de3d 100644 --- a/tests/e2e/Services/Notifications/NotificationsBase.php +++ b/tests/e2e/Services/Notifications/NotificationsBase.php @@ -194,9 +194,13 @@ trait NotificationsBase $this->assertNotEmpty($secret, '_APP_OPENSSL_KEY_V1 must be set for tracking pixel test'); $userId = $this->getRoot()['$id']; + // Track endpoint requires `purpose: 'alert_track'` — see C/M7 in + // PR #12195 review. Other claim purposes are silently ignored (which + // testTrackingPixelRejectsJwtWithoutPurposeClaim covers). $jwt = (new JWT($secret, 'HS256', 2592000, 0))->encode([ 'alertId' => $alertId, 'userId' => $userId, + 'purpose' => 'alert_track', ]); $response = $this->client->call( @@ -229,6 +233,82 @@ trait NotificationsBase self::$seededAlertId = null; } + /** + * Reviewer M7: a tracking JWT without a `purpose: 'alert_track'` claim + * must be silently rejected. Without the purpose check, any JWT minted + * with the same secret (sessions, password reset, etc.) could be + * replayed against this endpoint to mark arbitrary alerts as read. + * + * The endpoint always returns the 1x1 PNG (200 image/png) — the only + * observable difference is whether the alert flips to `read: true`. + */ + public function testTrackingPixelRejectsJwtWithoutPurposeClaim(): void + { + $alertId = $this->seedWebhookFailureAlert(); + $this->assertNotEmpty($alertId); + + $secret = System::getEnv('_APP_OPENSSL_KEY_V1'); + $this->assertNotEmpty($secret, '_APP_OPENSSL_KEY_V1 must be set for the JWT purpose-claim test'); + $userId = $this->getRoot()['$id']; + + // Mint a JWT with valid alertId/userId but NO purpose claim. + $jwtNoPurpose = (new JWT($secret, 'HS256', 2592000, 0))->encode([ + 'alertId' => $alertId, + 'userId' => $userId, + ]); + + $response = $this->client->call( + Client::METHOD_GET, + '/account/alerts/' . $alertId . '/track', + ['x-appwrite-project' => 'console'], + ['jwt' => $jwtNoPurpose] + ); + + $this->assertSame(200, $response['headers']['status-code'], 'endpoint must always return 200'); + $this->assertStringContainsString('image/png', $response['headers']['content-type']); + + $list = $this->client->call(Client::METHOD_GET, '/account/alerts', $this->getConsoleAlertHeaders()); + $found = null; + foreach ($list['body']['alerts'] as $alert) { + if ($alert['$id'] === $alertId) { + $found = $alert; + break; + } + } + $this->assertNotNull($found); + $this->assertFalse($found['read'], 'JWT without purpose claim must not flip the read flag'); + + // Mint a JWT with a wrong purpose value — same expectation: silently rejected. + $jwtWrongPurpose = (new JWT($secret, 'HS256', 2592000, 0))->encode([ + 'alertId' => $alertId, + 'userId' => $userId, + 'purpose' => 'something_else', + ]); + + $response = $this->client->call( + Client::METHOD_GET, + '/account/alerts/' . $alertId . '/track', + ['x-appwrite-project' => 'console'], + ['jwt' => $jwtWrongPurpose] + ); + + $this->assertSame(200, $response['headers']['status-code']); + $this->assertStringContainsString('image/png', $response['headers']['content-type']); + + $list = $this->client->call(Client::METHOD_GET, '/account/alerts', $this->getConsoleAlertHeaders()); + $found = null; + foreach ($list['body']['alerts'] as $alert) { + if ($alert['$id'] === $alertId) { + $found = $alert; + break; + } + } + $this->assertNotNull($found); + $this->assertFalse($found['read'], 'JWT with wrong purpose value must not flip the read flag'); + + self::$seededAlertId = $alertId; + } + public function testTrackingPixelInvalidTokenReturnsPng(): void { $alertId = $this->seedWebhookFailureAlert(); diff --git a/tests/unit/Platform/Workers/NotificationsTest.php b/tests/unit/Platform/Workers/NotificationsTest.php index c0aca4485f..f5cc60c04c 100644 --- a/tests/unit/Platform/Workers/NotificationsTest.php +++ b/tests/unit/Platform/Workers/NotificationsTest.php @@ -2,8 +2,10 @@ namespace Tests\Unit\Platform\Workers; +use Appwrite\Event\Notification; use Appwrite\Platform\Workers\Notifications; use PHPUnit\Framework\TestCase; +use Tests\Unit\Event\MockPublisher; use Utopia\Cache\Adapter\None as NoCache; use Utopia\Cache\Cache; use Utopia\Database\Adapter\Memory; @@ -109,10 +111,16 @@ class ZeroDeliveryConsoleNotifications extends Notifications /** * Captures the EmailMessage handed to the SMTP adapter so tests can assert * on the rendered HTML body without touching a real mail server. + * + * Set `$throwOnSend = true` to simulate a hard SMTP failure (DNS, refused + * connection, auth error). The adapter's `send()` calls `process()` directly, + * so a throw from here propagates exactly like a real PHPMailer error. */ class SpyEmailAdapter extends EmailAdapter { public ?EmailMessage $captured = null; + public int $sendCount = 0; + public bool $throwOnSend = false; public function getName(): string { @@ -126,7 +134,13 @@ class SpyEmailAdapter extends EmailAdapter protected function process(EmailMessage $message): array { + $this->sendCount++; $this->captured = $message; + + if ($this->throwOnSend) { + throw new \Exception('SMTP unavailable'); + } + return [ 'deliveredTo' => 1, 'type' => $this->getType(), @@ -697,4 +711,431 @@ class NotificationsTest extends TestCase $this->assertFalse($row->getAttribute('read'), 'new alerts default to unread'); $this->assertSame(\md5('persist-email'), $row->getAttribute('messageId')); } + + /** + * Reviewer C1: SMTP failure must NOT orphan a dedup row. + * + * Order in the worker matters: persist BEFORE send leaves a poisoned + * `messageId` row that the next retry will dedup-hit and never deliver. + * The fix is to persist only after a successful adapter send. A retry + * with the same payload must therefore actually deliver and produce + * exactly one alert row. + */ + public function testEmailSendFailureDoesNotPersistAlert(): void + { + $failing = new SpyEmailAdapter(); + $failing->throwOnSend = true; + $this->registry->set('smtp', static fn () => $failing); + + $previousSmtpHost = \getenv('_APP_SMTP_HOST'); + \putenv('_APP_SMTP_HOST=spy.smtp.test'); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + [ + 'address' => 'user@example.test', + 'channel' => NOTIFICATION_TYPE_EMAIL, + 'userId' => 'user-9', + ], + ], + 'subject' => 'Subj', + 'body' => 'Body', + 'deduplicationKey' => 'smtp-fail-key', + ]; + + $messageId = \md5('smtp-fail-key'); + + try { + $worker = new Notifications(); + + $threw = false; + try { + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + } catch (\Throwable $error) { + $threw = true; + $this->assertStringContainsString('SMTP unavailable', $error->getMessage()); + } + $this->assertTrue($threw, 'SMTP failure must propagate so the queue retries'); + $this->assertSame(1, $failing->sendCount, 'adapter must have been invoked exactly once'); + + // Critical: no orphan dedup row. If there is one, the retry below + // will short-circuit and the user never gets the email. + $orphans = $this->database->find('alerts', [ + \Utopia\Database\Query::equal('messageId', [$messageId]), + ]); + $this->assertCount(0, $orphans, 'failed SMTP send must not leave a dedup row behind'); + + // Retry with a working adapter using the same payload — must deliver + // AND persist exactly one alert row. + $working = new SpyEmailAdapter(); + $this->registry->set('smtp', static fn () => $working); + + $retryWorker = new Notifications(); + $retryWorker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + + $this->assertSame(1, $working->sendCount, 'retry must invoke the working adapter'); + + $rows = $this->database->find('alerts', [ + \Utopia\Database\Query::equal('messageId', [$messageId]), + ]); + $this->assertCount(1, $rows, 'retry must persist exactly one alert row'); + $this->assertSame('user-9', $rows[0]->getAttribute('userId')); + $this->assertFalse($rows[0]->getAttribute('read')); + } finally { + \putenv($previousSmtpHost === false ? '_APP_SMTP_HOST' : '_APP_SMTP_HOST=' . $previousSmtpHost); + } + } + + /** + * Reviewer C2: `Notification::reset()` must clear EVERY state-bearing + * field — `$preview` was missing from the original reset() body and + * leaked across DI-shared event reuses (alarming when a webhook-paused + * preview line bleeds into an unrelated alert). + */ + public function testNotificationEventResetClearsAllState(): void + { + $event = new Notification(new MockPublisher()); + + $event + ->setProject(new Document(['$id' => 'project-x'])) + ->setRecipient('legacy@example.test') + ->setName('Some Name') + ->setSubject('Subject Line') + ->setBody('Body content') + ->setPreview('Preview snippet') + ->setVariables(['key' => 'value']) + ->setBodyTemplate('/tmp/template.tpl') + ->setAttachment('content', 'file.txt') + ->setRecipients([ + ['address' => 'a@example.test', 'channel' => NOTIFICATION_TYPE_EMAIL], + ]) + ->setChannels([NOTIFICATION_TYPE_EMAIL]) + ->setTemplate('template-id') + ->setTemplateParams(['x' => 1]) + ->setDeduplicationKey('dedup') + ->setPermissions([Permission::read(Role::any())]); + + $event->reset(); + + $this->assertSame('', $event->getRecipient(), 'recipient must reset to empty string'); + $this->assertSame('', $event->getName(), 'name must reset to empty string'); + $this->assertSame('', $event->getSubject(), 'subject must reset to empty string'); + $this->assertSame('', $event->getBody(), 'body must reset to empty string'); + $this->assertSame('', $event->getPreview(), 'preview must reset to empty string (C2 regression)'); + $this->assertSame([], $event->getVariables(), 'variables must reset to empty array'); + $this->assertSame('', $event->getBodyTemplate(), 'bodyTemplate must reset to empty string'); + $this->assertSame([], $event->getAttachment(), 'attachment must reset to empty array'); + $this->assertSame([], $event->getRecipients(), 'recipients must reset to empty array'); + $this->assertSame([], $event->getChannels(), 'channels must reset to empty array'); + $this->assertSame('', $event->getTemplate(), 'template must reset to empty string'); + $this->assertSame([], $event->getTemplateParams(), 'templateParams must reset to empty array'); + $this->assertSame('', $event->getDeduplicationKey(), 'deduplicationKey must reset to empty string'); + $this->assertSame([], $event->getPermissions(), 'permissions must reset to empty array'); + $this->assertNull($event->getProject(), 'project must reset to null'); + } + + /** + * Reviewer C3: `Webhooks::sendAlert` resets the DI-shared Notification + * event before configuring the alert, so a second invocation in the same + * worker pass cannot bleed recipients, subject, body, preview, or dedup + * key from the first. + * + * We exercise this at the event level (rather than standing up the full + * Webhooks worker fixture, which needs `dbForPlatform.memberships` and + * full project shape) by replicating sendAlert's add → trigger → reset → + * add → trigger flow on a single Notification instance and asserting + * each enqueued payload is fully isolated. + */ + public function testWebhookSendAlertResetsBetweenCalls(): void + { + $publisher = new MockPublisher(); + $event = new Notification($publisher); + + // First "sendAlert" pass: a single user, webhook A. + $event + ->setProject(new Document(['$id' => 'project-a'])) + ->setSubject('Webhook A paused') + ->setPreview('Webhook A preview') + ->setBody('Webhook A body') + ->setDeduplicationKey('webhook:hookA:paused:10') + ->addRecipient('alice@example.test', NOTIFICATION_TYPE_EMAIL, null, 'user-A', 'team-1') + ->addRecipient('user-A', NOTIFICATION_TYPE_CONSOLE, null, 'user-A', 'team-1') + ->trigger(); + + // Mirror the production sendAlert: reset before configuring next alert. + $event->reset(); + + // Second pass: a different user/webhook entirely. + $event + ->setProject(new Document(['$id' => 'project-b'])) + ->setSubject('Webhook B paused') + ->setPreview('Webhook B preview') + ->setBody('Webhook B body') + ->setDeduplicationKey('webhook:hookB:paused:10') + ->addRecipient('bob@example.test', NOTIFICATION_TYPE_EMAIL, null, 'user-B', 'team-2') + ->addRecipient('user-B', NOTIFICATION_TYPE_CONSOLE, null, 'user-B', 'team-2') + ->trigger(); + + $events = $publisher->getEvents('v1-notifications'); + $this->assertNotNull($events, 'two trigger() calls must enqueue at least one batch'); + $this->assertCount(2, $events, 'each sendAlert pass must produce its own enqueued event'); + + $first = $events[0]; + $second = $events[1]; + + $this->assertSame('Webhook A paused', $first['subject']); + $this->assertSame('Webhook A preview', $first['preview']); + $this->assertSame('Webhook A body', $first['body']); + $this->assertSame('webhook:hookA:paused:10', $first['deduplicationKey']); + $this->assertCount(2, $first['recipients']); + + $this->assertSame('Webhook B paused', $second['subject']); + $this->assertSame('Webhook B preview', $second['preview']); + $this->assertSame('Webhook B body', $second['body']); + $this->assertSame('webhook:hookB:paused:10', $second['deduplicationKey']); + $this->assertCount(2, $second['recipients'], 'second event must hold ONLY its own recipients'); + + $secondAddresses = \array_map(static fn ($r) => $r['address'], $second['recipients']); + $this->assertNotContains('alice@example.test', $secondAddresses, 'reset() must drop prior email recipient'); + $this->assertNotContains('user-A', $secondAddresses, 'reset() must drop prior console recipient'); + $this->assertContains('bob@example.test', $secondAddresses); + $this->assertContains('user-B', $secondAddresses); + } + + /** + * Worker happy-path: email channel. + * + * Asserts the SMTP adapter is invoked once with the expected + * to/subject/body, the rendered body carries the tracking pixel before + * ``, and an alert row is persisted AFTER the send returns + * successfully (see C1: persist-after-send invariant). + */ + public function testEmailChannelHappyPath(): void + { + $spy = new SpyEmailAdapter(); + $this->registry->set('smtp', static fn () => $spy); + + $previousSmtpHost = \getenv('_APP_SMTP_HOST'); + $previousOpensslKey = \getenv('_APP_OPENSSL_KEY_V1'); + \putenv('_APP_SMTP_HOST=spy.smtp.test'); + \putenv('_APP_OPENSSL_KEY_V1=test-key-32bytes-min-aaaaaaaaaaaaaa'); + + try { + $worker = new CountingPersistAlertNotifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + [ + 'address' => 'happy@example.test', + 'channel' => NOTIFICATION_TYPE_EMAIL, + 'userId' => 'user-happy', + ], + ], + 'subject' => 'Welcome aboard', + 'body' => 'plain body', + 'deduplicationKey' => 'happy-email', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + } finally { + \putenv($previousSmtpHost === false ? '_APP_SMTP_HOST' : '_APP_SMTP_HOST=' . $previousSmtpHost); + \putenv($previousOpensslKey === false ? '_APP_OPENSSL_KEY_V1' : '_APP_OPENSSL_KEY_V1=' . $previousOpensslKey); + } + + $this->assertSame(1, $spy->sendCount, 'SMTP send must be invoked exactly once'); + $this->assertNotNull($spy->captured); + + $message = $spy->captured; + $this->assertSame('happy@example.test', $message->getTo()[0]['email'] ?? ''); + $this->assertSame('Welcome aboard', $message->getSubject()); + + $body = $message->getContent(); + $this->assertStringContainsString('assertStringContainsString('/v1/account/alerts/', $body); + $this->assertStringContainsString('/track?jwt=', $body); + + $closing = \strripos($body, ''); + $pixel = \strripos($body, ''); + + $this->assertSame(1, $worker->persistAlertCalls, 'email channel must persist exactly once after a successful send'); + $messageId = \md5('happy-email'); + $rows = $this->database->find('alerts', [ + \Utopia\Database\Query::equal('messageId', [$messageId]), + ]); + $this->assertCount(1, $rows); + $row = $rows[0]; + $this->assertSame('user-happy', $row->getAttribute('userId')); + $this->assertSame(NOTIFICATION_TYPE_EMAIL, $row->getAttribute('channel')); + $this->assertFalse($row->getAttribute('read')); + + // dispatchEmail's returned alertId must match the row $id (used by the + // tracking pixel URL). + $this->assertSame($worker->persistedIds[0], $row->getId()); + } + + /** + * Worker happy-path: console channel. + * + * The Console adapter writes the alert directly; the action loop must + * NOT call `persistAlert` for console recipients. Permissions must grant + * the recipient user AND team owners read/update/delete. + */ + public function testConsoleChannelHappyPath(): void + { + $worker = new CountingPersistAlertNotifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + [ + 'address' => 'console-recipient', + 'channel' => NOTIFICATION_TYPE_CONSOLE, + 'userId' => 'u1', + 'teamId' => 't1', + ], + ], + 'subject' => 'Heads up', + 'body' => 'console body', + 'deduplicationKey' => 'happy-console', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + + $rows = $this->database->find('alerts', [ + \Utopia\Database\Query::equal('channel', ['console']), + ]); + $this->assertCount(1, $rows, 'console adapter must write exactly one alert'); + + $row = $rows[0]; + $this->assertSame('u1', $row->getAttribute('userId')); + $this->assertSame('t1', $row->getAttribute('teamId')); + $this->assertSame(NOTIFICATION_TYPE_CONSOLE, $row->getAttribute('channel')); + $this->assertFalse($row->getAttribute('read')); + + // Per-recipient suffix scheme used by the Console adapter is + // `messageId . '_' . substr(md5('user:' . userId), 0, 8)`. + $messageId = \md5('happy-console'); + $expectedId = $messageId . '_' . \substr(\md5('user:u1'), 0, 8); + $this->assertSame($expectedId, $row->getId(), 'row $id must match adapter suffix scheme'); + + $permissions = $row->getPermissions(); + $this->assertContains(Permission::read(Role::user('u1')), $permissions); + $this->assertContains(Permission::update(Role::user('u1')), $permissions); + $this->assertContains(Permission::delete(Role::user('u1')), $permissions); + $this->assertContains(Permission::read(Role::team('t1')), $permissions); + $this->assertContains(Permission::update(Role::team('t1', 'owner')), $permissions); + $this->assertContains(Permission::delete(Role::team('t1', 'owner')), $permissions); + + $this->assertSame(0, $worker->persistAlertCalls, 'console channel must NOT trigger action-loop persistAlert'); + } + + /** + * Worker happy-path: webhook channel. + * + * The webhook adapter receives a POST with the rendered subject/body, + * an `X-Appwrite-Webhook-Signature` header derived from the per-recipient + * `signatureKey`, and a single alert row is persisted by the action loop + * AFTER the send returns successfully (webhook adapters do not persist + * themselves). + * + * We swap in a worker subclass that uses the in-process `CapturingWebhook` + * adapter so we exercise the real signing path without touching the network. + */ + public function testWebhookChannelHappyPath(): void + { + $captured = []; + $worker = new class ($captured) extends CountingPersistAlertNotifications { + /** @param array> $captured */ + public function __construct(public array &$captured) + { + parent::__construct(); + } + + protected function dispatchWebhook(array $recipient, array $payload, Log $log): ?string + { + $adapter = new \Tests\Unit\Utopia\Messaging\Adapter\CapturingWebhook(); + $message = new \Appwrite\Utopia\Messaging\Messages\Webhook( + urls: [$recipient['address']], + payload: [ + 'subject' => $payload['subject'] ?? '', + 'body' => $payload['body'] ?? '', + 'template' => $payload['template'] ?? '', + 'params' => $payload['templateParams'] ?? [], + 'project' => \is_array($payload['project'] ?? null) ? ($payload['project']['$id'] ?? null) : null, + 'deduplicationKey' => $payload['deduplicationKey'] ?? '', + 'events' => $payload['events'] ?? [], + ], + signingSecret: $recipient['signatureKey'] ?? null, + ); + + $result = $adapter->send($message); + $this->captured = $adapter->captured; + + if (($result['deliveredTo'] ?? 0) === 0) { + throw new \Exception('Webhook delivery failed'); + } + return null; + } + }; + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + [ + 'address' => 'https://hooks.example.test/in', + 'channel' => NOTIFICATION_TYPE_WEBHOOK, + 'signatureKey' => 'tenant-secret', + 'userId' => 'user-h', + ], + ], + 'subject' => 'Heads up', + 'body' => 'webhook body', + 'deduplicationKey' => 'happy-webhook', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + + $this->assertCount(1, $worker->captured, 'adapter must POST exactly once'); + $request = $worker->captured[0]; + + $this->assertSame('POST', $request['method']); + $this->assertSame('https://hooks.example.test/in', $request['url']); + + $sent = \json_decode($request['body'], true); + $this->assertSame('Heads up', $sent['subject']); + $this->assertSame('webhook body', $sent['body']); + + $headerLine = \implode("\n", $request['headers']); + $this->assertStringContainsString('Content-Type: application/json', $headerLine); + $this->assertStringContainsString('X-Appwrite-Webhook-Signature: sha256=', $headerLine); + + // Verify the HMAC matches the signing secret on the recipient struct. + $signature = null; + $timestamp = null; + foreach ($request['headers'] as $header) { + if (\str_starts_with($header, 'X-Appwrite-Webhook-Signature: ')) { + $signature = \substr($header, \strlen('X-Appwrite-Webhook-Signature: ')); + } elseif (\str_starts_with($header, 'X-Appwrite-Webhook-Timestamp: ')) { + $timestamp = \substr($header, \strlen('X-Appwrite-Webhook-Timestamp: ')); + } + } + $this->assertNotNull($signature); + $this->assertNotNull($timestamp); + $expected = 'sha256=' . \hash_hmac('sha256', $timestamp . '.' . $request['body'], 'tenant-secret'); + $this->assertSame($expected, $signature); + + // Action loop must persist exactly one webhook alert row AFTER the send. + $this->assertSame(1, $worker->persistAlertCalls); + $rows = $this->database->find('alerts', [ + \Utopia\Database\Query::equal('messageId', [\md5('happy-webhook')]), + ]); + $this->assertCount(1, $rows); + $this->assertSame(NOTIFICATION_TYPE_WEBHOOK, $rows[0]->getAttribute('channel')); + $this->assertFalse($rows[0]->getAttribute('read')); + } } diff --git a/tests/unit/Utopia/Messaging/Adapter/CapturingWebhook.php b/tests/unit/Utopia/Messaging/Adapter/CapturingWebhook.php new file mode 100644 index 0000000000..093a3cffb7 --- /dev/null +++ b/tests/unit/Utopia/Messaging/Adapter/CapturingWebhook.php @@ -0,0 +1,34 @@ +, body: string, timeout: int}> + */ + public array $captured = []; + + /** @var array{statusCode: int, response: string|null, error: string|null} */ + public array $response = ['statusCode' => 200, 'response' => 'OK', 'error' => null]; + + protected function dispatch(string $method, string $url, array $headers, string $body, int $timeout): array + { + $this->captured[] = [ + 'method' => $method, + 'url' => $url, + 'headers' => $headers, + 'body' => $body, + 'timeout' => $timeout, + ]; + return $this->response; + } +} diff --git a/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php b/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php index d1165fa002..5ed51d8db1 100644 --- a/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php +++ b/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php @@ -169,4 +169,54 @@ class ConsoleTest extends TestCase // ConsoleMessage extends nothing — pass an unrelated Message implementation $adapter->send(new \Appwrite\Utopia\Messaging\Messages\Webhook(urls: ['https://example.test'], payload: [])); } + + /** + * Reviewer C4: a `DuplicateException` thrown by `createDocument` must be + * treated as a SUCCESSFUL delivery, not a failure. The adapter previously + * lumped Duplicate into the generic Throwable catch, which surfaced as a + * per-recipient `error` and caused the worker to throw — re-queueing the + * notification and never marking the duplicate as delivered. + */ + public function testConsoleAdapterTreatsDuplicateAsDelivered(): void + { + $userId = 'user-dup'; + $messageId = 'msg-dup'; + $documentId = $this->alertId($messageId, userId: $userId); + + // Pre-insert an alert with the SAME id the adapter will compute. The + // adapter's createDocument will hit the primary-key DuplicateException + // and must treat it as a successful (idempotent) send. + $this->database->createDocument('alerts', new \Utopia\Database\Document([ + '$id' => $documentId, + '$permissions' => [Permission::read(Role::any())], + 'messageId' => $messageId, + 'channel' => 'console', + 'userId' => $userId, + 'title' => 'pre-existing', + 'body' => 'pre-existing', + ])); + + $message = new ConsoleMessage( + recipients: [['userId' => $userId]], + title: 'Same alert resent', + body: 'b', + messageId: ID::custom($messageId), + projectId: 'project-x', + ); + + $adapter = new Console($this->database); + $result = $adapter->send($message); + + $this->assertSame(1, $result['deliveredTo'], 'duplicate must count as a successful delivery'); + $this->assertCount(1, $result['results']); + $this->assertSame('success', $result['results'][0]['status'] ?? '', 'duplicate must report success status'); + $this->assertSame('', $result['results'][0]['error'] ?? 'unset', 'duplicate must not surface a per-recipient error'); + + // Still exactly ONE row — the pre-existing one. The adapter must not + // overwrite it nor create a sibling. + $rows = $this->database->find('alerts'); + $this->assertCount(1, $rows); + $this->assertSame($documentId, $rows[0]->getId()); + $this->assertSame('pre-existing', $rows[0]->getAttribute('title'), 'duplicate path must not overwrite existing row'); + } } diff --git a/tests/unit/Utopia/Messaging/Adapter/WebhookTest.php b/tests/unit/Utopia/Messaging/Adapter/WebhookTest.php index 5ad1a8c348..f396954949 100644 --- a/tests/unit/Utopia/Messaging/Adapter/WebhookTest.php +++ b/tests/unit/Utopia/Messaging/Adapter/WebhookTest.php @@ -2,38 +2,9 @@ namespace Tests\Unit\Utopia\Messaging\Adapter; -use Appwrite\Utopia\Messaging\Adapter\Webhook; use Appwrite\Utopia\Messaging\Messages\Webhook as WebhookMessage; use PHPUnit\Framework\TestCase; -/** - * Test double that captures the curl request the adapter would issue and - * returns a scripted response, so we exercise the real signing/header logic - * without touching the network. - */ -class CapturingWebhook extends Webhook -{ - /** - * @var array, body: string, timeout: int}> - */ - public array $captured = []; - - /** @var array{statusCode: int, response: string|null, error: string|null} */ - public array $response = ['statusCode' => 200, 'response' => 'OK', 'error' => null]; - - protected function dispatch(string $method, string $url, array $headers, string $body, int $timeout): array - { - $this->captured[] = [ - 'method' => $method, - 'url' => $url, - 'headers' => $headers, - 'body' => $body, - 'timeout' => $timeout, - ]; - return $this->response; - } -} - class WebhookTest extends TestCase { public function testPostsExpectedBodyShape(): void