Skip to content

Commit 69e7316

Browse files
committed
refactor(service-messaging): migrate isUniqueViolation onto the shared @objectstack/types predicate (#6542)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USNUyHEr7uaU6MoEWXitei
1 parent 24122a9 commit 69e7316

5 files changed

Lines changed: 86 additions & 20 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@objectstack/service-messaging": patch
3+
---
4+
5+
Migrate the inbox read-receipt race fallback onto the shared `isUniqueViolationError` predicate from `@objectstack/types` (#6542), deleting the last hand-written `isUniqueViolation()` copy that #6250 inventoried. One behaviour change, in the direction the call site wants: the shared predicate follows a bounded step down the `cause` chain, so a unique-constraint conflict wrapped by a pool or query-builder layer now triggers the `flipToRead()` convergence instead of being rethrown as a failed mark-read.

packages/services/service-messaging/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
"dependencies": {
2222
"@objectstack/core": "workspace:*",
2323
"@objectstack/platform-objects": "workspace:*",
24-
"@objectstack/spec": "workspace:*"
24+
"@objectstack/spec": "workspace:*",
25+
"@objectstack/types": "workspace:*"
2526
},
2627
"devDependencies": {
2728
"@objectstack/metadata-core": "workspace:*",

packages/services/service-messaging/src/messaging-service.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,80 @@ describe('MessagingService — inbox read API (ADR-0030)', () => {
568568
expect(receipts[0]).toMatchObject({ id: 'r_concurrent', state: 'read' });
569569
});
570570

571+
it('markRead converges when the conflict arrives wrapped in a cause chain (#6542)', async () => {
572+
// Pool and query-builder layers re-throw with the driver error attached
573+
// as `cause`. The retired local isUniqueViolation() read only the
574+
// top-level code/message, judged this wrapper "not a conflict", and
575+
// rethrew — markRead logged the failure and reported readCount 0 with
576+
// the receipt stuck at `delivered`. The shared isUniqueViolationError
577+
// (@objectstack/types, #6250) follows the bounded cause chain, so the
578+
// race fallback now converges exactly as it does for a bare driver
579+
// error. This is the ONE behaviour change of the migration, in the
580+
// direction the call site wants: a wrapped conflict is still a conflict.
581+
const engine = inboxEngine({
582+
inbox: [{ id: 'm1', user_id: 'u1', notification_id: 'n1', title: 'A', created_at: '1' }],
583+
});
584+
const realInsert = engine.insert.bind(engine);
585+
let raced = false;
586+
engine.insert = async (object: string, row: any) => {
587+
if (object === 'sys_notification_receipt' && !raced) {
588+
raced = true;
589+
engine.store.sys_notification_receipt.push({
590+
id: 'r_concurrent', notification_id: 'n1', user_id: 'u1', channel: 'inbox', state: 'delivered',
591+
});
592+
// The wrapper's own text deliberately matches neither the codes
593+
// nor the message substrings the predicate knows — only the
594+
// attached driver error carries the conflict signal.
595+
const wrapper = new Error('receipt insert failed (pool retry exhausted)');
596+
(wrapper as Error & { cause?: unknown }).cause = {
597+
code: '23505',
598+
message: 'duplicate key value violates unique constraint "sys_notification_receipt_notification_id_user_id_channel_key"',
599+
};
600+
throw wrapper;
601+
}
602+
return realInsert(object, row);
603+
};
604+
const svc = new MessagingService({ logger, getData: () => engine });
605+
606+
const res = await svc.markRead('u1', ['n1']);
607+
expect(res).toEqual({ success: true, readCount: 1 });
608+
const receipts = engine.store.sys_notification_receipt;
609+
expect(receipts).toHaveLength(1); // converged, not duplicated
610+
expect(receipts[0]).toMatchObject({ id: 'r_concurrent', state: 'read' });
611+
});
612+
613+
it.each([
614+
['postgres', { code: '23505', message: 'duplicate key value violates unique constraint "sys_notification_receipt_uniq"' }],
615+
['mysql', { code: 'ER_DUP_ENTRY', errno: 1062, message: "Duplicate entry 'n1-u1-inbox' for key 'sys_notification_receipt_uniq'" }],
616+
['sqlite', { message: 'UNIQUE constraint failed: sys_notification_receipt.notification_id, sys_notification_receipt.user_id, sys_notification_receipt.channel' }],
617+
])('markRead race fallback is unchanged for a plain (unwrapped) %s conflict (#6542)', async (_dialect, shape) => {
618+
// Pin that the migration onto the shared predicate did not move any
619+
// verdict the old local copy already gave: a bare three-dialect driver
620+
// error still converges identically.
621+
const engine = inboxEngine({
622+
inbox: [{ id: 'm1', user_id: 'u1', notification_id: 'n1', title: 'A', created_at: '1' }],
623+
});
624+
const realInsert = engine.insert.bind(engine);
625+
let raced = false;
626+
engine.insert = async (object: string, row: any) => {
627+
if (object === 'sys_notification_receipt' && !raced) {
628+
raced = true;
629+
engine.store.sys_notification_receipt.push({
630+
id: 'r_concurrent', notification_id: 'n1', user_id: 'u1', channel: 'inbox', state: 'delivered',
631+
});
632+
throw Object.assign(new Error(shape.message), shape);
633+
}
634+
return realInsert(object, row);
635+
};
636+
const svc = new MessagingService({ logger, getData: () => engine });
637+
638+
const res = await svc.markRead('u1', ['n1']);
639+
expect(res).toEqual({ success: true, readCount: 1 });
640+
const receipts = engine.store.sys_notification_receipt;
641+
expect(receipts).toHaveLength(1);
642+
expect(receipts[0]).toMatchObject({ id: 'r_concurrent', state: 'read' });
643+
});
644+
571645
it('markAllRead flips every unread message and leaves already-read ones', async () => {
572646
const engine = inboxEngine({
573647
inbox: [

packages/services/service-messaging/src/messaging-service.ts

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import type { IDataEngine } from '@objectstack/spec/contracts';
4+
import { isUniqueViolationError } from '@objectstack/types';
45
import type {
56
MessagingChannel,
67
MessagingChannelContext,
@@ -18,24 +19,6 @@ export const NOTIFICATION_EVENT_OBJECT = 'sys_notification';
1819
/** Receipt states that count as "read" for the inbox unread badge (ADR-0030). */
1920
const READ_RECEIPT_STATES = new Set(['read', 'clicked', 'dismissed']);
2021

21-
/**
22-
* Whether a driver error is a unique/primary-key constraint violation. Spans the
23-
* SQL drivers we ship: SQLite (`UNIQUE constraint failed`), Postgres (`23505` /
24-
* `duplicate key`), and MySQL (`ER_DUP_ENTRY` / `Duplicate entry`). Used to turn
25-
* a lost check-then-act race on a unique index into a fallback update.
26-
*/
27-
function isUniqueViolation(err: unknown): boolean {
28-
const e = err as { code?: string | number; message?: string } | undefined;
29-
if (!e) return false;
30-
if (e.code === '23505' || e.code === 'ER_DUP_ENTRY' || e.code === 'SQLITE_CONSTRAINT_UNIQUE') return true;
31-
const msg = String(e.message ?? '').toLowerCase();
32-
return (
33-
msg.includes('unique constraint failed') ||
34-
msg.includes('duplicate key') ||
35-
msg.includes('duplicate entry')
36-
);
37-
}
38-
3922
/**
4023
* One row of the inbox list REST response — the `Notification` shape in the API
4124
* spec (`NotificationSchema`). `id` is the notification's event id
@@ -569,7 +552,7 @@ export class MessagingService {
569552
});
570553
return 1;
571554
} catch (err) {
572-
if (isUniqueViolation(err) && (await flipToRead())) return 1;
555+
if (isUniqueViolationError(err) && (await flipToRead())) return 1;
573556
throw err;
574557
}
575558
}

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)