diff --git a/app/server/src/db/sessionStore.ts b/app/server/src/db/sessionStore.ts index 0e1eb325f..19473ec29 100644 --- a/app/server/src/db/sessionStore.ts +++ b/app/server/src/db/sessionStore.ts @@ -58,22 +58,23 @@ export class SessionStore { this._redis.hmget(this.sessionKey("label"), ...ids), this._redis.hmget(this.sessionKey("friendly"), ...ids), this._redis.hmget(this.sessionKey("hash"), ...ids) - ]).then((values) => { + ]).then(async (values) => { const times = values[0]; const labels = values[1]; const friendlies = values[2]; const hashes = values[3]; - // We return all labelled sessions, and also the latest session for any hash (labelled or unlabelled) - const labelledSessions: SessionMetadata[] = []; - const latestByHash: Record = {}; - const buildSessionMetadata = (id: string, idx: number) => ({ id, time: times[idx], label: labels[idx], friendlyId: friendlies[idx] }); + let allResults; if (removeDuplicates) { - ids.forEach(async (id: string, idx: number) => { + // We return all labelled sessions, and also the latest session for any hash (labelled or unlabelled) + const labelledSessions: SessionMetadata[] = []; + const latestByHash: Record = {}; + + for (const [idx, id] of ids.entries()) { if (times[idx] !== null) { const session = buildSessionMetadata(id, idx) as SessionMetadata; @@ -86,23 +87,24 @@ export class SessionStore { if (hash === null) { const data = await this.getSession(id); hash = this.hashForData(data!); - this._redis.hset(this.sessionKey("hash"), id, hash); + await this._redis.hset(this.sessionKey("hash"), id, hash); } if (!latestByHash[hash] || session.time > latestByHash[hash].time) { latestByHash[hash] = session; } } - }); - return [ + } + allResults = [ ...labelledSessions, ...Object.values(latestByHash).filter((s) => !s.label) // don't include labelled sessions twice - ].sort((a, b) => a.time > b.time ? 1 : -1); + ]; + } else { + // Return all sessions, including duplicates + allResults = ids.map((id: string, idx: number) => buildSessionMetadata(id, idx)) + .filter((session) => session.time !== null); } - - // Return all sessions, including duplicates - return ids.map((id: string, idx: number) => buildSessionMetadata(id, idx)) - .filter((session) => session.time !== null); + return allResults.sort((a, b) => a.time!! < b.time!! ? 1 : -1); }); } diff --git a/app/server/tests/db/sessionStore.test.ts b/app/server/tests/db/sessionStore.test.ts index 6719985b7..aaecbd7b4 100644 --- a/app/server/tests/db/sessionStore.test.ts +++ b/app/server/tests/db/sessionStore.test.ts @@ -201,6 +201,128 @@ describe("generate friendly id", () => { }); }); +describe("SessionStore handles duplicate sessions", () => { + const mockRedis = (savedHmGetValues: Record, savedHGetValues: Record = {}) => { + return { + hmget: jest.fn().mockImplementation(async (sessionKey: string) => { + const key = sessionKey.split(":").slice(-1)[0]; + return savedHmGetValues[key]; + }), + hget: jest.fn().mockImplementation(async (sessionKey: string, sessionId: string) => { + return savedHGetValues[sessionId]; + }), + hset: jest.fn() + } as any; + }; + + it("Filters out earlier duplicate sessions if removeDuplicates is true", async () => { + const savedValues = { + time: ["2023-10-25 11:10:01", "2023-10-25 11:10:02", "2023-10-25 11:10:03", "2023-10-25 11:10:04", "2023-10-25 11:10:05"], + label: [null, null, null, null, null], + friendly: ["good dog", "bad cat", "devious chaffinch", "happy bat", "sad owl"], + hash: ["123", "234", "567", "234", "123"] + }; + const redis = mockRedis(savedValues); + const sut = new SessionStore(redis, "Test Course", "testApp"); + const ids = ["abc", "def", "ghi", "jkl", "mno"]; + const result = await sut.getSessionsMetadata(ids, true); + expect(redis.hmget).toHaveBeenCalledTimes(4); + const sessionKeyPrefix = "Test Course:testApp:sessions:"; + expect(redis.hmget).toHaveBeenNthCalledWith(1, `${sessionKeyPrefix}time`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(2, `${sessionKeyPrefix}label`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(3, `${sessionKeyPrefix}friendly`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(4, `${sessionKeyPrefix}hash`, ...ids); + + // results should be ordered chrono desc + expect(result).toStrictEqual([ + {id: "mno", time: "2023-10-25 11:10:05", label: null, friendlyId: "sad owl"}, + {id: "jkl", time: "2023-10-25 11:10:04", label: null, friendlyId: "happy bat"}, + {id: "ghi", time: "2023-10-25 11:10:03", label: null, friendlyId: "devious chaffinch"} + ]); + }); + + it("Does not filter out earlier duplicate sessions if removeDuplicates is false", async () => { + const savedValues = { + time: ["2023-10-25 11:10:01", "2023-10-25 11:10:02", "2023-10-25 11:10:03", "2023-10-25 11:10:04", "2023-10-25 11:10:05"], + label: [null, null, null, null, null], + friendly: ["good dog", "bad cat", "devious chaffinch", "happy bat", "sad owl"], + hash: ["123", "234", "567", "234", "123"] + }; + const redis = mockRedis(savedValues); + const sut = new SessionStore(redis, "Test Course", "testApp"); + const ids = ["abc", "def", "ghi", "jkl", "mno"]; + const result = await sut.getSessionsMetadata(ids, false); + expect(redis.hmget).toHaveBeenCalledTimes(4); + const sessionKeyPrefix = "Test Course:testApp:sessions:"; + expect(redis.hmget).toHaveBeenNthCalledWith(1, `${sessionKeyPrefix}time`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(2, `${sessionKeyPrefix}label`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(3, `${sessionKeyPrefix}friendly`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(4, `${sessionKeyPrefix}hash`, ...ids); + + expect(result).toStrictEqual([ + {id: "mno", time: "2023-10-25 11:10:05", label: null, friendlyId: "sad owl"}, + {id: "jkl", time: "2023-10-25 11:10:04", label: null, friendlyId: "happy bat"}, + {id: "ghi", time: "2023-10-25 11:10:03", label: null, friendlyId: "devious chaffinch"}, + {id: "def", time: "2023-10-25 11:10:02", label: null, friendlyId: "bad cat"}, + {id: "abc", time: "2023-10-25 11:10:01", label: null, friendlyId: "good dog"} + ]); + }); + + it("returns duplicates if they have labels", async () => { + const savedValues = { + time: ["2023-10-25 11:10:01", "2023-10-25 11:10:02", "2023-10-25 11:10:03", "2023-10-25 11:10:04", "2023-10-25 11:10:05"], + label: ["oldest session", null, null, "newer session", null], + friendly: ["good dog", "bad cat", "devious chaffinch", "happy bat", "sad owl"], + hash: ["123", "234", "567", "234", "123"] + }; + const redis = mockRedis(savedValues); + const sut = new SessionStore(redis, "Test Course", "testApp"); + const ids = ["abc", "def", "ghi", "jkl", "mno"]; + const result = await sut.getSessionsMetadata(ids, true); + + expect(result).toStrictEqual([ + {id: "mno", time: "2023-10-25 11:10:05", label: null, friendlyId: "sad owl"}, + {id: "jkl", time: "2023-10-25 11:10:04", label: "newer session", friendlyId: "happy bat"}, + {id: "ghi", time: "2023-10-25 11:10:03", label: null, friendlyId: "devious chaffinch"}, + {id: "abc", time: "2023-10-25 11:10:01", label: "oldest session", friendlyId: "good dog"} + ]); + }); + + it("if a session has no hash, calculates and saves hash, and applies filter rules", async () => { + // 2 sessions without hashes, one is a newer and one an older duplicate of another sessions + const sessionContent1 = "test content 1"; + const sessionContent2 = "test content 2"; + + const sessionContent1Hash = md5(sessionContent1); + const sessionContent2Hash = md5(sessionContent2); + + const savedValues = { + time: ["2023-10-25 11:10:01", "2023-10-25 11:10:02", "2023-10-25 11:10:03", "2023-10-25 11:10:04", "2023-10-25 11:10:05"], + label: [null, null, null, null, null], + friendly: ["good dog", "bad cat", "devious chaffinch", "happy bat", "sad owl"], + hash: [sessionContent1Hash, null, "567", sessionContent2Hash, null] + }; + + const redis = mockRedis(savedValues, {def: sessionContent2, mno: sessionContent1}); + const sut = new SessionStore(redis, "Test Course", "testApp"); + const ids = ["abc", "def", "ghi", "jkl", "mno"]; + const result = await sut.getSessionsMetadata(ids, true); + expect(redis.hmget).toHaveBeenCalledTimes(4); + const sessionKeyPrefix = "Test Course:testApp:sessions:"; + expect(redis.hmget).toHaveBeenNthCalledWith(1, `${sessionKeyPrefix}time`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(2, `${sessionKeyPrefix}label`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(3, `${sessionKeyPrefix}friendly`, ...ids); + expect(redis.hmget).toHaveBeenNthCalledWith(4, `${sessionKeyPrefix}hash`, ...ids); + + // results should be ordered chrono desc + expect(result).toStrictEqual([ + {id: "mno", time: "2023-10-25 11:10:05", label: null, friendlyId: "sad owl"}, + {id: "jkl", time: "2023-10-25 11:10:04", label: null, friendlyId: "happy bat"}, + {id: "ghi", time: "2023-10-25 11:10:03", label: null, friendlyId: "devious chaffinch"} + ]); + }); +}); + describe("getSessionStore", () => { it("gets session store using request", () => { const mockRequest = {