diff --git a/api/db/migrations/20260730140000_create_member_data.ts b/api/db/migrations/20260730140000_create_member_data.ts new file mode 100644 index 00000000..f3ed8a3d --- /dev/null +++ b/api/db/migrations/20260730140000_create_member_data.ts @@ -0,0 +1,60 @@ +import { Knex } from 'knex'; + +/** + * Per-member named attributes -- the substrate the social layer hangs off. + * + * In CS 4.x this is the MD / memdata table, keyed by member id, and it is where the + * buddy list actually lives: ten slots BU0..BU9 holding NICKNAMES, settled by writing a + * buddy on a live 4.1 server and diffing the data files. Buddies are NOT a join table and + * NOT in groups/groupmem -- those back the Group entries in the access-rights model + * instead. sqserver.sql names this table Member_Data. + * + * A generic key/value store rather than a column per feature, because that is what the + * original is: buddies (BU0..BU9), the hide-yourself privacy flag (IMS) and the per-place + * defaults all live here as named attributes. Adding a feature should not need a + * migration. + * + * Deliberately NOT normalised into a friend table. A ten-slot nickname-keyed list is the + * fidelity target, and "improving" it into (member_id, friend_member_id) rows would lose + * two behaviours the original has: a buddy slot can name someone who does not exist (or + * who later renames), and the slot INDEX is meaningful and stable. + * + * `value` is text, not json: MySQL 5.7 is the deployment target, its JSON support is + * weaker than 8.0's, and every value the original stores here is a short scalar anyway. + */ + +const COLLATE = 'utf8mb4_unicode_ci'; +const tableName = 'member_data'; + +export async function up(knex: Knex): Promise { + if (await knex.schema.hasTable(tableName)) return; + + console.log(`Creating ${tableName} table`); + await knex.schema.createTable(tableName, table => { + table.collate(COLLATE); + table.increments('id').primary(); + table.timestamps(false, true); + + table.integer('member_id').unsigned().notNullable(); + table.foreign('member_id').references('member.id'); + + // Attribute name, e.g. BU0..BU9 for buddy slots, IMS for the privacy flag. + table.string('name', 32).notNullable(); + table.text('value'); + + // One row per (member, attribute). Writes are upserts against this. + // + // This is also the read index: a UNIQUE constraint IS a btree index in MySQL, and + // because member_id leads it already serves "all attributes for this member" and a + // prefix scan of one family (BU%). A separate index(['member_id', 'name']) alongside it + // would be the same index twice -- extra storage and extra write cost on every upsert, + // for no additional query the optimiser could not already satisfy. + table.unique(['member_id', 'name']); + }); +} + +export async function down(knex: Knex): Promise { + if (!await knex.schema.hasTable(tableName)) return; + console.log(`Dropping ${tableName} table`); + await knex.schema.dropTable(tableName); +} diff --git a/api/src/db/db.class.ts b/api/src/db/db.class.ts index 42a8104e..5190462d 100644 --- a/api/src/db/db.class.ts +++ b/api/src/db/db.class.ts @@ -31,6 +31,9 @@ export class Db { get member() { return this.knex('member'); } + get memberData() { + return this.knex('member_data'); + } get message() { return this.knex('message'); } diff --git a/api/src/repositories/index.ts b/api/src/repositories/index.ts index 7936f133..67a1cd4f 100644 --- a/api/src/repositories/index.ts +++ b/api/src/repositories/index.ts @@ -9,6 +9,7 @@ export * from './hood/hood.repository'; export * from './mall-object/mall-object.repository'; export * from './map-location/map-location.repository'; export * from './member/member.repository'; +export * from './member-data/member-data.repository'; export * from './message/message.repository'; export * from './object/object.repository'; export * from './object-instance/object-instance.repository'; diff --git a/api/src/repositories/member-data/member-data.repository.spec.ts b/api/src/repositories/member-data/member-data.repository.spec.ts new file mode 100644 index 00000000..15deada7 --- /dev/null +++ b/api/src/repositories/member-data/member-data.repository.spec.ts @@ -0,0 +1,205 @@ +import { Container } from 'typedi'; + +import { Db } from '../../db/db.class'; +import { MemberDataRepository } from './member-data.repository'; + +/** + * A local chainable mock rather than @spec/mocks' shared one. + * + * This repository calls `db.knex('member_data')` as a FUNCTION and awaits the resulting + * builder, whereas the shared mockDb exposes `knex` as a plain object whose builder is not + * thenable -- awaiting it yields the builder itself, not rows. Rather than reshape a mock + * every other repository spec depends on, this keeps the change contained. + */ +function queryBuilder(result: any) { + const calls: any[][] = []; + const qb: any = { + calls, + /** Makes the builder awaitable, which is how knex builders actually behave. */ + then: (resolve: (value: any) => void) => resolve(result), + }; + [ + 'select', 'where', 'andWhere', 'whereIn', 'orderBy', + 'insert', 'onConflict', 'merge', 'del', 'first', + ].forEach(method => { + qb[method] = jest.fn((...args: any[]) => { + calls.push([method, ...args]); + return qb; + }); + }); + return qb; +} + +/** The arguments of the first call to `method`, or undefined if it was never called. */ +const argsOf = (qb: any, method: string): any[] | undefined => + qb.calls.find((call: any[]) => call[0] === method)?.slice(1); + +const called = (qb: any, method: string): boolean => + qb.calls.some((call: any[]) => call[0] === method); + +describe('MemberDataRepository', () => { + const MEMBER = 11; + let repository: MemberDataRepository; + let qb: any; + let knex: any; + + /** + * Rest args rather than a default parameter: `.first()` resolves to undefined when there + * is no row, and a default would turn an explicit useBuilder(undefined) into [], which is + * truthy and would quietly test the opposite of the no-row case. + */ + const useBuilder = (...result: any[]) => { + qb = queryBuilder(result.length ? result[0] : []); + knex.mockReturnValue(qb); + return qb; + }; + + beforeEach(() => { + knex = jest.fn(); + knex.transaction = jest.fn(); + Container.reset(); + Container.set(Db, { knex } as any); + repository = Container.get(MemberDataRepository); + useBuilder([]); + }); + + it('should create', () => { + expect(repository).toBeTruthy(); + }); + + describe('get', () => { + it('returns the stored value', async () => { + useBuilder({ value: 'hello' }); + expect(await repository.get(MEMBER, 'IMS')).toBe('hello'); + }); + + /** Absent and empty must not be distinguishable to callers. */ + it('returns null when there is no row', async () => { + useBuilder(undefined); + expect(await repository.get(MEMBER, 'IMS')).toBeNull(); + }); + }); + + describe('getAll', () => { + it('reduces rows to a name -> value object', async () => { + useBuilder([ + { name: 'IMS', value: '1' }, + { name: 'BU0', value: 'HawK' }, + ]); + expect(await repository.getAll(MEMBER)).toEqual({ IMS: '1', BU0: 'HawK' }); + }); + + it('is an empty object rather than null when nothing is stored', async () => { + expect(await repository.getAll(MEMBER)).toEqual({}); + }); + }); + + describe('getByPrefix', () => { + it('matches on the prefix and orders by name', async () => { + useBuilder([{ name: 'BU0', value: 'a' }]); + await repository.getByPrefix(MEMBER, 'BU'); + expect(argsOf(qb, 'andWhere')).toEqual(['name', 'like', 'BU%']); + expect(called(qb, 'orderBy')).toBe(true); + }); + + /** + * '_' and '%' are LIKE wildcards. Unescaped, a prefix of 'BU_' would match 'BUxx' and a + * caller could widen their own read past the family they asked for. + */ + it('escapes LIKE wildcards in the prefix', async () => { + await repository.getByPrefix(MEMBER, 'B_U%'); + expect(argsOf(qb, 'andWhere')).toEqual(['name', 'like', 'B\\_U\\%%']); + }); + + it('escapes a literal backslash too', async () => { + await repository.getByPrefix(MEMBER, 'B\\U'); + expect(argsOf(qb, 'andWhere')).toEqual(['name', 'like', 'B\\\\U%']); + }); + }); + + describe('set', () => { + it('upserts a real value', async () => { + await repository.set(MEMBER, 'IMS', '1'); + expect(argsOf(qb, 'insert')).toEqual([{ member_id: MEMBER, name: 'IMS', value: '1' }]); + expect(argsOf(qb, 'onConflict')).toEqual([['member_id', 'name']]); + expect(called(qb, 'del')).toBe(false); + }); + + /** + * "Unset" has to have exactly one representation, or a cleared buddy slot reads back as + * '' from one path and null from another. + */ + it.each([ + ['null', null], + ['an empty string', ''], + ['undefined', undefined], + ])('deletes the row when given %s', async (_label, value) => { + await repository.set(MEMBER, 'IMS', value as any); + expect(called(qb, 'del')).toBe(true); + expect(called(qb, 'insert')).toBe(false); + }); + }); + + describe('setMany', () => { + /** + * Runs the transaction callback against a trx that yields the same builder. + * + * Returns the builder WRAPPED, because the builder is thenable: returning it bare from + * an async function makes the runtime await it, so the caller would receive the query + * result ([]) instead of the builder whose calls they want to assert on. + */ + const runTransaction = async (values: Record) => { + const trxBuilder = queryBuilder([]); + const trx: any = jest.fn(() => trxBuilder); + knex.transaction.mockImplementation(async (cb: any) => cb(trx)); + await repository.setMany(MEMBER, values); + return { builder: trxBuilder }; + }; + + it('does nothing at all when given no values', async () => { + await repository.setMany(MEMBER, {}); + expect(knex.transaction).not.toHaveBeenCalled(); + }); + + it('runs inside a transaction so a partial write cannot land', async () => { + await runTransaction({ BU0: 'a' }); + expect(knex.transaction).toHaveBeenCalledTimes(1); + }); + + it('upserts the set values', async () => { + const { builder: trxBuilder } = await runTransaction({ BU0: 'a', BU1: 'b' }); + expect(argsOf(trxBuilder, 'insert')).toEqual([[ + { member_id: MEMBER, name: 'BU0', value: 'a' }, + { member_id: MEMBER, name: 'BU1', value: 'b' }, + ]]); + }); + + it('deletes the cleared ones by name', async () => { + const { builder: trxBuilder } = await runTransaction({ BU0: null, BU1: '', BU2: undefined }); + expect(argsOf(trxBuilder, 'whereIn')).toEqual(['name', ['BU0', 'BU1', 'BU2']]); + expect(called(trxBuilder, 'del')).toBe(true); + }); + + /** A mixed batch has to do both halves, not pick one. */ + it('handles a mix of sets and clears in one call', async () => { + const { builder: trxBuilder } = await runTransaction({ BU0: 'a', BU1: null }); + expect(argsOf(trxBuilder, 'whereIn')).toEqual(['name', ['BU1']]); + expect(argsOf(trxBuilder, 'insert')).toEqual([[ + { member_id: MEMBER, name: 'BU0', value: 'a' }, + ]]); + }); + + it('does not issue a delete when nothing is being cleared', async () => { + const { builder: trxBuilder } = await runTransaction({ BU0: 'a' }); + expect(called(trxBuilder, 'del')).toBe(false); + }); + }); + + describe('removeAllForMember', () => { + it('deletes every attribute for the member', async () => { + await repository.removeAllForMember(MEMBER); + expect(argsOf(qb, 'where')).toEqual(['member_id', MEMBER]); + expect(called(qb, 'del')).toBe(true); + }); + }); +}); diff --git a/api/src/repositories/member-data/member-data.repository.ts b/api/src/repositories/member-data/member-data.repository.ts new file mode 100644 index 00000000..178d3798 --- /dev/null +++ b/api/src/repositories/member-data/member-data.repository.ts @@ -0,0 +1,124 @@ +import { Service } from 'typedi'; + +import { Db } from '../../db'; +import { MemberData } from '../../types/models'; + +/** + * Reads and writes per-member named attributes (the CS 4.x MD / memdata table). + * + * Callers should generally go through a feature-specific service -- the buddy list, the + * privacy flag -- rather than reaching for raw attribute names here, so the meaning of + * each name stays in one place. + */ +@Service() +export class MemberDataRepository { + constructor(private db: Db) {} + + /** Every attribute for a member, as a plain name -> value object. */ + public async getAll(memberId: number): Promise> { + const rows = await this.db.knex('member_data') + .select('name', 'value') + .where('member_id', memberId); + return rows.reduce((acc, row) => { + acc[row.name] = row.value; + return acc; + }, {} as Record); + } + + /** A single attribute value, or null if it is unset. */ + public async get(memberId: number, name: string): Promise { + const row = await this.db.knex('member_data') + .select('value') + .where({ member_id: memberId, name }) + .first(); + return row ? row.value : null; + } + + /** + * Attributes whose name starts with `prefix`, as name -> value. + * + * Used to read a whole family at once, e.g. 'BU' for every buddy slot. The prefix is + * escaped so a caller cannot smuggle LIKE wildcards in and widen the match. + */ + public async getByPrefix( + memberId: number, + prefix: string, + ): Promise> { + const escaped = prefix.replace(/[\\%_]/g, char => `\\${char}`); + const rows = await this.db.knex('member_data') + .select('name', 'value') + .where('member_id', memberId) + .andWhere('name', 'like', `${escaped}%`) + .orderBy('name'); + return rows.reduce((acc, row) => { + acc[row.name] = row.value; + return acc; + }, {} as Record); + } + + /** + * Sets an attribute, replacing any existing value. + * + * A null or empty value DELETES the row rather than storing an empty string, so + * "unset" has exactly one representation. Otherwise a cleared buddy slot could read + * back as '' from one code path and null from another. + */ + public async set( + memberId: number, + name: string, + // undefined is accepted alongside null, not dead defensive code: these values come from + // request bodies and dynamically built objects, where a missing key reads as undefined + // long before it reaches a typed boundary. The guard below treats it as "unset". + value: string | null | undefined, + ): Promise { + if (value === null || value === undefined || value === '') { + await this.unset(memberId, name); + return; + } + await this.db.knex('member_data') + .insert({ member_id: memberId, name, value }) + .onConflict(['member_id', 'name']) + .merge(['value']); + } + + /** Sets several attributes in one transaction, so a partial write cannot land. */ + public async setMany( + memberId: number, + values: Record, + ): Promise { + const entries = Object.entries(values); + if (!entries.length) return; + + await this.db.knex.transaction(async trx => { + const toDelete = entries + .filter(([, value]) => value === null || value === undefined || value === '') + .map(([name]) => name); + const toUpsert = entries + .filter(([, value]) => !(value === null || value === undefined || value === '')) + .map(([name, value]) => ({ member_id: memberId, name, value: value as string })); + + if (toDelete.length) { + await trx('member_data').where('member_id', memberId).whereIn('name', toDelete).del(); + } + if (toUpsert.length) { + await trx('member_data') + .insert(toUpsert) + .onConflict(['member_id', 'name']) + .merge(['value']); + } + }); + } + + public async unset(memberId: number, name: string): Promise { + await this.db.knex('member_data').where({ member_id: memberId, name }).del(); + } + + /** Raw rows, for callers that need timestamps or ids. */ + public async findByMember(memberId: number): Promise { + return this.db.memberData.where({ member_id: memberId }); + } + + public async removeAllForMember(memberId: number): Promise { + await this.db.knex('member_data').where('member_id', memberId).del(); + } +} diff --git a/api/src/types/models/index.ts b/api/src/types/models/index.ts index f6e807f6..f8527796 100644 --- a/api/src/types/models/index.ts +++ b/api/src/types/models/index.ts @@ -3,6 +3,7 @@ export * from './club-member.model'; export * from './mall.model'; export * from './map-location.model'; export * from './member.model'; +export * from './member-data.model'; export * from './message.model'; export * from './store.model'; export * from './object.model'; diff --git a/api/src/types/models/member-data.model.ts b/api/src/types/models/member-data.model.ts new file mode 100644 index 00000000..9939dd33 --- /dev/null +++ b/api/src/types/models/member-data.model.ts @@ -0,0 +1,16 @@ +import { Model } from './model'; + +/** + * A named per-member attribute, as stored in the db. + * + * The CS 4.x MD / memdata table. Buddy slots (BU0..BU9), the hide-yourself flag (IMS) and + * similar per-member state live here as named attributes rather than as dedicated columns. + */ +export interface MemberData extends Model { + /** ID of the member the attribute belongs to */ + member_id: number; + /** Attribute name, e.g. 'BU0' or 'IMS' */ + name: string; + /** Attribute value; null clears it */ + value: string | null; +}