Merge pull request #989 from getodk/faster-enketo-ids

Request Enketo IDs during request when form is created or published

lib/external/enketo.js

@@ -7,11 +7,6 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-// here we support previewing forms (and hopefully other things in the future)
-// by providing a very thin wrapper around node http requests to Enketo. given
-// an OpenRosa endpoint to access a form's xml and its media, Enketo should
-// return a preview url, which we then pass untouched to the client.
-
 const http = require('http');
 const https = require('https');
 const { posix } = require('path');
@@ -21,14 +16,10 @@ const { url } = require('../util/http');
 const { isBlank } = require('../util/util');
 const Problem = require('../util/problem');
 
-const mock = {
-  create: () => Promise.reject(Problem.internal.enketoNotConfigured()),
-  createOnceToken: () => Promise.reject(Problem.internal.enketoNotConfigured()),
-  edit: () => Promise.reject(Problem.internal.enketoNotConfigured())
-};
-
-const enketo = (hostname, pathname, port, protocol, apiKey) => {
-  const _enketo = (apiPath, responseField, token, postData) => new Promise((resolve, reject) => {
+// Returns a very thin wrapper around Node HTTP requests for sending requests to
+// Enketo. The methods of the object can be used to send requests to Enketo.
+const _init = (hostname, pathname, port, protocol, apiKey) => {
+  const enketoRequest = (apiPath, token, postData) => new Promise((resolve, reject) => {
     const path = posix.join(pathname, apiPath);
     const auth = `${apiKey}:`;
     const headers = {
@@ -51,7 +42,7 @@ const enketo = (hostname, pathname, port, protocol, apiKey) => {
           return reject(Problem.user.enketoEditRateLimit());
         if ((res.statusCode !== 200) && (res.statusCode !== 201))
           return reject(Problem.internal.enketoUnexpectedResponse('wrong status code'));
-        resolve(body[responseField]);
+        resolve(body);
       });
     });
 
@@ -61,29 +52,37 @@ const enketo = (hostname, pathname, port, protocol, apiKey) => {
     req.end();
   });
 
-  const _create = (apiPath, responseField) => (openRosaUrl, xmlFormId, token) =>
-    _enketo(apiPath, responseField, token, querystring.stringify({ server_url: openRosaUrl, form_id: xmlFormId }))
-      .then((result) => {
-        const match = /\/([:a-z0-9]+)$/i.exec(result);
-        if (match == null) throw Problem.internal.enketoUnexpectedResponse(`Could not parse token from enketo response url: ${result}`);
-        return match[1];
-      });
+  // openRosaUrl is the OpenRosa endpoint for Enketo to use to access the form's
+  // XML and attachments.
+  const create = async (openRosaUrl, xmlFormId, token) => {
+    const body = await enketoRequest('/api/v2/survey/all', token, querystring.stringify({
+      server_url: openRosaUrl,
+      form_id: xmlFormId
+    }));
 
-  const _edit = (apiPath, responseField) => (openRosaUrl, domain, form, logicalId, submissionDef, attachments, token) => {
+    // Parse enketoOnceId from single_once_url.
+    const match = /\/([:a-z0-9]+)$/i.exec(body.single_once_url);
+    if (match == null) throw Problem.internal.enketoUnexpectedResponse(`Could not parse token from single_once_url: ${body.single_once_url}`);
+    const enketoOnceId = match[1];
+
+    return { enketoId: body.enketo_id, enketoOnceId };
+  };
+
+  const edit = (openRosaUrl, domain, form, logicalId, submissionDef, attachments, token) => {
     const attsMap = {};
     for (const att of attachments)
       if (att.blobId != null)
         attsMap[url`instance_attachments[${att.name}]`] = domain + url`/v1/projects/${form.projectId}/forms/${form.xmlFormId}/submissions/${logicalId}/versions/${submissionDef.instanceId}/attachments/${att.name}`;
 
-    return _enketo(apiPath, responseField, token, querystring.stringify({
+    return enketoRequest('api/v2/instance', token, querystring.stringify({
       server_url: openRosaUrl,
       form_id: form.xmlFormId,
       instance: submissionDef.xml,
       instance_id: submissionDef.instanceId,
       ...attsMap,
       return_url: domain + url`/#/projects/${form.projectId}/forms/${form.xmlFormId}/submissions/${logicalId}`
     }))
-      .then((enketoUrlStr) => {
+      .then(({ edit_url: enketoUrlStr }) => {
         // need to override proto/host/port with our own.
         const enketoUrl = new URL(enketoUrlStr);
         const ownUrl = new URL(domain);
@@ -94,16 +93,16 @@ const enketo = (hostname, pathname, port, protocol, apiKey) => {
       });
   };
 
-  return {
-    create: _create('api/v2/survey', 'url'),
-    createOnceToken: _create('api/v2/survey/single/once', 'single_once_url'),
-    edit: _edit('api/v2/instance', 'edit_url')
-  };
+  return { create, edit };
 };
 
+const mock = {
+  create: () => Promise.reject(Problem.internal.enketoNotConfigured()),
+  edit: () => Promise.reject(Problem.internal.enketoNotConfigured())
+};
 
-// sorts through config and returns an object containing stubs or real functions for Enketo integration.
-// (okay, right now it's just one function)
+// sorts through config and returns an object containing either stubs or real
+// functions for Enketo integration.
 const init = (config) => {
   if (config == null) return mock;
   if (isBlank(config.url) || isBlank(config.apiKey)) return mock;
@@ -115,7 +114,7 @@ const init = (config) => {
     else throw ex;
   }
   const { hostname, pathname, port, protocol } = parsedUrl;
-  return enketo(hostname, pathname, port, protocol, config.apiKey);
+  return _init(hostname, pathname, port, protocol, config.apiKey);
 };
 
 module.exports = { init };

lib/model/frames/form.js

@@ -76,14 +76,18 @@ class Form extends Frame.define(
       .then((partial) => partial.with({ key: Option.of(key) }));
   }
 
+  _enketoIdForApi() {
+    if (this.def == null) return null;
+    if (this.def.id === this.draftDefId) return this.def.enketoId;
+    if (this.def.id === this.currentDefId) return this.enketoId;
+    return null;
+  }
+
   forApi() {
-    /* eslint-disable indent */
-    const enketoId =
-      (this.def.id === this.draftDefId) ? this.def.enketoId
-      : (this.def.id === this.currentDefId) ? this.enketoId
+    const enketoId = this._enketoIdForApi();
+    const enketoOnceId = this.def != null && this.def.id === this.currentDefId
+      ? this.enketoOnceId
       : null;
-    const enketoOnceId = (this.def.id === this.currentDefId) ? this.enketoOnceId : null;
-    /* eslint-enable indent */
 
     // Include deletedAt in response only if it is not null (most likely on resources about soft-deleted forms)
     // and also include the numeric form id (used to restore)

lib/model/query/assignments.js

@@ -64,7 +64,7 @@ select ${fields} from assignments
   where ${equals(options.condition)}`);
 const getByActeeId = (acteeId, options = QueryOptions.none) => ({ all }) =>
   _get(all, options.withCondition({ 'assignments.acteeId': acteeId }));
-const getByActeeAndRoleId = (acteeId, roleId, options) => ({ all }) =>
+const getByActeeAndRoleId = (acteeId, roleId, options = QueryOptions.none) => ({ all }) =>
   _get(all, options.withCondition({ 'assignments.acteeId': acteeId, roleId }));
 
 const _getForForms = extender(Assignment, Assignment.FormSummary)(Actor)((fields, extend, options) => sql`

lib/model/query/audits.js

@@ -94,7 +94,8 @@ const get = (options = QueryOptions.none) => ({ all }) =>
 
     // TODO: better if we don't have to loop over all this data twice.
     return rows.map((row) => {
-      const form = row.aux.form.map((f) => f.withAux('def', row.aux.def));
+      const form = row.aux.form.map((f) =>
+        row.aux.def.map((def) => f.withAux('def', def)).orElse(f));
       const actees = [ row.aux.acteeActor, form, row.aux.project, row.aux.dataset, row.aux.actee ];
       return new Audit(row, { actor: row.aux.actor, actee: Option.firstDefined(actees) });
     });

lib/model/query/forms.js

@@ -41,33 +41,77 @@ const fromXls = (stream, contentType, formIdFallback, ignoreWarnings) => ({ Blob
         .then(([ partial, xlsBlobId ]) => partial.withAux('xls', { xlsBlobId, itemsets }));
     });
 
+
+////////////////////////////////////////////////////////////////////////////////
+// PUSHING TO ENKETO
+
+// Time-bounds a request from enketo.create(). If the request times out or
+// results in an error, then an empty object is returned.
+const timeboundEnketo = (request, bound) =>
+  (bound != null ? timebound(request, bound).catch(() => ({})) : request);
+
+// Accepts either a Form or an object with a top-level draftToken property. Also
+// accepts an optional bound on the amount of time for the request to Enketo to
+// complete (in seconds). If a bound is specified, and the request to Enketo
+// times out or results in an error, then `null` is returned.
+const pushDraftToEnketo = ({ projectId, xmlFormId, def, draftToken = def?.draftToken }, bound = undefined) => async ({ enketo, env }) => {
+  const encodedFormId = encodeURIComponent(xmlFormId);
+  const path = `${env.domain}/v1/test/${draftToken}/projects/${projectId}/forms/${encodedFormId}/draft`;
+  const { enketoId } = await timeboundEnketo(enketo.create(path, xmlFormId), bound);
+  // Return `null` if enketoId is `undefined`.
+  return enketoId ?? null;
+};
+
+// Pushes a form that is published or about to be published to Enketo. Accepts
+// either a Form or a Form-like object. Also accepts an optional bound on the
+// amount of time for the request to Enketo to complete (in seconds). If a bound
+// is specified, and the request to Enketo times out or results in an error,
+// then an empty object is returned.
+const pushFormToEnketo = ({ projectId, xmlFormId, acteeId }, bound = undefined) => async ({ Actors, Assignments, Sessions, enketo, env }) => {
+  // Generate a single use actor that grants Enketo access just to this form for
+  // just long enough for it to pull the information it needs.
+  const expiresAt = new Date();
+  expiresAt.setMinutes(expiresAt.getMinutes() + 15);
+  const actor = await Actors.create(new Actor({
+    type: 'singleUse',
+    expiresAt,
+    displayName: `Enketo sync token for ${acteeId}`
+  }));
+  await Assignments.grantSystem(actor, 'formview', acteeId);
+  const { token } = await Sessions.create(actor, expiresAt);
+
+  const path = `${env.domain}/v1/projects/${projectId}`;
+  return timeboundEnketo(enketo.create(path, xmlFormId, token), bound);
+};
+
+
 ////////////////////////////////////////////////////////////////////////////////
 // CREATING NEW FORMS
 
-const _createNew = (form, def, project, publish) => ({ oneFirst, Actees, Forms }) =>
-  Actees.provision('form', project)
-    .then((actee) => oneFirst(sql`
+const _createNew = (form, def, project, publish) => ({ oneFirst, Forms }) =>
+  oneFirst(sql`
 with sch as
   (insert into form_schemas (id)
     values (default)
     returning *),
 def as
-  (insert into form_defs ("formId", "schemaId", xml, name, hash, sha, sha256, version, "keyId", "xlsBlobId", "draftToken", "createdAt", "publishedAt")
-  select nextval(pg_get_serial_sequence('forms', 'id')), sch.id, ${form.xml}, ${def.name}, ${def.hash}, ${def.sha}, ${def.sha256}, ${def.version}, ${def.keyId}, ${form.xls.xlsBlobId || null}, ${(publish !== true) ? generateToken() : null}, clock_timestamp(), ${(publish === true) ? sql`clock_timestamp()` : null}
+  (insert into form_defs ("formId", "schemaId", xml, name, hash, sha, sha256, version, "keyId", "xlsBlobId", "draftToken", "enketoId", "createdAt", "publishedAt")
+  select nextval(pg_get_serial_sequence('forms', 'id')), sch.id, ${form.xml}, ${def.name}, ${def.hash}, ${def.sha}, ${def.sha256}, ${def.version}, ${def.keyId}, ${form.xls.xlsBlobId || null}, ${def.draftToken || null}, ${def.enketoId || null}, clock_timestamp(), ${(publish === true) ? sql`clock_timestamp()` : null}
     from sch
   returning *),
 form as
-  (insert into forms (id, "xmlFormId", state, "projectId", ${sql.identifier([ (publish === true) ? 'currentDefId' : 'draftDefId' ])}, "acteeId", "createdAt")
-  select def."formId", ${form.xmlFormId}, ${form.state || 'open'}, ${project.id}, def.id, ${actee.id}, def."createdAt" from def
+  (insert into forms (id, "xmlFormId", state, "projectId", ${sql.identifier([ (publish === true) ? 'currentDefId' : 'draftDefId' ])}, "acteeId", "enketoId", "enketoOnceId", "createdAt")
+  select def."formId", ${form.xmlFormId}, ${form.state || 'open'}, ${project.id}, def.id, ${form.acteeId}, ${form.enketoId || null}, ${form.enketoOnceId || null}, def."createdAt" from def
   returning forms.*)
-select id from form`))
+select id from form`)
     .then(() => Forms.getByProjectAndXmlFormId(project.id, form.xmlFormId, false,
       (publish === true) ? undefined : Form.DraftVersion))
     .then((option) => option.get());
 
-const createNew = (partial, project, publish = false) => async ({ run, Datasets, FormAttachments, Forms, Keys }) => {
+const createNew = (partial, project, publish = false) => async ({ run, Actees, Datasets, FormAttachments, Forms, Keys }) => {
   // Check encryption keys before proceeding
-  const keyId = await partial.aux.key.map(Keys.ensure).orElse(resolve(null));
+  const defData = {};
+  defData.keyId = await partial.aux.key.map(Keys.ensure).orElse(resolve(null));
 
   // Parse form XML for fields and entity/dataset definitions
   const [fields, datasetName] = await Promise.all([
@@ -82,8 +126,32 @@ const createNew = (partial, project, publish = false) => async ({ run, Datasets,
   await Forms.checkDeletedForms(partial.xmlFormId, project.id);
   await Forms.rejectIfWarnings();
 
+  const formData = {};
+  formData.acteeId = (await Actees.provision('form', project)).id;
+
+  // We will try to push to Enketo. If doing so fails or is too slow, then the
+  // worker will try again later.
+  if (publish !== true) {
+    defData.draftToken = generateToken();
+    defData.enketoId = await Forms.pushDraftToEnketo(
+      { projectId: project.id, xmlFormId: partial.xmlFormId, draftToken: defData.draftToken },
+      0.5
+    );
+  } else {
+    const enketoIds = await Forms.pushFormToEnketo(
+      { projectId: project.id, xmlFormId: partial.xmlFormId, acteeId: formData.acteeId },
+      0.5
+    );
+    Object.assign(formData, enketoIds);
+  }
+
   // Save form
-  const savedForm = await Forms._createNew(partial, partial.def.with({ keyId }), project, publish);
+  const savedForm = await Forms._createNew(
+    partial.with(formData),
+    partial.def.with(defData),
+    project,
+    publish
+  );
 
   // Prepare the form fields
   const ids = { formId: savedForm.id, schemaId: savedForm.def.schemaId };
@@ -120,7 +188,7 @@ createNew.audit.withResult = true;
 // Inserts a new form def into the database for createVersion() below, setting
 // fields on the def according to whether the def will be the current def or the
 // draft def.
-const _createNewDef = (partial, form, publish, data) => async ({ one, enketo, env }) => {
+const _createNewDef = (partial, form, publish, data) => async ({ one, Forms }) => {
   const insertWith = (moreData) => one(insert(partial.def.with({
     formId: form.id,
     xlsBlobId: partial.xls.xlsBlobId,
@@ -135,14 +203,12 @@ const _createNewDef = (partial, form, publish, data) => async ({ one, enketo, en
   // generate a draft token and enketoId.
   if (form.def.id == null || form.def.id !== form.draftDefId) {
     const draftToken = generateToken();
-
     // Try to push the draft to Enketo. If doing so fails or is too slow, then
     // the worker will try again later.
-    const encodedId = encodeURIComponent(form.xmlFormId);
-    const path = `/v1/test/${draftToken}/projects/${form.projectId}/forms/${encodedId}/draft`;
-    const request = enketo.create(`${env.domain}${path}`, form.xmlFormId);
-    const enketoId = await timebound(request, 0.5).catch(() => null);
-
+    const enketoId = await Forms.pushDraftToEnketo(
+      { projectId: form.projectId, xmlFormId: form.xmlFormId, draftToken },
+      0.5
+    );
     return insertWith({ draftToken, enketoId });
   }
 
@@ -253,12 +319,18 @@ createVersion.audit.withResult = true;
 
 // TODO: we need to make more explicit what .def actually represents throughout.
 // for now, enforce an extra check here just in case.
-const publish = (form) => ({ Forms, Datasets }) => {
+const publish = (form) => async ({ Forms, Datasets }) => {
   if (form.draftDefId !== form.def.id) throw Problem.internal.unknown();
 
+  // Try to push the form to Enketo if it hasn't been pushed already. If doing
+  // so fails or is too slow, then the worker will try again later.
+  const enketoIds = form.enketoId == null || form.enketoOnceId == null
+    ? await Forms.pushFormToEnketo(form, 0.5)
+    : {};
+
   const publishedAt = (new Date()).toISOString();
   return Promise.all([
-    Forms._update(form, { currentDefId: form.draftDefId, draftDefId: null }),
+    Forms._update(form, { currentDefId: form.draftDefId, draftDefId: null, ...enketoIds }),
     Forms._updateDef(form.def, { draftToken: null, enketoId: null, publishedAt }),
     Datasets.publishIfExists(form.def.id, publishedAt)
   ])
@@ -718,7 +790,7 @@ const _newSchema = () => ({ one }) =>
     .then((s) => s.id);
 
 module.exports = {
-  fromXls, _createNew, createNew, _createNewDef, createVersion,
+  fromXls, pushDraftToEnketo, pushFormToEnketo, _createNew, createNew, _createNewDef, createVersion,
   publish, clearDraft,
   _update, update, _updateDef, del, restore, purge,
   clearUnneededDrafts,

lib/worker/form.js

@@ -7,9 +7,9 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { Actor, Form } = require('../model/frames');
+const { Form } = require('../model/frames');
 
-const pushDraftToEnketo = ({ Forms, enketo, env }, event) =>
+const pushDraftToEnketo = ({ Forms }, event) =>
   Forms.getByActeeIdForUpdate(event.acteeId, undefined, Form.DraftVersion)
     .then((maybeForm) => maybeForm.map((form) => {
       // if there was no draft or this form isn't the draft anymore just bail.
@@ -23,31 +23,19 @@ const pushDraftToEnketo = ({ Forms, enketo, env }, event) =>
       // and wrong. still want to log a fail but bail early.
       if (form.def.draftToken == null) throw new Error('Could not find a draft token!');
 
-      const path = `${env.domain}/v1/test/${form.def.draftToken}/projects/${form.projectId}/forms/${encodeURIComponent(form.xmlFormId)}/draft`;
-      return enketo.create(path, form.xmlFormId)
+      return Forms.pushDraftToEnketo(form)
         .then((enketoId) => Forms._updateDef(form.def, new Form.Def({ enketoId })));
     }).orNull());
 
-const pushFormToEnketo = ({ Actors, Assignments, Forms, Sessions, enketo, env }, event) =>
+const pushFormToEnketo = ({ Forms }, event) =>
   Forms.getByActeeIdForUpdate(event.acteeId)
     .then((maybeForm) => maybeForm.map((form) => {
       // if this form already has both enketo ids then we have no work to do here.
       // if the form is updated enketo will see the difference and update.
       if ((form.enketoId != null) && (form.enketoOnceId != null)) return;
 
-      // generate a single use actor that grants enketo access just to this
-      // form for just long enough for it to pull the information it needs.
-      const path = `${env.domain}/v1/projects/${form.projectId}`;
-      const expiresAt = new Date();
-      expiresAt.setMinutes(expiresAt.getMinutes() + 15);
-      const displayName = `Enketo sync token for ${form.acteeId}`;
-      return Actors.create(new Actor({ type: 'singleUse', expiresAt, displayName }))
-        .then((actor) => Assignments.grantSystem(actor, 'formview', form)
-          .then(() => Sessions.create(actor, expiresAt)))
-        .then(({ token }) => enketo.create(path, form.xmlFormId, token)
-          .then((enketoId) => enketo.createOnceToken(path, form.xmlFormId, token)
-            .then((enketoOnceId) =>
-              Forms.update(form, new Form({ enketoId, enketoOnceId })))));
+      return Forms.pushFormToEnketo(form)
+        .then((enketoIds) => Forms.update(form, new Form(enketoIds)));
     }).orNull());
 
 const create = pushDraftToEnketo;