db prep and test cleanup

.vscode/launch.json

@@ -5,7 +5,7 @@
     "version": "0.2.0",
     "configurations": [
         {
-            "name": "Launch via NPM",
+            "name": "Node Server",
             "request": "launch",
             "runtimeArgs": [
                 "run-script",
@@ -18,7 +18,7 @@
             "type": "node"
         },
         {
-            "name": "Test",
+            "name": "Full Test",
             "request": "launch",
             "runtimeArgs": [
                 "run-script",
@@ -29,6 +29,6 @@
                 "<node_internals>/**"
             ],
             "type": "node"
-        },
+        }
     ]
 }

app/appEndpoints.js

@@ -2,8 +2,6 @@ const express = require('express');
 const config = require("./config");
 const errors = require("./util/errors");
 const nunjucks = require("./nunjucks");
-const { initMail } = require("./emailer");
-const knex = require("./db");  // initialize on startup
 const app = express();
 
 // shift.conf for nginx sets the x-forward header
@@ -70,5 +68,5 @@ app.use(function(err, req, res, next) {
   console.error(err.stack);
 });
 
-// for testing
+// for starting a server or running tests
 module.exports = app;

app/config.js

@@ -1,10 +1,34 @@
+// global configuration for the backend
+// holds all environment, commandline options, paths, etc.
+//
+// const config = require('./config');
+//
 const path = require('path');
 const fs = require("fs");
+const { CommandLine } = require('./util/cmdLine.js');
 
+// helper to read environment variables
 function env_default(field, def) {
   return process.env[field] ?? def;
 }
 
+// helper to turn lines of quoted text into a single string
+function lines(...lines) {
+  return lines.join("\n");
+}
+
+const cmdLine = new CommandLine({
+  db: lines(
+    `an optional string. can be 'sqlite' or 'mysql'.`,
+    `the behavior depends on context. for tests, defaults to sqlite. for prod and dev defaults to mysql.`,
+    `- 'mysql': requires docker. for prod and dev pulls its full configuration from the environment; tests have a hardcoded configuration.`,
+    `- 'sqlite': can followed by the path to its file. For example: '-db=sqlite:../bin/events.db'. That example path is its default.`,
+  ),
+  db_debug: lines(
+    `an optional flag. adds extra debugging info for queries.`,
+  ),
+});
+
 // node server listen for its http requests
 // fix? there is no such environment variable right now.
 const listen = env_default('NODE_PORT', 3080);
@@ -15,23 +39,26 @@ const siteHost = siteUrl(listen);
 // location of app.js ( same as config.js )
 const appPath = path.resolve(__dirname);
 
-// for max file size
+// for max image size
 const bytesPerMeg = 1024*1024;
 
 const staticFiles = env_default('SHIFT_STATIC_FILES');
 
-const isTesting = !!(process.env.npm_lifecycle_event || "").match(/test$/);
-// read the command line parameter for db configuration
-const dbType = env_default('npm_config_db');
-const dbDebug = !!env_default('npm_config_db_debug');
+// running node --test with a glob ( ex.  node --test **/*_test.js )
+// injects all of those files onto the command line as separate arguments.
+const isTesting = process.env.npm_lifecycle_event && process.env.npm_lifecycle_event.startsWith('test');
+// ^ FIX: check for --test if running node directly?
+
+const dbConfig = getDatabaseConfig(cmdLine.options.db, cmdLine.bool('db_debug'), isTesting);
 
+// The config object exported
 const config = {
   appPath,
   api: {
     header: 'Api-Version',
     version: "3.60.0",
   },
-  db: getDatabaseConfig(dbType, isTesting),
+  db: dbConfig,
   // maybe bad, but some code likes to know:
   isTesting,
   // a nodemailer friendly config, or false if smtp is not configured.
@@ -195,17 +222,27 @@ function getSmtpSettings() {
   }
 }
 
