diff --git a/packages/backend/src/actors/FingerprintStore.ts b/packages/backend/src/actors/FingerprintStore.ts index 9dc4f4b8..3a43a07a 100644 --- a/packages/backend/src/actors/FingerprintStore.ts +++ b/packages/backend/src/actors/FingerprintStore.ts @@ -50,14 +50,11 @@ export class FingerprintStore extends SubscribableActor>(message, { StoreFingerprint: async fingerprint => { - this.state = await create(this.state, async draft => { - const currentFingerprint = (await this.getEntity(fingerprint.uid)).orElse({} as Fingerprint); - fingerprint.updated = toTimestamp(); - const newFingerprint = { ...currentFingerprint, ...fingerprint }; - draft.cache.set(fingerprint.uid, newFingerprint); - logger.debug(`Storing new fingerprint ${JSON.stringify(newFingerprint)}`); - this.storeEntity(newFingerprint); - }); + const currentFingerprint = (await this.getEntity(fingerprint.uid)).orElse({} as Fingerprint); + fingerprint.updated = toTimestamp(); + const newFingerprint = { ...currentFingerprint, ...fingerprint }; + logger.debug(`Storing new fingerprint ${JSON.stringify(newFingerprint)}`); + await this.storeEntity(newFingerprint); return unit(); }, Get: async id => { @@ -66,49 +63,42 @@ export class FingerprintStore extends SubscribableActor { - const mbFingerprint = await this.getEntity(id) - const {uid} = await this.ask("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id)); - return mbFingerprint.match( - fp => { - this.storeEntity({...fp, blocked: true}); - this.updateSubscribers({...fp, blocked: true}) - if (uid) - this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: false })); - return unit(); - }, - () => { - return new Error(`Unknown fingerprint id ${id}`); - } - ) + const fp = (await this.getEntity(id)).orUndefined(); + if (!fp) { + return new Error(`Unknown fingerprint id ${id}`); + } + const { uid } = await this.ask("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id)); + const updated = { ...fp, blocked: true }; + await this.storeEntity(updated); + this.updateSubscribers(updated); + if (uid) { + this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: false })); + } + return unit(); }, Unblock: async id => { - const mbFingerprint = await this.getEntity(id) - const {uid} = await this.ask("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id)); - return mbFingerprint.match( - fp => { - this.storeEntity({...fp, blocked: false}); - this.updateSubscribers({...fp, blocked: false}) - if (uid) - this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: true })); - return unit(); - }, - () => { - return new Error(`Unknown fingerprint id ${id}`); - } - ) + const fp = (await this.getEntity(id)).orUndefined(); + if (!fp) { + return new Error(`Unknown fingerprint id ${id}`); + } + const { uid } = await this.ask("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id)); + const updated = { ...fp, blocked: false }; + await this.storeEntity(updated); + this.updateSubscribers(updated); + if (uid) { + this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: true })); + } + return unit(); }, IncreaseCount: async ({fingerprint, userUid, initialQuiz}) => { - const mbFingerprint = await this.getEntity(fingerprint) - return mbFingerprint.match( - fp => { - this.storeEntity({...fp, usageCount: fp.usageCount + 1, lastSeen: toTimestamp(), userUid, initialQuiz: initialQuiz ?? fp.initialQuiz}); - this.updateSubscribers({...fp, usageCount: fp.usageCount + 1, lastSeen: toTimestamp(), userUid, initialQuiz: initialQuiz ?? fp.initialQuiz}) - return unit(); - }, - () => { - return new Error(`Unknown fingerprint id ${fingerprint}`); - } - ) + const fp = (await this.getEntity(fingerprint)).orUndefined(); + if (!fp) { + return new Error(`Unknown fingerprint id ${fingerprint}`); + } + const updated = { ...fp, usageCount: fp.usageCount + 1, lastSeen: toTimestamp(), userUid, initialQuiz: initialQuiz ?? fp.initialQuiz }; + await this.storeEntity(updated); + this.updateSubscribers(updated); + return unit(); }, GetMostRecent: async () => { const db = await this.connector.db(); diff --git a/packages/backend/src/actors/SessionStore.ts b/packages/backend/src/actors/SessionStore.ts index cdf6edb1..8eaebe99 100644 --- a/packages/backend/src/actors/SessionStore.ts +++ b/packages/backend/src/actors/SessionStore.ts @@ -47,16 +47,16 @@ export class SessionStore extends StoringActor { const result = await SessionStoreMessages.match>(message, { StoreSession: async session => { - this.state = await create(this.state, async draft => { - const currentSession = (await this.getEntity(session.uid)).orElse({} as Session); + const currentSession = (await this.getEntity(session.uid)).orElse({} as Session); + session.updated = toTimestamp(); + const newSession = { ...currentSession, ...session }; + this.state = create(this.state, draft => { if (session.actorSystem) { draft.clientIndex.set(session.actorSystem, session.uid); } - session.updated = toTimestamp(); - const newSession = { ...currentSession, ...session }; draft.cache.set(session.uid, newSession); - this.storeEntity(newSession); }); + await this.storeEntity(newSession); return unit(); }, CheckSession: async userId => { diff --git a/packages/backend/src/actors/StatisticsActor.ts b/packages/backend/src/actors/StatisticsActor.ts index deb8d1de..14f3ede5 100644 --- a/packages/backend/src/actors/StatisticsActor.ts +++ b/packages/backend/src/actors/StatisticsActor.ts @@ -243,7 +243,7 @@ export class StatisticsActor extends SubscribableActor< ); // console.log("STATS", stats); this.logger.info(`STATS stored quizId=${String(this.uid)}`); - this.storeEntity(stats); + await this.storeEntity(stats); for (const [subscriber] of this.state.collectionSubscribers) { this.send(subscriber, new StatisticsUpdateMessage(stats)); } diff --git a/packages/backend/src/actors/UserStore.ts b/packages/backend/src/actors/UserStore.ts index 82bc609e..478d9eca 100644 --- a/packages/backend/src/actors/UserStore.ts +++ b/packages/backend/src/actors/UserStore.ts @@ -353,26 +353,6 @@ export class UserStore extends SubscribableActor - ) - .then((session: Session) => { - session.role = newUser.role; - this.send(createActorUri("SessionStore"), SessionStoreMessages.StoreSession(session)); - }) - .catch((e: unknown) => { - this.logger.error( - `Failed to update session role for user ${String(newUser.uid)}: ` + - `${e instanceof Error ? e.stack : String(e)}` - ); - }); - } - for (const [subscriber, subscription] of this.state.collectionSubscribers) { this.send( subscriber, @@ -389,8 +369,30 @@ export class UserStore extends SubscribableActor newUser) .catch(error => error as Error); + if (!(storeResult instanceof Error) && oldUser.role !== newUser.role) { + try { + const sessionOrError = await this.ask( + createActorUri("SessionStore"), + SessionStoreMessages.GetSessionForUserId(newUser.uid) + ); + if (!(sessionOrError instanceof Error)) { + const session = sessionOrError as Session; + session.role = newUser.role; + await this.ask( + createActorUri("SessionStore"), + SessionStoreMessages.StoreSession(session) + ); + } + } catch (e: unknown) { + this.logger.error( + `Failed to update session role for user ${String(newUser.uid)}: ` + + `${e instanceof Error ? e.stack : String(e)}` + ); + } + } + return storeResult; }; } diff --git a/packages/backend/test/fingerprintStore.test.ts b/packages/backend/test/fingerprintStore.test.ts new file mode 100644 index 00000000..d3ff960f --- /dev/null +++ b/packages/backend/test/fingerprintStore.test.ts @@ -0,0 +1,98 @@ +import { Actor, ActorRef, ActorSystem } from "ts-actors"; +import { toTimestamp } from "itu-utils"; +import { vi } from "vitest"; +import { FingerprintStore } from "../src/actors/FingerprintStore"; +import { Fingerprint, FingerprintStoreMessages } from "@recapp/models"; +import { mockDbInstance } from "./setup/mongoMock"; + +// FingerprintStore.Block asks UserStore for the owning user; a minimal stub keeps the +// test free of the real UserStore while exercising the awaited block write. +class UserStoreStub extends Actor { + public constructor(name: string, system: ActorSystem) { + super(name, system); + } + public async receive(_from: ActorRef, _message: any): Promise { + return { uid: "user-1" }; + } +} + +// ts-actors surfaces a thrown handler error to the registered error receiver (it does not +// reject the ask). Capturing here lets us assert the awaited write's failure is surfaced. +const captured: unknown[] = []; +class ErrorReceiver extends Actor { + public constructor(name: string, system: ActorSystem) { + super(name, system); + } + public async receive(_from: ActorRef, message: unknown): Promise { + captured.push(message); + return undefined; + } +} + +async function waitFor(pred: () => boolean, timeoutMs = 2000): Promise { + const start = Date.now(); + while (!pred() && Date.now() - start < timeoutMs) { + await new Promise(r => setTimeout(r, 10)); + } +} + +function makeFingerprint(overrides: Partial = {}): Fingerprint { + const now = toTimestamp(); + return { + uid: "fp-1" as any, + created: now, + updated: now, + lastSeen: now, + usageCount: 1, + blocked: false, + userUid: "user-1" as any, + ...overrides, + }; +} + +// Characterises the await-and-call fix on the moderation store: block/unblock and store +// writes are awaited, so a blocked fingerprint is persisted before the ask resolves and +// cannot silently revert to unblocked after a restart (the note's headline risk). +describe("FingerprintStore — awaited persistence", () => { + let system: ActorSystem; + + beforeEach(async () => { + captured.length = 0; + system = await ActorSystem.create({ systemName: "recapp-backend" }); + }); + + afterEach(async () => { + await system.shutdown(); + vi.restoreAllMocks(); + }); + + test("StoreFingerprint commits the fingerprint to the DB before the ask resolves", async () => { + const ref = await system.createActor(FingerprintStore, { name: "FingerprintStore" }); + await system.ask(ref, FingerprintStoreMessages.StoreFingerprint(makeFingerprint())); + const doc = await mockDbInstance.collection("fingerprints").findOne({ uid: "fp-1" }); + expect(doc).not.toBeNull(); + expect((doc as any).blocked).toBe(false); + }); + + test("Block persists blocked:true (awaited) so a restart can't revert it", async () => { + await system.createActor(UserStoreStub, { name: "UserStore" }); + const ref = await system.createActor(FingerprintStore, { name: "FingerprintStore" }); + await system.ask(ref, FingerprintStoreMessages.StoreFingerprint(makeFingerprint())); + await system.ask(ref, FingerprintStoreMessages.Block("fp-1" as any)); + const doc = await mockDbInstance.collection("fingerprints").findOne({ uid: "fp-1" }); + expect((doc as any).blocked).toBe(true); + }); + + test("a failed fingerprint write surfaces to the error receiver instead of being swallowed", async () => { + await system.createActor(ErrorReceiver, { name: "ErrorActor", errorReceiver: true }); + const ref = await system.createActor(FingerprintStore, { name: "FingerprintStore" }); + vi.spyOn(mockDbInstance.collection("fingerprints"), "updateOne").mockRejectedValueOnce( + new Error("connection lost") + ); + // Fire without awaiting: on throw the ask never resolves, but the error is surfaced. + void system.ask(ref, FingerprintStoreMessages.StoreFingerprint(makeFingerprint())).catch(() => undefined); + await waitFor(() => captured.length > 0); + expect(captured.length).toBeGreaterThan(0); + expect(JSON.stringify(captured[0])).toContain("connection lost"); + }); +}); diff --git a/packages/backend/test/sessionStore.test.ts b/packages/backend/test/sessionStore.test.ts new file mode 100644 index 00000000..d7c13940 --- /dev/null +++ b/packages/backend/test/sessionStore.test.ts @@ -0,0 +1,83 @@ +import { Actor, ActorRef, ActorSystem } from "ts-actors"; +import { DateTime } from "luxon"; +import { toTimestamp } from "itu-utils"; +import { vi } from "vitest"; +import { SessionStore } from "../src/actors/SessionStore"; +import { Session, SessionStoreMessages } from "@recapp/models"; +import { mockDbInstance } from "./setup/mongoMock"; + +// ts-actors surfaces a thrown handler error to the registered error receiver (it does not +// reject the ask). Capturing here lets us assert the awaited write's failure is surfaced. +const captured: unknown[] = []; +class ErrorReceiver extends Actor { + public constructor(name: string, system: ActorSystem) { + super(name, system); + } + public async receive(_from: ActorRef, message: unknown): Promise { + captured.push(message); + return undefined; + } +} + +async function waitFor(pred: () => boolean, timeoutMs = 2000): Promise { + const start = Date.now(); + while (!pred() && Date.now() - start < timeoutMs) { + await new Promise(r => setTimeout(r, 10)); + } +} + +function makeSession(overrides: Partial = {}): Session { + const now = toTimestamp(); + const future = toTimestamp(DateTime.utc().plus({ hours: 1 })); + return { + idToken: "id", + accessToken: "access", + refreshToken: "refresh", + uid: "user-1" as any, + idExpires: future, + refreshExpires: future, + actorSystem: "client-system-1", + role: "STUDENT", + created: now, + updated: now, + ...overrides, + }; +} + +// Characterises the await-and-call fix: SessionStore.StoreSession awaits storeEntity, +// so the write is committed (and failures surfaced) before the ask resolves — previously +// it was fire-and-forget, so a failed write during a Mongo blip was silently lost. +describe("SessionStore — awaited persistence", () => { + let system: ActorSystem; + + beforeEach(async () => { + captured.length = 0; + system = await ActorSystem.create({ systemName: "recapp-backend" }); + }); + + afterEach(async () => { + await system.shutdown(); + vi.restoreAllMocks(); + }); + + test("StoreSession has committed the session to the DB by the time the ask resolves", async () => { + const ref = await system.createActor(SessionStore, { name: "SessionStore" }); + await system.ask(ref, SessionStoreMessages.StoreSession(makeSession())); + const doc = await mockDbInstance.collection("sessions").findOne({ uid: "user-1" }); + expect(doc).not.toBeNull(); + expect((doc as any).accessToken).toBe("access"); + }); + + test("a failed DB write surfaces to the error receiver instead of being swallowed", async () => { + await system.createActor(ErrorReceiver, { name: "ErrorActor", errorReceiver: true }); + const ref = await system.createActor(SessionStore, { name: "SessionStore" }); + vi.spyOn(mockDbInstance.collection("sessions"), "updateOne").mockRejectedValueOnce( + new Error("connection lost") + ); + // Fire without awaiting: on throw the ask never resolves, but the error is surfaced. + void system.ask(ref, SessionStoreMessages.StoreSession(makeSession())).catch(() => undefined); + await waitFor(() => captured.length > 0); + expect(captured.length).toBeGreaterThan(0); + expect(JSON.stringify(captured[0])).toContain("connection lost"); + }); +}); diff --git a/packages/frontend/src/actors/CurrentQuizActor.ts b/packages/frontend/src/actors/CurrentQuizActor.ts index 9af4520e..2b99cbdd 100644 --- a/packages/frontend/src/actors/CurrentQuizActor.ts +++ b/packages/frontend/src/actors/CurrentQuizActor.ts @@ -801,19 +801,28 @@ export class CurrentQuizActor extends StatefulActor { - // Question also needs to be deleted from the groups of the quiz - const groups = this.state.quiz.groups.map(g => { - g.questions = g.questions.filter(q => q !== id); - return g; - }); - await this.send( - actorUris.QuizActor, - QuizActorMessages.Update({ uid: this.state.quiz.uid, groups }) - ); - await this.ask( - `${actorUris.QuestionActorPrefix}${this.quiz.orElse(toId("-"))}`, - QuestionActorMessages.Delete(id) - ); + try { + // Remove the question from the quiz groups FIRST, then delete the + // question itself. The sequential await already aborts the delete if + // the groups update rejects, so a failed update can't leave dangling + // group references; the try/catch surfaces the rejection (matching + // AddQuestion) instead of dropping it. + const groups = this.state.quiz.groups.map(g => { + g.questions = g.questions.filter(q => q !== id); + return g; + }); + await this.ask( + actorUris.QuizActor, + QuizActorMessages.Update({ uid: this.state.quiz.uid, groups }) + ); + await this.ask( + `${actorUris.QuestionActorPrefix}${this.quiz.orElse(toId("-"))}`, + QuestionActorMessages.Delete(id) + ); + } catch (e) { + console.error("DeleteQuestion failed", e); + throw e; + } return unit(); }, AddQuestion: async ({ question, group }) => { diff --git a/packages/frontend/src/actors/SharingActor.ts b/packages/frontend/src/actors/SharingActor.ts index 66ef9bd2..94196251 100644 --- a/packages/frontend/src/actors/SharingActor.ts +++ b/packages/frontend/src/actors/SharingActor.ts @@ -76,6 +76,11 @@ export class SharingActor extends StatefulActor { + this.updateState(draft => { + draft.errors.push({ id: toId(v4()), queryNotFound: query }); + }); }); }, Clear: () => { diff --git a/packages/frontend/src/components/modals/ChangeNicknameModal.tsx b/packages/frontend/src/components/modals/ChangeNicknameModal.tsx index 3b526609..65ff6e2f 100644 --- a/packages/frontend/src/components/modals/ChangeNicknameModal.tsx +++ b/packages/frontend/src/components/modals/ChangeNicknameModal.tsx @@ -33,6 +33,7 @@ export const ChangeNicknameModal: React.FC = ({ show, defaultValue, onClo s .ask("actors://recapp-backend/UserStore", UserStoreMessages.IsNicknameUnique(newValue)) .then(result => !result && setError(i18n._("error-nickname-already-used"))) + .catch(() => { /* network error — skip uniqueness check */ }) ); } };