Deduplicate analysis policy config and appid SQL predicate

lib/analysisPolicy.js

@@ -0,0 +1,15 @@
+// Analysis scheduling policy, driven by environment variables. Centralised
+// here so models/Apps.js and the reporting scripts read identical defaults.
+//
+// Callers that load .env themselves (e.g. scripts using dotenv.config())
+// must require this module AFTER dotenv.config() has run, since the values
+// below are parsed once at require time.
+const CURRENT_ANALYSIS_VERSION = parseInt(process.env.CURRENT_ANALYSIS_VERSION || process.env.ANALYSIS_VERSION || '4', 10);
+const STALE_ANALYSIS_DAYS = parseInt(process.env.STALE_ANALYSIS_DAYS || '180', 10);
+const PROCESSING_TIMEOUT_MINUTES = parseInt(process.env.PROCESSING_TIMEOUT_MINUTES || '120', 10);
+
+module.exports = {
+  CURRENT_ANALYSIS_VERSION,
+  STALE_ANALYSIS_DAYS,
+  PROCESSING_TIMEOUT_MINUTES
+};

lib/appId.js

@@ -18,9 +18,19 @@ function isSameAppId(a, b) {
     && a.toLowerCase() === b.toLowerCase();
 }
 
+// SQL predicate that matches the same validity rules as isValidAppId(),
+// for use in queries against the appid column. patternParamIndex and
+// lengthParamIndex are the 1-based positional parameter indices to bind
+// APP_ID_PATTERN_SOURCE and MAX_APP_ID_LENGTH to (e.g. appIdSqlPredicate(2, 3)
+// produces a predicate using $2 and $3).
+function appIdSqlPredicate(patternParamIndex, lengthParamIndex) {
+  return `appid ~ $${patternParamIndex} AND appid !~ '[.]$' AND length(appid) <= $${lengthParamIndex}`;
+}
+
 module.exports = {
   APP_ID_PATTERN_SOURCE,
   MAX_APP_ID_LENGTH,
   isSameAppId,
-  isValidAppId
+  isValidAppId,
+  appIdSqlPredicate
 };

migrations/000_apps.sql

@@ -9,6 +9,8 @@ CREATE TABLE IF NOT EXISTS apps (
     details json,
     analysis json,
     analysisversion integer,
+    -- For failed analyses this is really "last attempt time": updateAnalysis
+    -- (models/Apps.js) stamps analysed = NOW() on failures too, not only on success.
     analysed timestamp without time zone,
     added timestamp without time zone NOT NULL DEFAULT NOW()
 );

models/Apps.js

@@ -2,8 +2,16 @@ const { Pool } = require('pg');
 const {
     APP_ID_PATTERN_SOURCE,
     MAX_APP_ID_LENGTH,
-    isValidAppId
+    isValidAppId,
+    appIdSqlPredicate
 } = require('../lib/appId');
