-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Migrate to dedicated station count endpoints #93
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,27 @@ | ||
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 | ||
]) | ||
return rows | ||
export const fetchDailyStationCount = async (pgPool, filter) => { | ||
return await getDailyDistinctCountQuery({ | ||
pgPool, | ||
table: 'daily_stations', | ||
column: 'station_id', | ||
filter | ||
}) | ||
} | ||
|
||
/** | ||
* @param {import('pg').Pool} pgPool | ||
* @param {import('./typings').Filter} filter | ||
*/ | ||
export const fetchMonthlyStationCount = async (pgPool, filter) => { | ||
return await getMonthlyDistinctCountQuery({ | ||
pgPool, | ||
table: 'daily_stations', | ||
column: 'station_id', | ||
filter | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -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] | ||||||
|
||||||
|
@@ -84,3 +85,72 @@ const setCacheControlForStatsResponse = (res, filter) => { | |||||
res.setHeader('cache-control', `public, max-age=${365 * 24 * 3600}, immutable`) | ||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
* @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' | ||||||
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 | ||||||
} | ||||||
|
||||||
/** | ||||||
* @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 ({ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto.
Suggested change
|
||||||
pgPool, | ||||||
table, | ||||||
column, | ||||||
filter, | ||||||
asColumn = null | ||||||
}) => { | ||||||
if (!asColumn) asColumn = column + '_count' | ||||||
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 ${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 | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have mixed feelings about the name However, I also see how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like how precise it is, also it is a bit leaky (potentially exposes too many internals). I suggest we iterate on this at a later point, when we understand our system better |
||
{ month: '2024-02-01', station_id_count: 1 } | ||
]) | ||
}) | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this function executes the query instead of returning the query, I think it should not have
Query
in the name.