diff --git a/frontend/src/app.ts b/frontend/src/app.ts index 1dc4101df..c21ad0fd9 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -11,9 +11,8 @@ import './components/participant_view/participant_view'; import './components/settings/settings'; import {MobxLitElement} from '@adobe/lit-mobx'; -import {CSSResultGroup, html, nothing, TemplateResult} from 'lit'; +import {CSSResultGroup, html, nothing} from 'lit'; import {customElement} from 'lit/decorators.js'; -import {classMap} from 'lit/directives/class-map.js'; import {core} from './core/core'; import {AnalyticsService} from './services/analytics.service'; @@ -23,8 +22,6 @@ import {Pages, RouterService} from './services/router.service'; import {SettingsService} from './services/settings.service'; import {PresenceService} from './services/presence.service'; -import {ColorMode} from './shared/types'; - import {styles} from './app.scss'; /** App main component. */ @@ -32,6 +29,12 @@ import {styles} from './app.scss'; export class App extends MobxLitElement { static override styles: CSSResultGroup = [styles]; + // Unique per-tab connection ID for presence tracking + private readonly connectionId: string = + typeof crypto !== 'undefined' && crypto.randomUUID + ? crypto.randomUUID() + : Math.random().toString(36).slice(2) + Date.now(); + private readonly analyticsService = core.getService(AnalyticsService); private readonly authService = core.getService(AuthService); private readonly homeService = core.getService(HomeService); @@ -92,6 +95,7 @@ export class App extends MobxLitElement { this.presenceService.setupPresence( this.routerService.activeRoute.params['experiment'], this.routerService.activeRoute.params['participant'], + this.connectionId, ); return html` diff --git a/frontend/src/services/presence.service.ts b/frontend/src/services/presence.service.ts index 7284fecae..9e81b7f0f 100644 --- a/frontend/src/services/presence.service.ts +++ b/frontend/src/services/presence.service.ts @@ -22,8 +22,15 @@ export class PresenceService extends Service { makeObservable(this); } - setupPresence(experimentId: string, participantPrivateId: string) { - const statusRef = ref(this.sp.firebaseService.rtdb, `/status/${experimentId}/${participantPrivateId}`); + setupPresence( + experimentId: string, + participantPrivateId: string, + connectionId: string, + ) { + const statusRef = ref( + this.sp.firebaseService.rtdb, + `/status/${experimentId}/${participantPrivateId}/${connectionId}`, + ); const isOffline = { connected: false, @@ -35,17 +42,22 @@ export class PresenceService extends Service { last_changed: serverTimestamp(), }; - onValue(ref(this.sp.firebaseService.rtdb, '.info/connected'), (snapshot) => { - const isConnected = snapshot.val(); - if (!isConnected) { - return; - } - - // Set the user's status in rtdb. The callback will reset it to online - // if the user reconnects. - onDisconnect(statusRef).set(isOffline).then(() => { - set(statusRef, isOnline); - }); - }); + onValue( + ref(this.sp.firebaseService.rtdb, '.info/connected'), + (snapshot) => { + const isConnected = snapshot.val(); + if (!isConnected) { + return; + } + + // Set the user's status in rtdb. The callback will reset it to online + // if the user reconnects. + onDisconnect(statusRef) + .set(isOffline) + .then(() => { + set(statusRef, isOnline); + }); + }, + ); } } diff --git a/functions/src/participant.endpoints.ts b/functions/src/participant.endpoints.ts index 85b56c54d..f3fe334b2 100644 --- a/functions/src/participant.endpoints.ts +++ b/functions/src/participant.endpoints.ts @@ -53,9 +53,6 @@ export const createParticipant = onCall(async (request) => { prolificId: data.prolificId, }); - // Temporarily always mark participants as connected (PR #537) - participantConfig.connected = true; // TODO: Remove this line - // If agent config is specified, add to participant config if (data.agentConfig) { participantConfig.agentConfig = data.agentConfig; diff --git a/functions/src/presence.triggers.test.ts b/functions/src/presence.triggers.test.ts index 47fec3d4a..eab8312ee 100644 --- a/functions/src/presence.triggers.test.ts +++ b/functions/src/presence.triggers.test.ts @@ -3,9 +3,9 @@ import firebaseFunctionsTest from 'firebase-functions-test'; // Mock './app' before importing function under test jest.mock('./app', () => { const setMock = jest.fn(); - const getMock = jest.fn().mockResolvedValue({ exists: true }); - const docMock = jest.fn().mockReturnValue({ get: getMock, set: setMock }); - const firestoreMock = jest.fn().mockReturnValue({ doc: docMock }); + const getMock = jest.fn().mockResolvedValue({exists: true}); + const docMock = jest.fn().mockReturnValue({get: getMock, set: setMock}); + const firestoreMock = jest.fn().mockReturnValue({doc: docMock}); // Expose mocks via module-scoped object for assertions return { @@ -22,58 +22,87 @@ jest.mock('./app', () => { }; }); -import { mirrorPresenceToFirestore } from './presence.triggers'; -// @ts-ignore -import { __mocks__ } from './app'; +import {mirrorPresenceToFirestore} from './presence.triggers'; +// @ts-expect-error __mocks__ doesn't exist on the real app module, it's just for accessing mocks +import {__mocks__} from './app'; +import {DataSnapshot} from 'firebase-admin/database'; +import {Change} from 'firebase-functions/v1'; -const testEnv = firebaseFunctionsTest({ projectId: 'deliberate-lab-test' }); +const testEnv = firebaseFunctionsTest({projectId: 'deliberate-lab-test'}); describe('mirrorPresenceToFirestore', () => { afterEach(() => { jest.clearAllMocks(); }); - let wrapped: any; - let beforeSnap: any; - let afterSnap: any; - let change: any; + interface MockParent { + once: jest.Mock< + Promise<{ + val: () => Record; + }> + >; + child: jest.Mock<{set: jest.Mock}>; + } + + let beforeSnap: DataSnapshot; + let afterSnap: DataSnapshot; + let change: {before: typeof beforeSnap; after: typeof afterSnap}; beforeEach(() => { - wrapped = testEnv.wrap(mirrorPresenceToFirestore); + const parentVal = { + conn789: {connected: true, last_changed: 1234567890}, + conn456: {connected: false, last_changed: 1234560000}, + _aggregate: {state: 'offline', ts: 1234560000}, + }; + const mockParent: MockParent = { + once: jest.fn().mockResolvedValue({val: () => parentVal}), + child: jest.fn().mockReturnValue({set: jest.fn()}), + }; - beforeSnap = testEnv.database.makeDataSnapshot(null, '/status/exp123/user456'); - afterSnap = testEnv.database.makeDataSnapshot( - { connected: true, last_changed: 1234567890 }, - '/status/exp123/user456' - ); + // @ts-expect-error Type is not really compatible with DataSnapshot, but this is just a mock + afterSnap = { + ref: {parent: mockParent}, + val: () => ({connected: true, last_changed: 1234567890}), + } as DataSnapshot; - change = { before: beforeSnap, after: afterSnap }; + beforeSnap = afterSnap as DataSnapshot; // before is not actually used + + change = {before: beforeSnap, after: afterSnap} as Change; }); it('updates firestore with the connected status from rtdb', async () => { - const { getMock, docMock, setMock } = __mocks__; + const {getMock, docMock, setMock} = __mocks__; + const wrapped = testEnv.wrap(mirrorPresenceToFirestore); getMock.mockResolvedValueOnce({ exists: true, - data: jest.fn(() => ({ connected: true })), + data: jest.fn(() => ({connected: true})), }); + // @ts-expect-error mocks don't completely duplicate the real types await wrapped(change, { params: { experimentId: 'exp123', participantPrivateId: 'user456', + connectionId: 'conn789', }, }); - expect(docMock).toHaveBeenCalledWith('experiments/exp123/participants/user456'); - expect(setMock).toHaveBeenCalledWith({ connected: true }, { merge: true }); + expect(docMock).toHaveBeenCalledWith( + 'experiments/exp123/participants/user456', + ); + expect(setMock).toHaveBeenCalledWith( + expect.objectContaining({connected: true}), + {merge: true}, + ); }); it('does not mirror presence data for agent participants', async () => { - const { getMock, docMock, setMock } = __mocks__; + const {getMock, docMock, setMock} = __mocks__; + const wrapped = testEnv.wrap(mirrorPresenceToFirestore); const mockFirestoreProfile = { - agentConfig: { role: 'observer' }, + agentConfig: {role: 'observer'}, }; getMock.mockResolvedValueOnce({ @@ -81,14 +110,18 @@ describe('mirrorPresenceToFirestore', () => { data: jest.fn(() => mockFirestoreProfile), }); + // @ts-expect-error mocks don't completely duplicate the real types await wrapped(change, { params: { experimentId: 'exp123', participantPrivateId: 'user456', + connectionId: 'conn789', }, }); - expect(docMock).toHaveBeenCalledWith('experiments/exp123/participants/user456'); + expect(docMock).toHaveBeenCalledWith( + 'experiments/exp123/participants/user456', + ); expect(setMock).not.toHaveBeenCalled(); }); }); diff --git a/functions/src/presence.triggers.ts b/functions/src/presence.triggers.ts index b33ccb77a..c45c32e55 100644 --- a/functions/src/presence.triggers.ts +++ b/functions/src/presence.triggers.ts @@ -1,50 +1,99 @@ -import {database} from 'firebase-functions'; +import * as admin from 'firebase-admin'; + +import {database, pubsub} from 'firebase-functions'; import {app} from './app'; +const dbInstance = database.instance( + `${process.env.GCLOUD_PROJECT}-default-rtdb`, +); + /** - * Mirror the presence status from RTDB to Firestore. + * Mirror presence from RTDB → Firestore and maintain an aggregate node. Use + * separate connection IDs to track individual connections, to properly support + * multiple browser tabs or devices. * - * This function is triggered when the presence status of a participant changes in the RTDB. - * It updates the corresponding participant document in Firestore with the new status. + * RTDB write path: + * /status/{experimentId}/{participantPrivateId}/{connectionId} * - * Currently, rtdb is only used in Deliberate Lab for presence tracking (using the rtdb websocket). + * Firestore doc path: + * experiments/{experimentId}/participants/{participantPrivateId} */ -export const mirrorPresenceToFirestore = database - .instance(`${process.env.GCLOUD_PROJECT}-default-rtdb`) // other parts of firebase use the -default-rtdb suffix, so stay consistent - .ref('/status/{experimentId}/{participantPrivateId}') // rtdb path, not firestore path +export const mirrorPresenceToFirestore = dbInstance + .ref('/status/{experimentId}/{participantPrivateId}/{connectionId}') .onWrite(async (change, context) => { - const {experimentId, participantPrivateId} = context.params; - console.log( - `mirrorPresenceToFirestore triggered for experimentId=${experimentId} participantPrivateId=${participantPrivateId}`, - ); - const status = change.after.val(); + const {experimentId, participantPrivateId, connectionId} = context.params; - if (!status) return null; // status was deleted + if (connectionId.startsWith('_')) return null; - // Find the matching participant doc - const participantRef = app + const parentRef = change.after.ref.parent; // participantPrivateId + const aggRef = parentRef!.child('_aggregate'); + const fsRef = app .firestore() .doc(`experiments/${experimentId}/participants/${participantPrivateId}`); - const participantSnapshot = await participantRef.get(); - if (!participantSnapshot.exists) { + const siblingsSnapshot = await parentRef!.once('value'); + + let online = false; + for (const key in siblingsSnapshot.val()) { + if (key.startsWith('_')) { + // ignore _aggregate, future meta-nodes + continue; + } + + if (siblingsSnapshot.val()[key].connected) { + online = true; + break; + } + } + + await aggRef.set({ + state: online ? 'online' : 'offline', + ts: admin.database.ServerValue.TIMESTAMP, + }); + + const snapshot = await fsRef.get(); + if (!snapshot.exists) { console.warn( - `No participant found with id=${participantPrivateId} in experiment=${experimentId}`, + `No participant ${participantPrivateId} in experiment ${experimentId}`, ); return null; } - const participant = participantSnapshot.data(); - - if (participant && participant.agentConfig) { - return null; // Don't update presence for agent participants - } - - // Temporarily prevent participants from switching from connected - // to disconnected (PR #537) - // TODO: Remove this logic to resume actual presence detection - if (!status.connected) { + if (snapshot.data()?.agentConfig) { + // Skip bot/agent participants return null; } - return participantRef.set({connected: status.connected}, {merge: true}); + return fsRef.set( + { + connected: online, + last_changed: admin.firestore.FieldValue.serverTimestamp(), + }, + {merge: true}, + ); + }); + +export const scrubStalePresence = pubsub + .schedule('every 24 hours') + .onRun(async () => { + const cutoff = Date.now() - 72 * 60 * 60 * 1000; // 72 hours + const root = admin.app().database().ref('status'); + const usersSnapshot = await root.get(); + const userSnapshots: admin.database.DataSnapshot[] = []; + for (const userSnapshot of Object.values(usersSnapshot.val() || {})) { + userSnapshots.push(userSnapshot as admin.database.DataSnapshot); + } + for (const userSnapshot of userSnapshots) { + const connSnapshots: admin.database.DataSnapshot[] = []; + userSnapshot.forEach((connSnapshot) => { + connSnapshots.push(connSnapshot); + }); + for (const connSnapshot of connSnapshots) { + if ( + !connSnapshot.key!.startsWith('_') && + connSnapshot.child('ts').val() < cutoff + ) { + connSnapshot.ref.remove(); + } + } + } });