+// index.js runs dotenv.config() before requiring this module (via server.js),
+// so process.env is already populated by the time analysisPolicy is read.
+const {
+    CURRENT_ANALYSIS_VERSION,
+    STALE_ANALYSIS_DAYS,
+    PROCESSING_TIMEOUT_MINUTES
+} = require('../lib/analysisPolicy');
 const pool = new Pool(
     process.env.DATABASE_URL
         ? { connectionString: process.env.DATABASE_URL }
@@ -59,19 +67,15 @@ const countQueue = async (added) => {
             FROM apps
             WHERE status = 'queued'
                 AND added < $1
-                AND appid ~ $2
-                AND appid !~ '[.]$'
-                AND length(appid) <= $3
+                AND ${appIdSqlPredicate(2, 3)}
         `, [added, APP_ID_PATTERN_SOURCE, MAX_APP_ID_LENGTH]);
         return result.rows[0].count;
     } else {
         const result = await pool.query(`
             SELECT COUNT(*)
             FROM apps
             WHERE status = 'queued'
-                AND appid ~ $1
-                AND appid !~ '[.]$'
-                AND length(appid) <= $2
+                AND ${appIdSqlPredicate(1, 2)}
         `, [APP_ID_PATTERN_SOURCE, MAX_APP_ID_LENGTH]);
         return result.rows[0].count;
     }
@@ -93,9 +97,9 @@ const popularityExpression = `
         ELSE 0
     END`;
 
-const currentAnalysisVersion = parseInt(process.env.CURRENT_ANALYSIS_VERSION || process.env.ANALYSIS_VERSION || '4', 10);
-const staleAnalysisDays = parseInt(process.env.STALE_ANALYSIS_DAYS || '180', 10);
-const processingTimeoutMinutes = parseInt(process.env.PROCESSING_TIMEOUT_MINUTES || '120', 10);
+const currentAnalysisVersion = CURRENT_ANALYSIS_VERSION;
+const staleAnalysisDays = STALE_ANALYSIS_DAYS;
+const processingTimeoutMinutes = PROCESSING_TIMEOUT_MINUTES;
 
 const nextApp = async () => {
     const client = await pool.connect();
@@ -111,9 +115,7 @@ const nextApp = async () => {
         const candidate = await client.query(`
             SELECT appid
             FROM apps
-            WHERE appid ~ $4
-                AND appid !~ '[.]$'
-                AND length(appid) <= $5
+            WHERE ${appIdSqlPredicate(4, 5)}
                 AND (
                     status = 'queued'
                     OR (
@@ -185,6 +187,10 @@ const updateAnalysis = async (appId, analysis, analysisVersion) => {
         // display and existing HTTP responses depend on it) AND set the new
         // scheduling columns. processing_started is cleared now that the lock
         // is resolved.
+        //
+        // Note: analysed is stamped with NOW() even when status ends up
+        // 'failed', so for failed apps this column really means "time of the
+        // last analysis attempt", not "time of the last successful analysis".
         const result = await client.query(
             `UPDATE apps
              SET analysis = $1,

scripts/priority-report.js

@@ -7,11 +7,16 @@ const { Client } = require('pg');
 dotenv.config({ path: path.join(__dirname, '..', '.env') });
 dotenv.config({ path: path.join(__dirname, '..', 'analyser', '.env') });
 
+// Required after dotenv.config() above, since analysisPolicy reads
+// process.env at require time.
+const {
+  CURRENT_ANALYSIS_VERSION: currentAnalysisVersion,
+  STALE_ANALYSIS_DAYS: staleAnalysisDays,
+  PROCESSING_TIMEOUT_MINUTES: processingTimeoutMinutes
+} = require('../lib/analysisPolicy');
+
 const limitArg = process.argv.find((arg) => arg.startsWith('--limit='));
 const limit = limitArg ? Math.max(1, parseInt(limitArg.split('=')[1], 10) || 20) : 20;
-const currentAnalysisVersion = parseInt(process.env.CURRENT_ANALYSIS_VERSION || process.env.ANALYSIS_VERSION || '4', 10);
-const staleAnalysisDays = parseInt(process.env.STALE_ANALYSIS_DAYS || '180', 10);
-const processingTimeoutMinutes = parseInt(process.env.PROCESSING_TIMEOUT_MINUTES || '120', 10);
 
 const reviewsExpr = `
   CASE

scripts/queue-status.js

@@ -7,9 +7,13 @@ const { Client } = require('pg');
 dotenv.config({ path: path.join(__dirname, '..', '.env') });
 dotenv.config({ path: path.join(__dirname, '..', 'analyser', '.env') });
 
-const currentAnalysisVersion = parseInt(process.env.CURRENT_ANALYSIS_VERSION || process.env.ANALYSIS_VERSION || '4', 10);
-const staleAnalysisDays = parseInt(process.env.STALE_ANALYSIS_DAYS || '180', 10);
-const processingTimeoutMinutes = parseInt(process.env.PROCESSING_TIMEOUT_MINUTES || '120', 10);
+// Required after dotenv.config() above, since analysisPolicy reads
+// process.env at require time.
+const {
+  CURRENT_ANALYSIS_VERSION: currentAnalysisVersion,
+  STALE_ANALYSIS_DAYS: staleAnalysisDays,
+  PROCESSING_TIMEOUT_MINUTES: processingTimeoutMinutes
+} = require('../lib/analysisPolicy');
 
 const reviewsExpr = `
   CASE