diff --git a/src/modules/emote-bar/application/GetMyEmoteBar.ts b/src/modules/emote-bar/application/GetMyEmoteBar.ts index 860dc6f..f53a994 100644 --- a/src/modules/emote-bar/application/GetMyEmoteBar.ts +++ b/src/modules/emote-bar/application/GetMyEmoteBar.ts @@ -3,9 +3,9 @@ import { EMOTE_PACK_MANIFEST } from "../../catalog/domain/EmotePack"; import { Cosmetic } from "../../catalog/domain/Cosmetic"; import { CosmeticRepository } from "../../catalog/domain/CosmeticRepository"; import { EntitlementsGatekeeper } from "../../entitlements/application/EntitlementsGatekeeper"; -import { EmoteBar } from "../domain/EmoteBar"; +import { EMOTE_BAR_SLOTS } from "../domain/EmoteBar"; import { EmoteBarRepository } from "../domain/EmoteBarRepository"; -import { DEFAULT_EMOTE_IDS } from "../domain/EmoteRef"; +import { DEFAULT_EMOTE_IDS, EmoteRef } from "../domain/EmoteRef"; import { activeEmotePacksBySlug, emoteFileName, packHasEmote } from "./activeEmotePacks"; export interface EmoteBarSlot { @@ -23,19 +23,30 @@ export interface EmoteBarView { slots: EmoteBarSlot[]; } -function defaultSlot(index: number): EmoteBarSlot { - return { - ref: DEFAULT_EMOTE_IDS[index], - packId: null, - assetUrl: null, - packUrl: null, - assetsExpiresAt: null, - }; +function defaultSlot(ref: string): EmoteBarSlot { + return { ref, packId: null, assetUrl: null, packUrl: null, assetsExpiresAt: null }; +} + +function parseRef(raw: string): EmoteRef | null { + try { + return EmoteRef.parse(raw); + } catch { + return null; + } +} + +/** Each stale slot takes the first default emote that no other slot of the bar shows. */ +function fillStaleSlots(resolved: readonly (EmoteBarSlot | null)[]): EmoteBarSlot[] { + const used = new Set(resolved.flatMap((slot) => (slot ? [slot.ref] : []))); + const spare = DEFAULT_EMOTE_IDS.filter((id) => !used.has(id)); + return resolved.map((slot) => slot ?? defaultSlot(spare.shift() as string)); } /** - * Resolves any user's bar against the catalog and their current entitlements. A saved slot - * that is no longer usable reads as the default emote of that slot; storage is not rewritten. + * Resolves any user's bar against the catalog and their current entitlements. Stored refs are + * read leniently: a slot that is malformed, repeated, or no longer usable reads as the first + * default emote the rest of the bar does not show, so the result always has distinct emotes; + * storage is not rewritten. A stored bar of the wrong length reads as the default bar. */ export class GetMyEmoteBar { constructor( @@ -46,14 +57,22 @@ export class GetMyEmoteBar { ) {} async run(userId: string): Promise { - const bar = (await this.bars.findByUserId(userId)) ?? EmoteBar.defaults(); - const refs = bar.refs(); - if (refs.every((ref) => ref.isDefault)) { - return { slots: refs.map((ref, index) => ({ ...defaultSlot(index), ref: ref.toString() })) }; - } + const stored = await this.bars.findByUserId(userId); + const raws = stored?.length === EMOTE_BAR_SLOTS ? stored : DEFAULT_EMOTE_IDS; + + const seen = new Set(); + const refs = raws.map((raw) => { + const ref = parseRef(raw); + if (!ref || seen.has(raw)) return null; + seen.add(raw); + return ref; + }); - const packs = activeEmotePacksBySlug(await this.cosmetics.findAll()); - const access = await this.gatekeeper.accessFor(userId); + const needsPacks = refs.some((ref) => ref && !ref.isDefault); + const packs = needsPacks + ? activeEmotePacksBySlug(await this.cosmetics.findAll()) + : new Map(); + const access = needsPacks ? await this.gatekeeper.accessFor(userId) : null; const manifests = new Map>(); const manifestOf = (pack: Cosmetic) => { let manifest = manifests.get(pack.id); @@ -64,17 +83,18 @@ export class GetMyEmoteBar { return manifest; }; - const slots = await Promise.all( - refs.map(async (ref, index): Promise => { - if (ref.isDefault) return { ...defaultSlot(index), ref: ref.toString() }; + const resolved = await Promise.all( + refs.map(async (ref): Promise => { + if (!ref) return null; + if (ref.isDefault) return defaultSlot(ref.toString()); const pack = packs.get(ref.packSlug as string); const key = ref.emoteKey as string; - if (!pack || !access.canUse(pack) || !packHasEmote(pack, key)) return defaultSlot(index); + if (!pack || !access?.canUse(pack) || !packHasEmote(pack, key)) return null; const manifest = await manifestOf(pack); const assetUrl = manifest.assets[emoteFileName(key)]; - if (!assetUrl) return defaultSlot(index); + if (!assetUrl) return null; return { ref: ref.toString(), packId: pack.id, @@ -84,6 +104,6 @@ export class GetMyEmoteBar { }; }), ); - return { slots }; + return { slots: fillStaleSlots(resolved) }; } } diff --git a/src/modules/emote-bar/application/activeEmotePacks.ts b/src/modules/emote-bar/application/activeEmotePacks.ts index 02269af..1876173 100644 --- a/src/modules/emote-bar/application/activeEmotePacks.ts +++ b/src/modules/emote-bar/application/activeEmotePacks.ts @@ -4,13 +4,20 @@ import { EMOTE_PACK_MANIFEST } from "../../catalog/domain/EmotePack"; const PACK_PREFIX = "emote-packs/"; +function slugOf(assetRef: string): string | null { + const rest = assetRef.slice(PACK_PREFIX.length); + const slug = rest.endsWith("/") ? rest.slice(0, -1) : rest; + return slug === "" || slug.includes("/") ? null : slug; +} + /** Active EMOTE_PACK cosmetics keyed by the slug in their `emote-packs//` folder. */ export function activeEmotePacksBySlug(cosmetics: readonly Cosmetic[]): Map { const bySlug = new Map(); for (const cosmetic of cosmetics) { if (cosmetic.type !== CosmeticType.EMOTE_PACK || !cosmetic.active) continue; if (!cosmetic.assetRef.startsWith(PACK_PREFIX)) continue; - bySlug.set(cosmetic.assetRef.slice(PACK_PREFIX.length, -1), cosmetic); + const slug = slugOf(cosmetic.assetRef); + if (slug) bySlug.set(slug, cosmetic); } return bySlug; } diff --git a/src/modules/emote-bar/domain/EmoteBarRepository.ts b/src/modules/emote-bar/domain/EmoteBarRepository.ts index baae6df..14541cb 100644 --- a/src/modules/emote-bar/domain/EmoteBarRepository.ts +++ b/src/modules/emote-bar/domain/EmoteBarRepository.ts @@ -1,8 +1,12 @@ import { EmoteBar } from "./EmoteBar"; export interface EmoteBarRepository { - /** The saved bar, or null when the user never saved one. */ - findByUserId(userId: string): Promise; + /** + * The stored refs exactly as saved, or null when the user never saved a bar or the stored + * array does not hold one ref per slot. Stored refs are not validated: they may predate + * the current rules, so the reader decides what each one means. + */ + findByUserId(userId: string): Promise; /** Replaces the whole bar atomically. */ save(userId: string, bar: EmoteBar): Promise; diff --git a/src/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.ts b/src/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.ts index 4065e3b..fc1cb2a 100644 --- a/src/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.ts +++ b/src/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.ts @@ -1,18 +1,23 @@ import { cosmeticsDataSource } from "../../../cosmetics-data-source"; -import { EmoteBar } from "../domain/EmoteBar"; +import { EMOTE_BAR_SLOTS, EmoteBar } from "../domain/EmoteBar"; import { EmoteBarRepository } from "../domain/EmoteBarRepository"; import { UserEmoteBarEntity } from "./UserEmoteBarEntity"; export class EmoteBarPostgresRepository implements EmoteBarRepository { - async findByUserId(userId: string): Promise { + async findByUserId(userId: string): Promise { const row = await cosmeticsDataSource.getRepository(UserEmoteBarEntity).findOneBy({ userId }); - return row ? EmoteBar.create(row.emoteRefs) : null; + return row?.emoteRefs?.length === EMOTE_BAR_SLOTS ? row.emoteRefs : null; } async save(userId: string, bar: EmoteBar): Promise { - // Upsert by primary key: the whole bar is replaced in a single statement. + // A single INSERT ... ON CONFLICT (user_id) DO UPDATE replaces the whole bar, so + // concurrent first saves for one user cannot collide on the primary key. await cosmeticsDataSource - .getRepository(UserEmoteBarEntity) - .save({ userId, emoteRefs: bar.refs().map(String) }); + .createQueryBuilder() + .insert() + .into(UserEmoteBarEntity) + .values({ userId, emoteRefs: bar.refs().map(String) }) + .orUpdate(["emote_refs", "updated_at"], ["user_id"]) + .execute(); } } diff --git a/tests/unit/modules/emote-bar/application/GetMyEmoteBar.test.ts b/tests/unit/modules/emote-bar/application/GetMyEmoteBar.test.ts index 3ce1553..6e5d46b 100644 --- a/tests/unit/modules/emote-bar/application/GetMyEmoteBar.test.ts +++ b/tests/unit/modules/emote-bar/application/GetMyEmoteBar.test.ts @@ -75,7 +75,109 @@ describe("GetMyEmoteBar", () => { const result = await build({ bar }).run("user-1"); expect(result.slots[0].ref).toBe("halloween/ghost"); - expect(result.slots[1]).toEqual({ ref: "thumbsup", ...DEFAULT_SLOT }); + expect(result.slots[1]).toEqual({ ref: "wave", ...DEFAULT_SLOT }); + }); + + it("never repeats a default when a stale slot falls back", async () => { + const bar = barWith([0, "fire"], [2, "halloween/ghost"]); + + const result = await build({ bar, cosmetics: [] }).run("user-1"); + + const refs = result.slots.map((slot) => slot.ref); + expect(refs).toEqual([ + "fire", + "thumbsup", + "wave", + "thinking", + "cool", + "mindblown", + "sweat", + "cry", + ]); + expect(new Set(refs).size).toBe(8); + }); + + it("gives each of several stale slots a different unused default", async () => { + const bar = barWith( + [0, "halloween/ghost"], + [1, "halloween/bat"], + [4, "gone/emote"], + [7, "gone/other"], + ); + + const result = await build({ bar, cosmetics: [] }).run("user-1"); + + const refs = result.slots.map((slot) => slot.ref); + expect(refs).toEqual([ + "wave", + "thumbsup", + "fire", + "thinking", + "cool", + "mindblown", + "sweat", + "cry", + ]); + expect(new Set(refs).size).toBe(8); + }); + + it("reads stored refs that are no longer valid as stale slots", async () => { + const stored = [ + "removed-default", + "Not/A/Ref", + "fire", + "halloween/ghost", + "cool", + "mindblown", + "sweat", + "cry", + ]; + const useCase = new GetMyEmoteBar( + cosmeticsOf(pack()), + barsOf(stored).repository, + gatekeeperOf(grant(PACK_ID)), + signer, + ); + + const result = await useCase.run("user-1"); + + expect(result.slots.map((slot) => slot.ref)).toEqual([ + "wave", + "thumbsup", + "fire", + "halloween/ghost", + "cool", + "mindblown", + "sweat", + "cry", + ]); + }); + + it("reads a stored array of the wrong length as the default bar", async () => { + const useCase = new GetMyEmoteBar( + cosmeticsOf(pack()), + barsOf(["wave", "fire"]).repository, + gatekeeperOf(), + signer, + ); + + const result = await useCase.run("user-1"); + + expect(result.slots.map((slot) => slot.ref)).toEqual([...DEFAULT_EMOTE_IDS]); + }); + + it("reads repeated stored refs as one slot plus fallbacks", async () => { + const stored = ["wave", "wave", ...DEFAULT_EMOTE_IDS.slice(2)]; + const useCase = new GetMyEmoteBar( + cosmeticsOf(pack()), + barsOf(stored).repository, + gatekeeperOf(), + signer, + ); + + const result = await useCase.run("user-1"); + + expect(result.slots.map((slot) => slot.ref)).toEqual([...DEFAULT_EMOTE_IDS]); }); it("resolves the bar for whichever user id it is asked about", async () => { diff --git a/tests/unit/modules/emote-bar/application/activeEmotePacks.test.ts b/tests/unit/modules/emote-bar/application/activeEmotePacks.test.ts new file mode 100644 index 0000000..5565ab5 --- /dev/null +++ b/tests/unit/modules/emote-bar/application/activeEmotePacks.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "bun:test"; + +import { activeEmotePacksBySlug } from "../../../../../src/modules/emote-bar/application/activeEmotePacks"; +import { pack } from "./fixtures"; + +describe("activeEmotePacksBySlug", () => { + it("keys a pack by the slug in its folder, with or without a trailing slash", () => { + const withSlash = pack({ id: "a", assetRef: "emote-packs/halloween/" }); + const withoutSlash = pack({ id: "b", assetRef: "emote-packs/winter" }); + + const bySlug = activeEmotePacksBySlug([withSlash, withoutSlash]); + + expect(bySlug.get("halloween")).toBe(withSlash); + expect(bySlug.get("winter")).toBe(withoutSlash); + }); + + it.each([ + "emote-packs/", + "emote-packs//", + "emote-packs/a/b/", + "emote-packs/a/b", + "playmats/x/", + ])("ignores the asset ref %s", (assetRef) => { + const bySlug = activeEmotePacksBySlug([pack({ assetRef })]); + + expect(bySlug.size).toBe(0); + }); + + it("ignores inactive packs", () => { + expect(activeEmotePacksBySlug([pack({ active: false })]).size).toBe(0); + }); +}); diff --git a/tests/unit/modules/emote-bar/application/fixtures.ts b/tests/unit/modules/emote-bar/application/fixtures.ts index 017e5cf..123ad5f 100644 --- a/tests/unit/modules/emote-bar/application/fixtures.ts +++ b/tests/unit/modules/emote-bar/application/fixtures.ts @@ -51,10 +51,11 @@ export function gatekeeperOf(...entitlements: Entitlement[]): EntitlementsGateke }); } -export function barsOf(initial: EmoteBar | null) { +export function barsOf(initial: EmoteBar | readonly string[] | null) { + const stored = initial instanceof EmoteBar ? initial.refs().map(String) : initial; const saved: Array<{ userId: string; bar: EmoteBar }> = []; const repository: EmoteBarRepository = { - findByUserId: async () => initial, + findByUserId: async () => stored, save: async (userId, bar) => { saved.push({ userId, bar }); }, diff --git a/tests/unit/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.test.ts b/tests/unit/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.test.ts new file mode 100644 index 0000000..e11e57d --- /dev/null +++ b/tests/unit/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it, mock, spyOn } from "bun:test"; + +import { cosmeticsDataSource } from "../../../../../src/cosmetics-data-source"; +import { EmoteBar } from "../../../../../src/modules/emote-bar/domain/EmoteBar"; +import { DEFAULT_EMOTE_IDS } from "../../../../../src/modules/emote-bar/domain/EmoteRef"; +import { EmoteBarPostgresRepository } from "../../../../../src/modules/emote-bar/infrastructure/EmoteBarPostgresRepository"; +import { UserEmoteBarEntity } from "../../../../../src/modules/emote-bar/infrastructure/UserEmoteBarEntity"; + +function stubUpsertChain() { + const chain: Record> = {}; + for (const method of ["insert", "into", "values", "orUpdate"]) { + chain[method] = mock(() => chain); + } + chain.execute = mock(async () => ({})); + const spy = spyOn(cosmeticsDataSource, "createQueryBuilder").mockReturnValue(chain as never); + return { chain, spy }; +} + +describe("EmoteBarPostgresRepository", () => { + it("saves the bar with one INSERT ... ON CONFLICT (user_id) DO UPDATE statement", async () => { + const { chain, spy } = stubUpsertChain(); + const repository = spyOn(cosmeticsDataSource, "getRepository").mockImplementation(() => { + throw new Error("save must not go through a SELECT-then-write repository"); + }); + + await new EmoteBarPostgresRepository().save("user-1", EmoteBar.defaults()); + + expect(chain.into).toHaveBeenCalledWith(UserEmoteBarEntity); + expect(chain.values).toHaveBeenCalledWith({ + userId: "user-1", + emoteRefs: [...DEFAULT_EMOTE_IDS], + }); + expect(chain.orUpdate).toHaveBeenCalledWith(["emote_refs", "updated_at"], ["user_id"]); + expect(chain.execute).toHaveBeenCalledTimes(1); + + spy.mockRestore(); + repository.mockRestore(); + }); + + it("returns null when the user never saved a bar", async () => { + const spy = spyOn(cosmeticsDataSource, "getRepository").mockReturnValue({ + findOneBy: mock(async () => null), + } as never); + + expect(await new EmoteBarPostgresRepository().findByUserId("user-1")).toBeNull(); + + spy.mockRestore(); + }); + + it("returns the stored refs as they are, even when they no longer validate", async () => { + const emoteRefs = ["removed-default", "Not/A/Ref", "wave", "wave", "a", "b", "c", "d"]; + const spy = spyOn(cosmeticsDataSource, "getRepository").mockReturnValue({ + findOneBy: mock(async () => ({ userId: "user-1", emoteRefs })), + } as never); + + expect(await new EmoteBarPostgresRepository().findByUserId("user-1")).toEqual(emoteRefs); + + spy.mockRestore(); + }); + + it("reads a stored array of the wrong length as no saved bar", async () => { + const spy = spyOn(cosmeticsDataSource, "getRepository").mockReturnValue({ + findOneBy: mock(async () => ({ userId: "user-1", emoteRefs: ["wave"] })), + } as never); + + expect(await new EmoteBarPostgresRepository().findByUserId("user-1")).toBeNull(); + + spy.mockRestore(); + }); +}); diff --git a/tests/unit/server/routes/emote-bar-router.test.ts b/tests/unit/server/routes/emote-bar-router.test.ts index 13388c5..2852179 100644 --- a/tests/unit/server/routes/emote-bar-router.test.ts +++ b/tests/unit/server/routes/emote-bar-router.test.ts @@ -37,7 +37,7 @@ const pack = Cosmetic.from({ assetFiles: ["pack.json", "ghost.json"], }); -let stored = null as EmoteBar | null; +let stored: string[] | null = null; const jwt = new JWT(config.jwt); const app = new Elysia() .onError(({ error, set }) => { @@ -71,7 +71,7 @@ const spies = [ async () => stored, ), spyOn(EmoteBarPostgresRepository.prototype, "save").mockImplementation(async (_id, bar) => { - stored = bar; + stored = bar.refs().map(String); }), ]; @@ -92,17 +92,20 @@ function call(method: string, token?: string, body?: unknown) { ); } +const bodyFor = (method: string) => + method === "PUT" ? { slots: [...DEFAULT_EMOTE_IDS] } : undefined; + const withPackEmote = (): string[] => ["halloween/ghost", ...DEFAULT_EMOTE_IDS.slice(1)]; describe("/me/emote-bar", () => { for (const method of ["GET", "PUT"]) { it(`answers 401 to ${method} without a token`, async () => { - expect((await call(method, undefined, { slots: [...DEFAULT_EMOTE_IDS] })).status).toBe(401); + expect((await call(method, undefined, bodyFor(method))).status).toBe(401); }); it(`answers 403 to ${method} when the user is banned`, async () => { const token = jwt.generate({ id: bannedUserId }); - expect((await call(method, token, { slots: [...DEFAULT_EMOTE_IDS] })).status).toBe(403); + expect((await call(method, token, bodyFor(method))).status).toBe(403); }); } @@ -125,7 +128,7 @@ describe("/me/emote-bar", () => { expect(response.status).toBe(200); const body = (await response.json()) as { slots: { ref: string; packId: string | null }[] }; expect(body.slots[0]).toMatchObject({ ref: "halloween/ghost", packId: "pack-1" }); - expect((stored as EmoteBar | null)?.refs().map(String)).toEqual(withPackEmote()); + expect(stored as string[] | null).toEqual(withPackEmote()); }); it("answers 403 when saving a pack the user does not own", async () => {