-// our semi-agnostic database configuration
-function getDatabaseConfig(dbType, isTesting) {
-  // dbType comes from the command-line
-  // if nothing was specfied, use the MYSQL_DATABASE environment variable
-  const env = env_default('MYSQL_DATABASE')
-  if (!dbType && env) {
-    dbType = env.startsWith("sqlite") ? env : null;
-  }
+// nothing specified in the env fails
+// sqlite specified in the env uses.... sqlite.
+// anything other than sqlite in the env is assumed to be a mysql spec.
+function readDbEnv(env = 'MYSQL_DATABASE') {
+  const dbType = env_default(env);
   if (!dbType) {
-    dbType =  isTesting ? 'sqlite' : 'mysql'
+    throw new Error(`expected db type on command line or '${env}' env variable`);
   }
+  return dbType.startsWith("sqlite") ? dbType : 'mysql';
+}
+
+// returns our semi-agnostic database configuration.
+// read the command line and environment for the desired db.
+//
+// dbType can be 'sqlite' or 'mysql' or none.
+// 'mysql' pulls its full configuration from the environment.
+// 'sqlite' can optionally followed by the path to its file.
+// testing defaults to sqlite; dev and prod fallback to the environment.
+//
+function getDatabaseConfig(dbType, dbDebug, isTesting) {
+  dbType = dbType || (isTesting ? 'sqlite' : readDbEnv());
   const [name, parts] = dbType.split(':');
   const config = {
     mysql: !isTesting ? getMysqlDefault : getMysqlTesting,

app/db.js

@@ -7,6 +7,7 @@ const dt = require("./util/dateTime");
 const dbConfig = unpackConfig(config.db);
 const useSqlite = config.db.type === 'sqlite';
 const dropOnCreate = config.db.connect?.name === 'shift_test';
+let prevSetup;
 
 const db = {
   config: config.db,
@@ -19,10 +20,11 @@ const db = {
   // waits to open a connection.
   async initialize(name) {
     if (db.query) {
-      throw new Error("db already initialized");
+      throw new Error(`db being initialized by ${name} when already initialized by ${this.initialized}.`);
     }
     const connection = knex(dbConfig);
     db.query = connection;
+    db.initialized = name;
     await connection;
   },
 
@@ -33,6 +35,7 @@ const db = {
       throw new Error("db already destroyed");
     }
     db.query = false;
+    db.initialized = false;
     return connection.destroy();
   },
 

app/endpoints/crawl.js

@@ -15,7 +15,7 @@ const { CalDaily } = require("../models/calDaily");
 const { to12HourString, from24HourString, friendlyDate } = require("../util/dateTime");
 
 exports.get = function(req, res, next) {
-  let id = req.query.id;
+  const id = req.query.id;
   const p = {
     title: config.crawl.title,
     description : config.crawl.description,

app/endpoints/delete_event.js

@@ -19,18 +19,16 @@ const express = require('express');
 const textError = require("../util/errors");
 const { CalEvent } = require("../models/calEvent");
 const { uploader } = require("../uploader");
+const { safeParse } = require("../models/calEventValidator");
 
 // the front end sends a multi-part form post
 // so... we need to handle that.
 exports.post = [ uploader.makeHandler(), handleRequest ];
 
 function handleRequest(req, res, next) {
-  let data = req.body;
   // fix? client uploads form data containing json...
   // probably to match manage_event where its currently required.
-  if (data && data.json) {
-    data = safeParse(data.json);
-  }
+  const data = safeParse(req.body);
   if (!data) {
     return res.textError('JSON could not be decoded');
   }
@@ -39,7 +37,8 @@ function handleRequest(req, res, next) {
   }
   // get the event: delete seems to send an int, where manage is a string.
   // normalize it to a string for consistency ( tbd: is that good, or even needed? )
-  return CalEvent.getByID(''+data.id).then((evt) => {
+  const seriesId = '' + data.id; // normalize int into a string
+  return CalEvent.getByID(seriesId).then((evt) => {
     // verify the event exists.
     if (!evt) {
       return res.textError('Event not found');
@@ -52,7 +51,7 @@ function handleRequest(req, res, next) {
 
     // if the event was never published, we can delete it completely;
     // otherwise, soft delete it.
-    let q = !evt.isPublished() ? evt.eraseEvent() : evt.softDelete();
+    const q = !evt.isPublished() ? evt.eraseEvent() : evt.softDelete();
     q.then((_) => {
       // note: the frontend currently doesn't use this json;
       // instead it looks for request success ( http 200 )
@@ -66,13 +65,3 @@ function handleRequest(req, res, next) {
     });
   }).catch(next);
 }
-
-// read json into a javascript object.
-// returns undefined for any error.
-function safeParse(json) {
-  try {
-    return JSON.parse(json);
-  } catch (err) {
-    console.error(err);
-  }
-}

app/endpoints/events.js

@@ -29,26 +29,25 @@ const { fromYMDString, to24HourString, toYMDString } = require("../util/dateTime
 const { CalEvent } = require("../models/calEvent");
 const { CalDaily } = require("../models/calDaily");
 const { EventsRange } = require("../models/calConst");
+const validator = require('validator');
 
 // the events endpoint:
 exports.get = function(req, res, next) {
-  let id = req.query.id;
+  const dayId = readInt(req.query.id);  // pkid
   let start = req.query.startdate;
   let end = req.query.enddate;
   const includeAllEvents = (req.query.all === "true") || (req.query.all === "1");
-  if (id && start && end) {
+  if (dayId && start && end) {
     res.textError("expected only an id or date range"); // fix, i think its supposed be sending a json error.
-  } else if (id) {
-    // return the summary of a particular daily event:
-    return CalDaily.getByDailyID(id).then((daily) => {
+  } else if (dayId) {
+    // return the summary of a particular event on a particular day:
+    return CalDaily.getByDailyID(dayId).then((daily) => {
       if (!daily) {
-        res.textError("no such time");
+        res.textError("no such day");
       } else  {
         return getSummaries([daily]).then((events) => {
           res.set(config.api.header, config.api.version);
-          res.json({
-            events
-          });
+          res.json({events});
         });
       }
     }).catch(next);
@@ -82,6 +81,10 @@ exports.get = function(req, res, next) {
   }
 }
 
+function readInt(i, opt) {
+  return (i !== undefined) && validator.isInt(i, opt) && validator.toInt(i);
+}
+
 // promise an array containing json-friendly summaries of all the passed CalDaily(s)
 // see also: buildEntries()
 function getSummaries(dailies) {

app/endpoints/ical.js

@@ -227,6 +227,7 @@ function buildCalEntry(evt, at) {
     // php handles this just fine.
     startAt = dt.combineDateAndTime(at.eventdate, dt.from12HourString("12:00 PM"));
   }
+
   const endAt = evt.addDuration(startAt);
   const url = at.getShareable();
   let title = evt.title;
@@ -249,7 +250,7 @@ function buildCalEntry(evt, at) {
     description: [
       news,
       evt.descr, evt.timedetails,
-      evt.locend? "Ends at "+ evt.locend: null,
+      evt.locend ? ("Ends at "+ evt.locend) : null,
       url
     ],
     location: [

app/endpoints/manage_event.js

@@ -23,7 +23,7 @@ const dt = require("../util/dateTime");
 const config = require("../config");
 const emailer = require("../emailer");
 const nunjucks = require("../nunjucks");
-const { validateEvent } = require("../models/calEventValidator");
+const { safeParse, validateEvent } = require("../models/calEventValidator");
 
 // read multipart (and curl) posts.
 exports.post = [ uploader.makeHandler(), handleRequest ];
@@ -37,13 +37,10 @@ exports.post = [ uploader.makeHandler(), handleRequest ];
  * it should only reject due to things like database or programmer errors.
  */
 function handleRequest(req, res, next) {
-  let input = req.body;
   // fix? the client uploads form data containing json
   // ( rather than raw json ) because multi-part forms require that.
   // a more rest-like api might use a separate put at some event/image url.
-  if (input && input.json) {
-    input = safeParse(input.json);
-  }
+  const input = safeParse(req.body);
   if (!input) {
     return res.textError("invalid request");
   }
@@ -66,7 +63,7 @@ function handleRequest(req, res, next) {
       // save the uploaded file (if any)
       const saveImage = !req.file ? Promise.resolve() :
         // the image gets written to disk as "id.ext"
-        uploader.write( req.file, evt.id ).then(f => {
+        uploader.write(req.file, evt.id.toString()).then(f => {
           // the image name gets stored in the db as "id-sequence.ext"
           // the sequence number needs to be different for each new image.
           // ( shift.conf strips off the sequence when the file is requested; see cache_busting.md )
@@ -161,13 +158,3 @@ function sendConfirmationEmail(evt) {
     // attachments
   }, evt.name, evt.email, evt.title, url);
 }
-
-// read json into a javascript object.
-// returns undefined for any error.
-function safeParse(json) {
-  try {
-    return JSON.parse(json);
-  } catch (err) {
-    console.error(err);
-  }
-}

app/models/calEventValidator.js

@@ -85,6 +85,7 @@ function makeValidator(input, errors) {
       if (validator.isEmpty(str)) {
         errors.addError('time');
       } else {
+        // input is AM/PM style
         let t = dt.from12HourString(str);
         if (!t.isValid()) {
           t = dt.from24HourString(str);
@@ -252,8 +253,22 @@ function validateEvent(input) {
   };
 }
 
+// read a possible json request into a javascript object.
+// returns undefined for any error.
+function safeParse(data) {
+  if (data && data.json) {
+    try {
+      data = JSON.parse(data.json);
+    } catch (err) {
+      console.error(err);
+    }
+  }
+  return data;
+}
+
 module.exports = {
   validateEvent,
+  safeParse,
   // exported for testing:
   makeValidator,
   ErrorCollector

app/package.json

@@ -20,7 +20,7 @@
   },
   "scripts": {
     "start": "node app.js",
-    "test": "node --test --trace-warnings --test-global-setup=test/db_setup.js --test-concurrency=1 **/*_test.js --"
+    "test": "node ./test/testRunner.js"
   },
   "devDependencies": {
     "shelljs": "^0.10.0",