Fix: Migrate to dedicated station count endpoints (#93)

* Migrate to dedicated station count endpoints * Improved local helper function SQL security * Renamed request helper methods
CheckerNetwork · May 10, 2024 · 5e1b373 · 5e1b373
1 parent 42098af
commit 5e1b373
Show file tree

Hide file tree

Showing 5 changed files with 152 additions and 57 deletions.
diff --git 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
@@ -1,16 +1,27 @@
+import { getDailyDistinctCount, getMonthlyDistinctCount } 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 getDailyDistinctCount({
+    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 getMonthlyDistinctCount({
+    pgPool,
+    table: 'daily_stations',
+    column: 'station_id',
+    filter
+  })
 }
diff --git 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]
 
@@ -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 getDailyDistinctCount = 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 getMonthlyDistinctCount = 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
+      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
+}
diff --git a/lib/stats-fetchers.js b/lib/stats-fetchers.js
@@ -1,3 +1,5 @@
+import { getDailyDistinctCount, getMonthlyDistinctCount } from './request-helpers.js'
+
 /**
  * @param {import('pg').Pool} pgPool
  * @param {import('./typings').Filter} filter
@@ -22,48 +24,24 @@ 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
-  ])
-  return rows
+  return await getDailyDistinctCount({
+    pgPool,
+    table: 'daily_participants',
+    column: 'participant_id',
+    filter,
+    asColumn: 'participants'
+  })
 }
 
-/**
- * @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
-  ])
-  return rows
+  return await getMonthlyDistinctCount({
+    pgPool,
+    table: 'daily_participants',
+    column: 'participant_id',
+    filter,
+    asColumn: 'participants'
+  })
 }
 
 /**

diff --git 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 }
       ])
     })
   })