From ca17df75055a70f9035cee400b42ca992557ad9c Mon Sep 17 00:00:00 2001 From: PatrickNercessian Date: Mon, 6 May 2024 20:30:26 +0200 Subject: [PATCH 1/3] Migrate to dedicated station count endpoints --- lib/platform-routes.js | 14 ++++++++--- lib/platform-stats-fetchers.js | 29 +++++++++++++-------- lib/request-helpers.js | 27 ++++++++++++++++++++ lib/stats-fetchers.js | 46 ++++++++-------------------------- test/platform-routes.test.js | 38 ++++++++++++++++++++++++---- 5 files changed, 100 insertions(+), 54 deletions(-) diff --git a/lib/platform-routes.js b/lib/platform-routes.js index eeab76e5..daf72027 100644 --- a/lib/platform-routes.js +++ b/lib/platform-routes.js @@ -1,17 +1,25 @@ import { getStatsWithFilterAndCaching } from './request-helpers.js' -import { fetchDailyStationMetrics } from './platform-stats-fetchers.js' +import { fetchDailyStationCount, fetchMonthlyStationCount } from './platform-stats-fetchers.js' export const handlePlatformRoutes = async (req, res, pgPool) => { // Caveat! `new URL('//foo', 'http://127.0.0.1')` would produce "http://foo/" - not what we want! const { pathname, searchParams } = new URL(`http://127.0.0.1${req.url}`) const segs = pathname.split('/').filter(Boolean) - if (req.method === 'GET' && segs[0] === 'stations' && segs[1] === 'raw' && segs.length === 2) { + if (req.method === 'GET' && segs[0] === 'stations' && segs[1] === 'daily' && segs.length === 2) { await getStatsWithFilterAndCaching( pathname, searchParams, res, pgPool, - fetchDailyStationMetrics) + fetchDailyStationCount) + return true + } else if (req.method === 'GET' && segs[0] === 'stations' && segs[1] === 'monthly' && segs.length === 2) { + await getStatsWithFilterAndCaching( + pathname, + searchParams, + res, + pgPool, + fetchMonthlyStationCount) return true } else if (req.method === 'GET' && segs.length === 0) { // health check - required by Grafana datasources diff --git a/lib/platform-stats-fetchers.js b/lib/platform-stats-fetchers.js index eca4fdfd..8639fedb 100644 --- a/lib/platform-stats-fetchers.js +++ b/lib/platform-stats-fetchers.js @@ -1,16 +1,25 @@ +import { getDailyDistinctCountQuery, getMonthlyDistinctCountQuery } from './request-helpers.js' + /** * @param {import('pg').Pool} pgPool * @param {import('./typings').Filter} filter */ -export const fetchDailyStationMetrics = async (pgPool, filter) => { - const { rows } = await pgPool.query(` - SELECT day::TEXT, station_id - FROM daily_stations - WHERE day >= $1 AND day <= $2 - GROUP BY day, station_id - `, [ - filter.from, - filter.to - ]) +export const fetchDailyStationCount = async (pgPool, filter) => { + const { rows } = await pgPool.query( + getDailyDistinctCountQuery('daily_stations', 'station_id'), + [filter.from, filter.to] + ) + return rows +} + +/** + * @param {import('pg').Pool} pgPool + * @param {import('./typings').Filter} filter + */ +export const fetchMonthlyStationCount = async (pgPool, filter) => { + const { rows } = await pgPool.query( + getMonthlyDistinctCountQuery('daily_stations', 'station_id'), + [filter.from, filter.to] + ) return rows } diff --git a/lib/request-helpers.js b/lib/request-helpers.js index 1bcb806e..51f76219 100644 --- a/lib/request-helpers.js +++ b/lib/request-helpers.js @@ -84,3 +84,30 @@ const setCacheControlForStatsResponse = (res, filter) => { res.setHeader('cache-control', `public, max-age=${365 * 24 * 3600}, immutable`) } } + +// Fetch the "day" (DATE) as a string (TEXT) to prevent node-postgres from converting it into +// a JavaScript Date with a timezone, as that could change the date one day forward or back. +export const getDailyDistinctCountQuery = (table, column, asColumn = null) => { + if (!asColumn) asColumn = column + '_count' + return ` + SELECT day::TEXT, COUNT(DISTINCT ${column})::INT as ${asColumn} + FROM ${table} + WHERE day >= $1 AND day <= $2 + GROUP BY day + ORDER BY day + ` +} +export const getMonthlyDistinctCountQuery = (table, column, asColumn = null) => { + if (!asColumn) asColumn = column + '_count' + return ` + SELECT + date_trunc('month', day)::DATE::TEXT as month, + COUNT(DISTINCT ${column})::INT as ${asColumn} + FROM ${table} + WHERE + day >= date_trunc('month', $1::DATE) + AND day < date_trunc('month', $2::DATE) + INTERVAL '1 month' + GROUP BY month + ORDER BY month + ` +} diff --git a/lib/stats-fetchers.js b/lib/stats-fetchers.js index fb178a13..07da5027 100644 --- a/lib/stats-fetchers.js +++ b/lib/stats-fetchers.js @@ -1,3 +1,5 @@ +import { getDailyDistinctCountQuery, getMonthlyDistinctCountQuery } from './request-helpers.js' + /** * @param {import('pg').Pool} pgPool * @param {import('./typings').Filter} filter @@ -22,47 +24,19 @@ export const fetchRetrievalSuccessRate = async (pgPool, filter) => { return stats } -/** - * @param {import('pg').Pool} pgPool - * @param {import('./typings').Filter} filter - */ export const fetchDailyParticipants = async (pgPool, filter) => { - // Fetch the "day" (DATE) as a string (TEXT) to prevent node-postgres from converting it into - // a JavaScript Date with a timezone, as that could change the date one day forward or back. - const { rows } = await pgPool.query(` - SELECT day::TEXT, COUNT(DISTINCT participant_id)::INT as participants - FROM daily_participants - WHERE day >= $1 AND day <= $2 - GROUP BY day - ORDER BY day - `, [ - filter.from, - filter.to - ]) + const { rows } = await pgPool.query( + getDailyDistinctCountQuery('daily_participants', 'participant_id', 'participants'), + [filter.from, filter.to] + ) return rows } -/** - * @param {import('pg').Pool} pgPool - * @param {import('./typings').Filter} filter - */ export const fetchMonthlyParticipants = async (pgPool, filter) => { - // Fetch the "day" (DATE) as a string (TEXT) to prevent node-postgres from converting it into - // a JavaScript Date with a timezone, as that could change the date one day forward or back. - const { rows } = await pgPool.query(` - SELECT - date_trunc('month', day)::DATE::TEXT as month, - COUNT(DISTINCT participant_id)::INT as participants - FROM daily_participants - WHERE - day >= date_trunc('month', $1::DATE) - AND day < date_trunc('month', $2::DATE) + INTERVAL '1 month' - GROUP BY month - ORDER BY month - `, [ - filter.from, - filter.to - ]) + const { rows } = await pgPool.query( + getMonthlyDistinctCountQuery('daily_participants', 'participant_id', 'participants'), + [filter.from, filter.to] + ) return rows } diff --git a/test/platform-routes.test.js b/test/platform-routes.test.js index c9675907..e9ae0982 100644 --- a/test/platform-routes.test.js +++ b/test/platform-routes.test.js @@ -46,7 +46,7 @@ describe('Platform Routes HTTP request handler', () => { await pgPool.query('DELETE FROM daily_stations') }) - describe('GET /stations/raw', () => { + describe('GET /stations/daily', () => { it('returns daily station metrics for the given date range', async () => { await givenDailyStationMetrics(pgPool, '2024-01-10', ['station1']) await givenDailyStationMetrics(pgPool, '2024-01-11', ['station2']) @@ -55,7 +55,7 @@ describe('Platform Routes HTTP request handler', () => { const res = await fetch( new URL( - '/stations/raw?from=2024-01-11&to=2024-01-12', + '/stations/daily?from=2024-01-11&to=2024-01-12', baseUrl ), { redirect: 'manual' @@ -64,9 +64,37 @@ describe('Platform Routes HTTP request handler', () => { await assertResponseStatus(res, 200) const metrics = await res.json() assert.deepStrictEqual(metrics, [ - { day: '2024-01-11', station_id: 'station2' }, - { day: '2024-01-12', station_id: 'station2' }, - { day: '2024-01-12', station_id: 'station3' } + { day: '2024-01-11', station_id_count: 1 }, + { day: '2024-01-12', station_id_count: 2 } + ]) + }) + }) + + describe('GET /stations/monthly', () => { + it('returns monthly station metrics for the given date range ignoring the day number', async () => { + // before the date range + await givenDailyStationMetrics(pgPool, '2023-12-31', ['station1']) + // in the date range + await givenDailyStationMetrics(pgPool, '2024-01-10', ['station1']) + await givenDailyStationMetrics(pgPool, '2024-01-11', ['station2']) + await givenDailyStationMetrics(pgPool, '2024-01-12', ['station2', 'station3']) + await givenDailyStationMetrics(pgPool, '2024-02-13', ['station1']) + // after the date range + await givenDailyStationMetrics(pgPool, '2024-03-01', ['station1']) + + const res = await fetch( + new URL( + '/stations/monthly?from=2024-01-11&to=2024-02-11', + baseUrl + ), { + redirect: 'manual' + } + ) + await assertResponseStatus(res, 200) + const metrics = await res.json() + assert.deepStrictEqual(metrics, [ + { month: '2024-01-01', station_id_count: 3 }, + { month: '2024-02-01', station_id_count: 1 } ]) }) }) From f4db89d89f2bbac7ecac7b259b405aa92f71d712 Mon Sep 17 00:00:00 2001 From: PatrickNercessian Date: Thu, 9 May 2024 11:13:10 +0200 Subject: [PATCH 2/3] Improved local helper function SQL security --- lib/platform-stats-fetchers.js | 22 ++++++----- lib/request-helpers.js | 67 ++++++++++++++++++++++++++++------ lib/stats-fetchers.js | 24 +++++++----- 3 files changed, 81 insertions(+), 32 deletions(-) diff --git a/lib/platform-stats-fetchers.js b/lib/platform-stats-fetchers.js index 8639fedb..8f1ac6d1 100644 --- a/lib/platform-stats-fetchers.js +++ b/lib/platform-stats-fetchers.js @@ -5,11 +5,12 @@ import { getDailyDistinctCountQuery, getMonthlyDistinctCountQuery } from './requ * @param {import('./typings').Filter} filter */ export const fetchDailyStationCount = async (pgPool, filter) => { - const { rows } = await pgPool.query( - getDailyDistinctCountQuery('daily_stations', 'station_id'), - [filter.from, filter.to] - ) - return rows + return await getDailyDistinctCountQuery({ + pgPool, + table: 'daily_stations', + column: 'station_id', + filter + }) } /** @@ -17,9 +18,10 @@ export const fetchDailyStationCount = async (pgPool, filter) => { * @param {import('./typings').Filter} filter */ export const fetchMonthlyStationCount = async (pgPool, filter) => { - const { rows } = await pgPool.query( - getMonthlyDistinctCountQuery('daily_stations', 'station_id'), - [filter.from, filter.to] - ) - return rows + return await getMonthlyDistinctCountQuery({ + pgPool, + table: 'daily_stations', + column: 'station_id', + filter + }) } diff --git a/lib/request-helpers.js b/lib/request-helpers.js index 51f76219..4d9c0727 100644 --- a/lib/request-helpers.js +++ b/lib/request-helpers.js @@ -1,6 +1,7 @@ import assert from 'http-assert' import { json } from 'http-responders' import { URLSearchParams } from 'node:url' +import pg from 'pg' const getDayAsISOString = (d) => d.toISOString().split('T')[0] @@ -85,29 +86,71 @@ const setCacheControlForStatsResponse = (res, filter) => { } } -// Fetch the "day" (DATE) as a string (TEXT) to prevent node-postgres from converting it into -// a JavaScript Date with a timezone, as that could change the date one day forward or back. -export const getDailyDistinctCountQuery = (table, column, asColumn = null) => { +/** + * @param {object} args + * @param {pg.Pool} args.pgPool + * @param {string} args.table + * @param {string} args.column + * @param {import('./typings').Filter} args.filter + * @param {string} [args.asColumn] + */ +export const getDailyDistinctCountQuery = async ({ + pgPool, + table, + column, + filter, + asColumn = null +}) => { if (!asColumn) asColumn = column + '_count' - return ` - SELECT day::TEXT, COUNT(DISTINCT ${column})::INT as ${asColumn} - FROM ${table} + const safeTable = pg.escapeIdentifier(table) + const safeColumn = pg.escapeIdentifier(column) + const safeAsColumn = pg.escapeIdentifier(asColumn) + + // Fetch the "day" (DATE) as a string (TEXT) to prevent node-postgres from converting it into + // a JavaScript Date with a timezone, as that could change the date one day forward or back. + const { rows } = await pgPool.query(` + SELECT day::TEXT, COUNT(DISTINCT ${safeColumn})::INT as ${safeAsColumn} + FROM ${safeTable} WHERE day >= $1 AND day <= $2 GROUP BY day ORDER BY day - ` + `, [filter.from, filter.to]) + return rows } -export const getMonthlyDistinctCountQuery = (table, column, asColumn = null) => { + +/** + * @param {object} args + * @param {pg.Pool} args.pgPool + * @param {string} args.table + * @param {string} args.column + * @param {import('./typings').Filter} args.filter + * @param {string} [args.asColumn] + */ +export const getMonthlyDistinctCountQuery = async ({ + pgPool, + table, + column, + filter, + asColumn = null +}) => { if (!asColumn) asColumn = column + '_count' - return ` + const safeTable = pg.escapeIdentifier(table) + const safeColumn = pg.escapeIdentifier(column) + const safeAsColumn = pg.escapeIdentifier(asColumn) + + // Fetch the "day" (DATE) as a string (TEXT) to prevent node-postgres from converting it into + // a JavaScript Date with a timezone, as that could change the date one day forward or back. + const { rows } = await pgPool.query(` SELECT date_trunc('month', day)::DATE::TEXT as month, - COUNT(DISTINCT ${column})::INT as ${asColumn} - FROM ${table} + COUNT(DISTINCT ${safeColumn})::INT as ${safeAsColumn} + FROM ${safeTable} WHERE day >= date_trunc('month', $1::DATE) AND day < date_trunc('month', $2::DATE) + INTERVAL '1 month' GROUP BY month ORDER BY month - ` + `, [filter.from, filter.to] + ) + return rows } diff --git a/lib/stats-fetchers.js b/lib/stats-fetchers.js index 07da5027..ebcae531 100644 --- a/lib/stats-fetchers.js +++ b/lib/stats-fetchers.js @@ -25,19 +25,23 @@ export const fetchRetrievalSuccessRate = async (pgPool, filter) => { } export const fetchDailyParticipants = async (pgPool, filter) => { - const { rows } = await pgPool.query( - getDailyDistinctCountQuery('daily_participants', 'participant_id', 'participants'), - [filter.from, filter.to] - ) - return rows + return await getDailyDistinctCountQuery({ + pgPool, + table: 'daily_participants', + column: 'participant_id', + filter, + asColumn: 'participants' + }) } export const fetchMonthlyParticipants = async (pgPool, filter) => { - const { rows } = await pgPool.query( - getMonthlyDistinctCountQuery('daily_participants', 'participant_id', 'participants'), - [filter.from, filter.to] - ) - return rows + return await getMonthlyDistinctCountQuery({ + pgPool, + table: 'daily_participants', + column: 'participant_id', + filter, + asColumn: 'participants' + }) } /** From 1f3fbecd32c5af069f16e4e91ea638a56c58ece0 Mon Sep 17 00:00:00 2001 From: PatrickNercessian Date: Thu, 9 May 2024 17:48:25 +0200 Subject: [PATCH 3/3] Renamed request helper methods --- lib/platform-stats-fetchers.js | 6 +++--- lib/request-helpers.js | 4 ++-- lib/stats-fetchers.js | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/platform-stats-fetchers.js b/lib/platform-stats-fetchers.js index 8f1ac6d1..89fd4cf1 100644 --- a/lib/platform-stats-fetchers.js +++ b/lib/platform-stats-fetchers.js @@ -1,11 +1,11 @@ -import { getDailyDistinctCountQuery, getMonthlyDistinctCountQuery } from './request-helpers.js' +import { getDailyDistinctCount, getMonthlyDistinctCount } from './request-helpers.js' /** * @param {import('pg').Pool} pgPool * @param {import('./typings').Filter} filter */ export const fetchDailyStationCount = async (pgPool, filter) => { - return await getDailyDistinctCountQuery({ + return await getDailyDistinctCount({ pgPool, table: 'daily_stations', column: 'station_id', @@ -18,7 +18,7 @@ export const fetchDailyStationCount = async (pgPool, filter) => { * @param {import('./typings').Filter} filter */ export const fetchMonthlyStationCount = async (pgPool, filter) => { - return await getMonthlyDistinctCountQuery({ + return await getMonthlyDistinctCount({ pgPool, table: 'daily_stations', column: 'station_id', diff --git a/lib/request-helpers.js b/lib/request-helpers.js index 4d9c0727..780d6c7c 100644 --- a/lib/request-helpers.js +++ b/lib/request-helpers.js @@ -94,7 +94,7 @@ const setCacheControlForStatsResponse = (res, filter) => { * @param {import('./typings').Filter} args.filter * @param {string} [args.asColumn] */ -export const getDailyDistinctCountQuery = async ({ +export const getDailyDistinctCount = async ({ pgPool, table, column, @@ -126,7 +126,7 @@ export const getDailyDistinctCountQuery = async ({ * @param {import('./typings').Filter} args.filter * @param {string} [args.asColumn] */ -export const getMonthlyDistinctCountQuery = async ({ +export const getMonthlyDistinctCount = async ({ pgPool, table, column, diff --git a/lib/stats-fetchers.js b/lib/stats-fetchers.js index ebcae531..5cbd9760 100644 --- a/lib/stats-fetchers.js +++ b/lib/stats-fetchers.js @@ -1,4 +1,4 @@ -import { getDailyDistinctCountQuery, getMonthlyDistinctCountQuery } from './request-helpers.js' +import { getDailyDistinctCount, getMonthlyDistinctCount } from './request-helpers.js' /** * @param {import('pg').Pool} pgPool @@ -25,7 +25,7 @@ export const fetchRetrievalSuccessRate = async (pgPool, filter) => { } export const fetchDailyParticipants = async (pgPool, filter) => { - return await getDailyDistinctCountQuery({ + return await getDailyDistinctCount({ pgPool, table: 'daily_participants', column: 'participant_id', @@ -35,7 +35,7 @@ export const fetchDailyParticipants = async (pgPool, filter) => { } export const fetchMonthlyParticipants = async (pgPool, filter) => { - return await getMonthlyDistinctCountQuery({ + return await getMonthlyDistinctCount({ pgPool, table: 'daily_participants', column: 'participant_id',