Merge pull request #331 from getodk/issa/refactor

refactor

.eslintrc.json

@@ -14,6 +14,7 @@
     "import/order": "off",
     "lines-between-class-members": "off",
     "max-len": "off",
+    "no-continue": "off",
     "no-debugger": "off",
     "no-else-return": "off",
     "no-multiple-empty-lines": "off",
@@ -29,6 +30,7 @@
     "operator-linebreak": "off",
     "padded-blocks": "off",
     "prefer-template": "off",
+    "space-infix-ops": "off",
     "spaced-comment": "off"
   }
 }

Makefile

@@ -5,7 +5,7 @@ node_modules:
 	npm install
 
 migrations: node_modules
-	node -e 'const { withDatabase, migrate } = require("./lib/model/database"); withDatabase(require("config").get("default.database"))(migrate);'
+	node -e 'const { withDatabase, migrate } = require("./lib/model/migrate"); withDatabase(require("config").get("default.database"))(migrate);'
 
 base: node_modules migrations
 

lib/bin/backup.js

@@ -24,7 +24,7 @@ const { initDrive, ensureDirectory, uploadFile, persistCredentials } = require('
 run(emailing('backupFailed', auditing('backup', async () => {
   // fetch backup config. fail early and silently unless it exists.
   let config;
-  try { config = await getConfiguration('backups.main'); } catch { return 'no backup configured'; }
+  try { config = await getConfiguration('backups.main'); } catch (_) { return 'no backup configured'; }
   const configValue = JSON.parse(config.value);
 
   // run the pgdump and encrypt it into a zipfile.

lib/bin/run-server.js

@@ -14,36 +14,39 @@ const { merge } = require('ramda');
 const config = require('config');
 const exit = require('express-graceful-exit');
 
+global.tap = (x) => { console.log(x); return x; }; // eslint-disable-line no-console
+
 ////////////////////////////////////////////////////////////////////////////////
 // CONTAINER SETUP
 
-// initialize our top-level static database instance.
-const { connect } = require('../model/database');
-const db = connect(config.get('default.database'));
+// initialize our slonik connection pool.
+const { slonikPool } = require('../external/slonik');
+const db = slonikPool(config.get('default.database'));
 
 // set up our mailer.
 const env = config.get('default.env');
-const { mailer } = require('../outbound/mail');
+const { mailer } = require('../external/mail');
 const mail = mailer(merge(config.get('default.email'), { env }));
 
 // get a google client.
-const googler = require('../outbound/google');
+const googler = require('../external/google');
 const google = googler(config.get('default.external.google'));
 
-// get a sentry and xlsform client, and a crypto module.
-const Sentry = require('../util/sentry').init(config.get('default.external.sentry'));
-const xlsform = require('../util/xlsform').init(config.get('default.xlsform'));
-const crypto = require('../util/crypto');
+// get a sentry and xlsform client, and a password module.
+const Sentry = require('../external/sentry').init(config.get('default.external.sentry'));
+const xlsform = require('../external/xlsform').init(config.get('default.xlsform'));
+const bcrypt = require('../util/crypto').password(require('bcrypt'));
 
 // get an Enketo client
-const enketo = require('../util/enketo').init(config.get('default.enketo'));
+const enketo = require('../external/enketo').init(config.get('default.enketo'));
 
 
 ////////////////////////////////////////////////////////////////////////////////
 // START HTTP SERVICE
 
 // initialize our container, then generate an http service out of it.
-const container = require('../model/package').withDefaults({ db, mail, env, google, Sentry, crypto, xlsform, enketo });
+const container = require('../model/container')
+  .withDefaults({ db, mail, env, google, Sentry, bcrypt, xlsform, enketo });
 const service = require('../http/service')(container);
 
 // insert the graceful exit middleware.
@@ -77,7 +80,7 @@ const term = () => {
   exit.gracefulExitHandler(service, server, {
     log: true,
     exitProcess: false,
-    callback: () => db.destroy(() => process.exit(0))
+    callback: () => db.end().then(() => process.exit(0))
   });
 };
 

lib/data/attachments.js

@@ -30,7 +30,9 @@ const streamAttachments = (inStream, decryptor) => {
   const writable = new Writable({
     objectMode: true,
     highWaterMark: 5, // the default is 16, we'll be a little more conservative.
-    write(att, _, done) {
+    write(x, _, done) {
+      const att = x.row;
+
       // this sanitization means that two filenames could end up identical.
       // luckily, this is not actually illegal in the zip spec; two files can live at precisely
       // the same location, and the conflict is dealt with interactively by the unzipping client.

lib/data/briefcase.js

@@ -245,10 +245,8 @@ const streamBriefcaseCsvs = (inStream, inFields, xmlFormId, decryptor, rootOnly
   let rootHeaderSent = false;
   const rootStream = new Transform({
     objectMode: true,
-    transform(row, _, done) {
+    transform(submission, _, done) {
       try {
-        const { submission } = row; // just save some syntax
-
         // send header if we have to. i wish there were a cleaner way to do this.
         if (rootHeaderSent === false) {
           this.push(rootHeader);
@@ -258,10 +256,11 @@ const streamBriefcaseCsvs = (inStream, inFields, xmlFormId, decryptor, rootOnly
         // if we have encData instead of xml, we must decrypt before we can read data.
         // the decryptor will return null if it does not have a decryption key for the
         // record. this is okay; we pass the null through and processRow deals with it.
+        const { encryption } = submission.aux;
         const xml =
-          (row.localKey == null) ? row.xml :
-          (row.encHasData === false) ? 'missing' : // eslint-disable-line indent
-          decryptor(row.encData, row.encKeyId, row.localKey, submission.instanceId, row.encIndex); // eslint-disable-line indent
+          (submission.def.localKey == null) ? submission.xml :
+          (encryption.encHasData === false) ? 'missing' : // eslint-disable-line indent
+          decryptor(encryption.encData, encryption.encKeyId, submission.def.localKey, submission.instanceId, encryption.encIndex); // eslint-disable-line indent
 
         // if something about the xml didn't work so well, we can figure out what
         // to say and bail out early.
@@ -270,13 +269,13 @@ const streamBriefcaseCsvs = (inStream, inFields, xmlFormId, decryptor, rootOnly
           (xml === null) ? 'not decrypted' : null; // eslint-disable-line indent
         if (status != null) {
           const result = [];
-          writeMetadata(result, rootMeta, submission, row.submitter, row.attachments, status);
+          writeMetadata(result, rootMeta, submission, submission.aux.submitter, submission.aux.attachment, status);
           return done(null, result);
         }
 
         // write the root row we get back from parsing the xml.
         processRow(xml, submission.instanceId, fields, rootOnly).then((result) => {
-          writeMetadata(result, rootMeta, submission, row.submitter, row.attachments);
+          writeMetadata(result, rootMeta, submission, submission.aux.submitter, submission.aux.attachment);
           done(null, result);
         }, done); // pass through errors.
       } catch (ex) { done(ex); }

lib/data/client-audits.js

@@ -29,6 +29,7 @@ for (const header of headers) headerLookup[header] = true;
 //
 // TODO: if the csv is ragged our behaviour is somewhat undefined.
 const parseClientAudits = (buffer) => {
+  const { ClientAudit } = require('../model/frames'); // TODO: better to eliminate the cyclic require.
   const parser = parse(buffer, parseOptions);
   const audits = [];
 
@@ -45,9 +46,9 @@ const parseClientAudits = (buffer) => {
     // and now set ourselves up to actually process each cell of each row.
     parser.on('data', (row) => {
       const audit = { remainder: {} };
-      audits.push(audit);
       for (let idx = 0; (idx < row.length) && (idx < names.length); idx += 1)
         (known[idx] ? audit : audit.remainder)[names[idx]] = row[idx];
+      audits.push(new ClientAudit(audit));
     });
   });
 
@@ -71,7 +72,9 @@ const streamClientAudits = (inStream, form) => {
   let first = true;
   const csvifier = new Transform({
     objectMode: true,
-    transform(data, _, done) {
+    transform(x, _, done) {
+      const data = x.row;
+
       // TODO: we do not currently try/catch this block because it feels low risk.
       // this may not actually be the case..
       if (first === true) {

lib/data/odata-filter.js

@@ -7,68 +7,60 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { identity } = require('ramda');
+const { sql } = require('slonik');
+const { raw } = require('slonik-sql-tag-raw');
 const odataParser = require('odata-v4-parser');
 const Problem = require('../util/problem');
 
 ////////////////////////////////////////
 // AST NODE TRANSFORMATION
 
 const extractFunctions = [ 'year', 'month', 'day', 'hour', 'minute', 'second' ];
-const methodCall = (fn, params, sql, bindings) => {
+const methodCall = (fn, params) => {
   // n.b. odata-v4-parser appears to already validate function name and arity.
   const lowerName = fn.toLowerCase();
-  if (extractFunctions.includes(lowerName)) {
-    sql.push(`extract(${lowerName} from `);
-    op(params[0], sql, bindings); // eslint-disable-line no-use-before-define
-    sql.push(')');
-  } else if (fn === 'now') {
-    sql.push('now()');
-  }
-};
-const binaryOp = (left, right, operator, sql, bindings) => {
-  sql.push('('); // always explicitly express the original AST op precedence for safety.
-  op(left, sql, bindings); // eslint-disable-line no-use-before-define
-  sql.push(` ${operator} `);
-  op(right, sql, bindings); // eslint-disable-line no-use-before-define
-  sql.push(')');
+  if (extractFunctions.includes(lowerName))
+    return sql`extract(${raw(lowerName)} from ${op(params[0])})`; // eslint-disable-line no-use-before-define
+  else if (fn === 'now')
+    return sql`now()`;
 };
-const op = (node, sql, bindings) => {
+const binaryOp = (left, right, operator) =>
+  // always use parens to ensure the original AST op precedence.
+  sql`(${op(left)} ${raw(operator)} ${op(right)})`; // eslint-disable-line no-use-before-define
+
+const op = (node) => {
   if (node.type === 'FirstMemberExpression') {
     if (node.raw === '__system/submissionDate') {
-      sql.push('??');
-      bindings.push('submissions.createdAt'); // TODO: HACK HACK
+      return sql.identifier([ 'submissions', 'createdAt' ]); // TODO: HACK HACK
     } else if (node.raw === '__system/submitterId') {
-      sql.push('??');
-      bindings.push('submissions.submitterId'); // TODO: HACK HACK
+      return sql.identifier([ 'submissions', 'submitterId' ]); // TODO: HACK HACK
+    } else if (node.raw === '__system/reviewStatus') {
+      return sql.identifier([ 'submissions', 'reviewStatus' ]); // TODO: HACK HACK
     } else {
       throw Problem.internal.unsupportedODataField({ at: node.position, text: node.raw });
     }
   } else if (node.type === 'Literal') {
-    sql.push('?');
-    bindings.push(node.raw);
+    return node.raw;
   } else if (node.type === 'MethodCallExpression') {
-    methodCall(node.value.method, node.value.parameters, sql, bindings);
+    return methodCall(node.value.method, node.value.parameters);
   } else if (node.type === 'EqualsExpression') {
-    binaryOp(node.value.left, node.value.right, '=', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, '=');
   } else if (node.type === 'NotEqualsExpression') {
-    binaryOp(node.value.left, node.value.right, '!=', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, '!=');
   } else if (node.type === 'LesserThanExpression') {
-    binaryOp(node.value.left, node.value.right, '<', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, '<');
   } else if (node.type === 'LesserOrEqualsExpression') {
-    binaryOp(node.value.left, node.value.right, '<=', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, '<=');
   } else if (node.type === 'GreaterThanExpression') {
-    binaryOp(node.value.left, node.value.right, '>', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, '>');
   } else if (node.type === 'GreaterOrEqualsExpression') {
-    binaryOp(node.value.left, node.value.right, '>=', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, '>=');
   } else if (node.type === 'AndExpression') {
-    binaryOp(node.value.left, node.value.right, 'and', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, 'and');
   } else if (node.type === 'OrExpression') {
-    binaryOp(node.value.left, node.value.right, 'or', sql, bindings);
+    return binaryOp(node.value.left, node.value.right, 'or');
   } else if (node.type === 'NotExpression') {
-    sql.push('(not ');
-    op(node.value, sql, bindings);
-    sql.push(')');
+    return sql`(not ${op(node.value)})`;
   } else {
     throw Problem.internal.unsupportedODataExpression({ at: node.position, type: node.type, text: node.raw });
   }
@@ -77,20 +69,15 @@ const op = (node, sql, bindings) => {
 ////////////////////////////////////////
 // MAIN ENTRY POINT
 
-const applyODataFilter = (expr) => {
-  if (expr == null) return identity;
+const odataFilter = (expr) => {
+  if (expr == null) return sql`true`;
 
-  return (db) => {
-    let ast; // still hate this.
-    try { ast = odataParser.filter(expr); } // eslint-disable-line brace-style
-    catch (ex) { throw Problem.user.unparseableODataExpression({ reason: ex.message }); }
+  let ast; // still hate this.
+  try { ast = odataParser.filter(expr); } // eslint-disable-line brace-style
+  catch (ex) { throw Problem.user.unparseableODataExpression({ reason: ex.message }); }
 
-    const sql = [];
-    const bindings = [];
-    op(ast, sql, bindings);
-    return db.whereRaw(sql.join(''), bindings);
-  };
+  return op(ast);
 };
 
-module.exports = { applyODataFilter };
+module.exports = { odataFilter };
 

lib/data/json.js → lib/data/odata.js

@@ -94,7 +94,10 @@ const navigationLink = (schemaStack, instanceId, slicer = identity) => {
 
 // manually extracts fields from a row into a js obj given a schema fieldlist.
 // NOTE: expects extended submission metadata, for exporting submitter metadata!
-const submissionToOData = (fields, table, { xml, encHasData, submission, submitter, attachments, localKey }, options = {}) => new Promise((resolve) => {
+const submissionToOData = (fields, table, submission, options = {}) => new Promise((resolve) => {
+  const { submitter, attachment } = submission.aux;
+  const encHasData = (submission.aux.encryption != null) ? submission.aux.encryption.encHasData : false;
+
   // we use SchemaStack to navigate the tree.
   const schemaStack = new SchemaStack(fields);
 
@@ -105,7 +108,7 @@ const submissionToOData = (fields, table, { xml, encHasData, submission, submitt
 
   // if we have an encDataAttachmentName, we must be an encrypted row. flag that
   // and we'll deal with it pretty much immediately.
-  const encrypted = (localKey != null);
+  const encrypted = (submission.def.localKey != null);
 
   // we always build a tree structure from root to track where we are, even if
   // we are actually outputting some subtrees of it.
@@ -128,8 +131,8 @@ const submissionToOData = (fields, table, { xml, encHasData, submission, submitt
         submissionDate: submission.createdAt,
         submitterId,
         submitterName: submitter.displayName || null,
-        attachmentsPresent: attachments.present || 0,
-        attachmentsExpected: attachments.expected || 0,
+        attachmentsPresent: attachment.present || 0,
+        attachmentsExpected: attachment.expected || 0,
         status: encrypted ? (encHasData ? 'NotDecrypted' : 'MissingEncryptedFormData') : null
       }
     });
@@ -311,7 +314,7 @@ const submissionToOData = (fields, table, { xml, encHasData, submission, submitt
     }
   }, { xmlMode: true, decodeEntities: true });
 
-  parser.write(xml);
+  parser.write(submission.xml);
   parser.end();
 });
 

lib/util/enketo.js → lib/external/enketo.js

@@ -16,8 +16,8 @@ const http = require('http');
 const https = require('https');
 const path = require('path');
 const querystring = require('querystring');
-const { isBlank } = require('./util');
-const Problem = require('./problem');
+const { isBlank } = require('../util/util');
+const Problem = require('../util/problem');
 
 const mock = {
   create: () => Promise.reject(Problem.internal.enketoNotConfigured()),