diff --git a/src/backend/clients/database/DatabaseClient.ts b/src/backend/clients/database/DatabaseClient.ts index f666059a6..14b5b1de6 100644 --- a/src/backend/clients/database/DatabaseClient.ts +++ b/src/backend/clients/database/DatabaseClient.ts @@ -330,6 +330,30 @@ export class AbstractDatabaseClient extends PuterClient { }); } + /** + * Whole `periodSeconds` buckets since the epoch, from a unix-seconds + * column. MySQL's `/` yields a decimal, which would split a bucket. + */ + periodBucket(secondsExpression: string, periodSeconds: number): string { + return this.case({ + mysql: `(${secondsExpression} DIV ${periodSeconds})`, + otherwise: `(${secondsExpression} / ${periodSeconds})`, + }); + } + + /** + * Counts each (`userExpression`, bucket) pair once. `COUNT(DISTINCT a, b)` + * is MySQL-only, so the pair folds into one number — injective while the + * bucket stays under the multiplier, which hourly reaches in the 2980s. + */ + countDistinctUserPeriods( + userExpression: string, + secondsExpression: string, + periodSeconds: number, + ): string { + return `COUNT(DISTINCT ${userExpression} * 10000000 + ${this.periodBucket(secondsExpression, periodSeconds)})`; + } + nullCoalesce(...expressions: readonly string[]): string { if (expressions.length === 0) { throw new Error('nullCoalesce requires at least one expression'); diff --git a/src/backend/controllers/apps/AppController.test.ts b/src/backend/controllers/apps/AppController.test.ts index a387ae60a..db65e1faa 100644 --- a/src/backend/controllers/apps/AppController.test.ts +++ b/src/backend/controllers/apps/AppController.test.ts @@ -338,6 +338,42 @@ describe('AppController POST /rao', () => { expect(rows).toHaveLength(1); }); + it('keeps every open as a row, and counts one per account per hour', async () => { + const owner = await makeUser(); + const other = await makeUser(); + const app = await createApp(owner.actor); + + const open = async (user: { actor: Actor; userId: number }) => { + const { res } = makeRes(); + await withActor(user.actor, () => + callRoute( + 'post', + '/rao', + makeReq({ body: { app_uid: app.uid }, actor: user.actor }), + res, + ), + ); + }; + + for (let i = 0; i < 5; i++) await open(owner); + await open(other); + await server.controllers.apps.drainPendingAppOpens(); + + // Every open is still a row: the recent-apps list orders on them. + const rows = (await server.clients.db.read( + 'SELECT `user_id` FROM `app_opens` WHERE `app_uid` = ?', + [app.uid], + )) as Array<{ user_id: number }>; + expect(rows).toHaveLength(6); + + // The count is accounts-per-hour, so repeats do not move it. + const stats = await server.stores.app.getAppsStats([app.uid]); + expect(stats.get(app.uid)).toMatchObject({ + open_count: 2, + user_count: 2, + }); + }); + // `app_opens` is analytics: the response carries nothing derived from it // and the client never awaits the post. Holding the response open for a // primary write put that write's latency inside the app launch it was diff --git a/src/backend/stores/app/AppStore.js b/src/backend/stores/app/AppStore.js index 406411fd7..9d6ff037d 100644 --- a/src/backend/stores/app/AppStore.js +++ b/src/backend/stores/app/AppStore.js @@ -62,6 +62,8 @@ const OLD_APP_NAME_TTL_MONTHS = 3; const BULK_QUERY_CHUNK_SIZE = 200; // Placeholders per index_url `IN (…)` chunk; under SQLite's old 999 default. const INDEX_URL_CHUNK_SIZE = 900; +// Window an account's repeat opens of one app collapse into a single count. +const APP_OPEN_COUNT_PERIOD_SECONDS = 60 * 60; // Top-level all-time open/user counts: hot path, slow to compute. Cached // lazily on read (pipelined MGET on every app list/read; misses query the @@ -1446,7 +1448,7 @@ export class AppStore extends PuterStore { const res = await clickhouse.query({ query: ` SELECT app_uid, - count(_id) AS open_count, + uniqExact((user_id, intDiv(ts, ${APP_OPEN_COUNT_PERIOD_SECONDS}))) AS open_count, count(DISTINCT user_id) AS user_count FROM app_opens WHERE app_uid IN {uids:Array(String)} @@ -1468,7 +1470,7 @@ export class AppStore extends PuterStore { const placeholders = uids.map(() => '?').join(','); const rows = await this.clients.db.read( `SELECT app_uid, - COUNT(_id) AS open_count, + ${this.clients.db.countDistinctUserPeriods('user_id', 'ts', APP_OPEN_COUNT_PERIOD_SECONDS)} AS open_count, COUNT(DISTINCT user_id) AS user_count FROM app_opens WHERE app_uid IN (${placeholders}) @@ -1495,7 +1497,7 @@ export class AppStore extends PuterStore { } const res = await clickhouse.query({ query: ` - SELECT count(_id) AS open_count, + SELECT uniqExact((user_id, intDiv(ts, ${APP_OPEN_COUNT_PERIOD_SECONDS}))) AS open_count, count(DISTINCT user_id) AS user_count FROM app_opens WHERE app_uid = {appUid:String} @@ -1518,7 +1520,7 @@ export class AppStore extends PuterStore { : [appUid]; const where = timeRange ? 'AND ts >= ? AND ts < ?' : ''; const rows = await this.clients.db.read( - `SELECT COUNT(_id) AS open_count, + `SELECT ${this.clients.db.countDistinctUserPeriods('user_id', 'ts', APP_OPEN_COUNT_PERIOD_SECONDS)} AS open_count, COUNT(DISTINCT user_id) AS user_count FROM app_opens WHERE app_uid = ? ${where}`, @@ -1544,7 +1546,7 @@ export class AppStore extends PuterStore { const res = await clickhouse.query({ query: ` SELECT ${groupBy} AS period, - count(_id) AS open_count, + uniqExact((user_id, intDiv(ts, ${APP_OPEN_COUNT_PERIOD_SECONDS}))) AS open_count, count(DISTINCT user_id) AS user_count FROM app_opens WHERE app_uid = {appUid:String} @@ -1579,7 +1581,7 @@ export class AppStore extends PuterStore { }); const rows = await this.clients.db.read( `SELECT ${periodExpr} AS period, - COUNT(_id) AS open_count, + ${this.clients.db.countDistinctUserPeriods('user_id', 'ts', APP_OPEN_COUNT_PERIOD_SECONDS)} AS open_count, COUNT(DISTINCT user_id) AS user_count FROM app_opens WHERE app_uid = ? AND ts >= ? AND ts < ? diff --git a/src/backend/stores/app/AppStore.test.js b/src/backend/stores/app/AppStore.test.js index 533be320d..b35e654ba 100644 --- a/src/backend/stores/app/AppStore.test.js +++ b/src/backend/stores/app/AppStore.test.js @@ -77,7 +77,7 @@ describe('AppStore app stats (cache-on-read)', () => { it('computes all-time open/unique-user counts from SQL when no ClickHouse client is registered', async () => { const uid = freshUid(); - // 3 opens across 2 distinct users. + // 3 rows across 2 users, two of them the same user within one hour. await insertOpen(uid, 1, 1700000000); await insertOpen(uid, 1, 1700000100); await insertOpen(uid, 2, 1700000200); @@ -85,12 +85,39 @@ describe('AppStore app stats (cache-on-read)', () => { const stats = await appStore.getAppsStats([uid]); expect(stats.get(uid)).toEqual({ - open_count: 3, + // An account's repeat opens in an hour count once. + open_count: 2, user_count: 2, referral_count: null, }); }); + it('counts the same account again in a later hour', async () => { + const uid = freshUid(); + const hour = 3600; + await insertOpen(uid, 1, 1700000000); + await insertOpen(uid, 1, 1700000000 + 30); + await insertOpen(uid, 1, 1700000000 + hour); + await insertOpen(uid, 1, 1700000000 + 2 * hour); + + expect((await appStore.getAppsStats([uid])).get(uid)).toMatchObject({ + open_count: 3, + user_count: 1, + }); + }); + + it('buckets on the clock hour rather than a rolling window', async () => { + const uid = freshUid(); + // 1699999200 starts an hour; these straddle its end a second apart. + await insertOpen(uid, 1, 1699999200 + 3599); + await insertOpen(uid, 1, 1699999200 + 3600); + + expect((await appStore.getAppsStats([uid])).get(uid)).toMatchObject({ + open_count: 2, + user_count: 1, + }); + }); + it('caches counts on read with an autoexpire TTL', async () => { const uid = freshUid(); await insertOpen(uid, 7, 1700000000); @@ -1425,10 +1452,10 @@ describe('AppStore detailed stats', () => { db = server.clients.db; await clearAppCache(server.clients.redis); - // Three opens right now from two distinct users. + // Two users; the repeat shares a timestamp so it is always same-hour. for (const [userId, offset] of [ [1, 0], - [1, 60], + [1, 0], [2, 120], ]) { await db.write( @@ -1445,7 +1472,8 @@ describe('AppStore detailed stats', () => { it('computes all-time totals from SQL by default', async () => { const stats = await appStore.getAppStatsDetailed(APP_UID); expect(stats).toEqual({ - open_count: 3, + // Three rows, but two of them one account in one hour. + open_count: 2, user_count: 2, referral_count: null, }); @@ -1471,7 +1499,8 @@ describe('AppStore detailed stats', () => { const stats = await appStore.getAppStatsDetailed(APP_UID, { period, }); - expect(stats.open_count).toBe(3); + // The fixture's three rows are two accounts in one hour. + expect(stats.open_count).toBe(2); expect(stats.user_count).toBe(2); }); @@ -1498,7 +1527,7 @@ describe('AppStore detailed stats', () => { const stats = await appStore.getAppStatsDetailed(APP_UID, { period: 'not-a-period', }); - expect(stats.open_count).toBe(3); + expect(stats.open_count).toBe(2); }); it('rejects an unsupported grouping', async () => { @@ -1516,7 +1545,7 @@ describe('AppStore detailed stats', () => { grouping: 'day', }); - expect(stats.open_count).toBe(3); + expect(stats.open_count).toBe(2); expect(stats.user_count).toBe(2); const open = stats.grouped_stats.open_count; // A 7-day window yields 8 day buckets (inclusive of both ends). @@ -1524,7 +1553,7 @@ describe('AppStore detailed stats', () => { expect(open.every((b) => typeof b.count === 'number')).toBe(true); const today = new Date().toISOString().slice(0, 10); const todayBucket = open.find((b) => b.period === today); - expect(todayBucket?.count).toBe(3); + expect(todayBucket?.count).toBe(2); // Every earlier bucket is an explicit zero, not a gap. expect( open.filter((b) => b.period !== today).every((b) => b.count === 0), @@ -1538,7 +1567,7 @@ describe('AppStore detailed stats', () => { period: 'today', grouping, }); - expect(stats.open_count).toBe(3); + expect(stats.open_count).toBe(2); expect(stats.grouped_stats.open_count.length).toBeGreaterThan(0); expect(stats.grouped_stats.user_count.length).toBe( stats.grouped_stats.open_count.length, @@ -1555,7 +1584,7 @@ describe('AppStore detailed stats', () => { period: '30d', grouping: 'week', }); - expect(stats.open_count).toBe(3); + expect(stats.open_count).toBe(2); expect(stats.grouped_stats.open_count.length).toBeGreaterThan(0); }); @@ -1830,13 +1859,23 @@ describe('AppStore findCanonicalUidByIndexUrlCandidates', () => { it('prefers the private row, then the oldest match', async () => { const url = `https://canon-${Math.random().toString(36).slice(2, 10)}.example.com/`; const pub = await appStore.create( - { name: `pub-${Math.random().toString(36).slice(2, 8)}`, title: 't', index_url: url }, + { + name: `pub-${Math.random().toString(36).slice(2, 8)}`, + title: 't', + index_url: url, + }, { ownerUserId: 1 }, ); const privUid = `app-${Math.random().toString(36).slice(2, 10)}`; await server.clients.db.write( 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `is_private`) VALUES (?, ?, ?, ?, ?)', - [privUid, `priv-${Math.random().toString(36).slice(2, 8)}`, 't', url, 1], + [ + privUid, + `priv-${Math.random().toString(36).slice(2, 8)}`, + 't', + url, + 1, + ], ); const uid = await appStore.findCanonicalUidByIndexUrlCandidates([url]); @@ -1845,8 +1884,12 @@ describe('AppStore findCanonicalUidByIndexUrlCandidates', () => { }); it('returns null for an empty, non-array, or non-matching candidate list', async () => { - expect(await appStore.findCanonicalUidByIndexUrlCandidates([])).toBeNull(); - expect(await appStore.findCanonicalUidByIndexUrlCandidates(null)).toBeNull(); + expect( + await appStore.findCanonicalUidByIndexUrlCandidates([]), + ).toBeNull(); + expect( + await appStore.findCanonicalUidByIndexUrlCandidates(null), + ).toBeNull(); expect( await appStore.findCanonicalUidByIndexUrlCandidates([ 'https://nothing-here.example.com/', @@ -1903,13 +1946,25 @@ describe('AppStore listIndexUrlWinners', () => { const priv1Uid = `app-${Math.random().toString(36).slice(2, 10)}`; await server.clients.db.write( 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `is_private`) VALUES (?, ?, ?, ?, ?)', - [priv1Uid, `priv1-${Math.random().toString(36).slice(2, 8)}`, 't', url, 1], + [ + priv1Uid, + `priv1-${Math.random().toString(36).slice(2, 8)}`, + 't', + url, + 1, + ], ); const priv1 = await appStore.getByUid(priv1Uid); const priv2Uid = `app-${Math.random().toString(36).slice(2, 10)}`; await server.clients.db.write( 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `is_private`) VALUES (?, ?, ?, ?, ?)', - [priv2Uid, `priv2-${Math.random().toString(36).slice(2, 8)}`, 't', url, 1], + [ + priv2Uid, + `priv2-${Math.random().toString(36).slice(2, 8)}`, + 't', + url, + 1, + ], ); const priv2 = await appStore.getByUid(priv2Uid);