diff --git a/lib/external/enketo.js b/lib/external/enketo.js index 6b1da0c14..a24500677 100644 --- a/lib/external/enketo.js +++ b/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,21 +52,29 @@ 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, @@ -83,7 +82,7 @@ const enketo = (hostname, pathname, port, protocol, apiKey) => { ...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 }; diff --git a/lib/model/frames/form.js b/lib/model/frames/form.js index 63dad8abd..c8dece8e8 100644 --- a/lib/model/frames/form.js +++ b/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) diff --git a/lib/model/query/assignments.js b/lib/model/query/assignments.js index 7a13125a6..c3ec33db2 100644 --- a/lib/model/query/assignments.js +++ b/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` diff --git a/lib/model/query/audits.js b/lib/model/query/audits.js index 43bcd92d6..24ae80cb8 100644 --- a/lib/model/query/audits.js +++ b/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) }); }); diff --git a/lib/model/query/forms.js b/lib/model/query/forms.js index 22634ead3..11691ab86 100644 --- a/lib/model/query/forms.js +++ b/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) ]) @@ -712,7 +784,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, diff --git a/lib/worker/form.js b/lib/worker/form.js index 93c17da56..c08372f7c 100644 --- a/lib/worker/form.js +++ b/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; diff --git a/test/integration/api/audits.js b/test/integration/api/audits.js index b0f381d3a..5131cfe78 100644 --- a/test/integration/api/audits.js +++ b/test/integration/api/audits.js @@ -6,6 +6,9 @@ const { testService } = require('../setup'); const testData = require('../../data/xml'); const { exhaust } = require(appRoot + '/lib/worker/worker'); +const assertAuditActions = (audits, expected) => { + audits.map(a => a.action).should.deepEqual(expected); +}; const submitThree = (asAlice) => asAlice.post('/v1/projects/1/forms/simple/submissions') .send(testData.instances.simple.one) @@ -45,7 +48,7 @@ describe('/audits', () => { Users.getByEmail('david@getodk.org').then((o) => o.get()) ])) .then(([ audits, project, alice, david ]) => { - assertAuditActions(audits, [ // eslint-disable-line no-use-before-define + assertAuditActions(audits, [ 'user.create', 'project.update', 'project.create', @@ -105,7 +108,7 @@ describe('/audits', () => { Users.getByEmail('david@getodk.org').then((o) => o.get()) ])) .then(([ audits, [ project, form ], alice, david ]) => { - assertAuditActions(audits, [ // eslint-disable-line no-use-before-define + assertAuditActions(audits, [ 'user.create', 'form.update.publish', 'form.create', @@ -158,6 +161,25 @@ describe('/audits', () => { audits[4].loggedAt.should.be.a.recentIsoDate(); }))))); + it('should return Enketo IDs for form', testService(async (service) => { + const asAlice = await service.login('alice'); + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + const { body: audits } = await asAlice.get('/v1/audits') + .set('X-Extended-Metadata', true) + .expect(200); + assertAuditActions(audits, [ + 'form.update.publish', + 'form.create', + 'user.session.create' + ]); + const form = audits[0].actee; + form.enketoId.should.equal('::abcdefgh'); + form.enketoOnceId.should.equal('::::abcdefgh'); + })); + it('should not expand actor if there is no actor', testService((service, { run }) => run(sql`insert into audits (action, "loggedAt") values ('analytics', now())`) .then(() => service.login('alice', (asAlice) => @@ -234,7 +256,7 @@ describe('/audits', () => { .then(() => asAlice.get('/v1/audits?action=user') .expect(200) .then(({ body }) => { - assertAuditActions(body, [ // eslint-disable-line no-use-before-define + assertAuditActions(body, [ 'user.delete', 'user.assignment.delete', 'user.assignment.create', @@ -732,7 +754,3 @@ describe('/audits', () => { }); }); }); - -function assertAuditActions(audits, expected) { - audits.map(a => a.action).should.deepEqual(expected); -} diff --git a/test/integration/api/forms/draft.js b/test/integration/api/forms/draft.js index b94f57495..7f371cf9a 100644 --- a/test/integration/api/forms/draft.js +++ b/test/integration/api/forms/draft.js @@ -84,7 +84,7 @@ describe('api: /projects/:id/forms (drafts)', () => { it('should request an enketoId while setting a new draft', testService(async (service, { env }) => { const asAlice = await service.login('alice'); - global.enketo.token = '::ijklmnop'; + global.enketo.enketoId = '::ijklmnop'; await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200); global.enketo.callCount.should.equal(1); global.enketo.receivedUrl.startsWith(env.domain).should.be.true(); @@ -101,10 +101,10 @@ describe('api: /projects/:id/forms (drafts)', () => { await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200); await asAlice.post('/v1/projects/1/forms/simple/draft/publish?version=two') .expect(200); - global.enketo.callCount.should.equal(1); - global.enketo.token = '::ijklmnop'; - await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200); global.enketo.callCount.should.equal(2); + global.enketo.enketoId = '::ijklmnop'; + await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200); + global.enketo.callCount.should.equal(3); global.enketo.receivedUrl.startsWith(env.domain).should.be.true(); const match = global.enketo.receivedUrl.match(/\/v1\/test\/([a-z0-9$!]{64})\/projects\/1\/forms\/simple\/draft$/i); should.exist(match); @@ -137,7 +137,7 @@ describe('api: /projects/:id/forms (drafts)', () => { global.enketo.state = 'error'; await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200); global.enketo.callCount.should.equal(1); - global.enketo.token = '::ijklmnop'; + global.enketo.enketoId = '::ijklmnop'; await exhaust(container); global.enketo.callCount.should.equal(2); global.enketo.receivedUrl.startsWith(container.env.domain).should.be.true(); @@ -158,15 +158,14 @@ describe('api: /projects/:id/forms (drafts)', () => { global.enketo.callCount.should.equal(1); })); - it('should manage draft/published enketo tokens separately', testService((service, container) => + it('should manage draft/published enketo tokens separately', testService((service) => service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms?publish=true') .set('Content-Type', 'application/xml') .send(testData.forms.simple2) .expect(200) - .then(() => exhaust(container)) .then(() => { - global.enketo.token = '::ijklmnop'; + global.enketo.enketoId = '::ijklmnop'; return asAlice.post('/v1/projects/1/forms/simple2/draft') .expect(200) .then(() => Promise.all([ @@ -898,15 +897,14 @@ describe('api: /projects/:id/forms (drafts)', () => { body.lastSubmission.should.be.a.recentIsoDate(); }))))); - it('should return the correct enketoId with extended draft', testService((service, container) => + it('should return the correct enketoId with extended draft', testService((service) => service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms?publish=true') .set('Content-Type', 'application/xml') .send(testData.forms.simple2) .expect(200) - .then(() => exhaust(container)) .then(() => { - global.enketo.token = '::ijklmnop'; + global.enketo.enketoId = '::ijklmnop'; return asAlice.post('/v1/projects/1/forms/simple2/draft') .expect(200) .then(() => asAlice.get('/v1/projects/1/forms/simple2/draft') @@ -1144,6 +1142,152 @@ describe('api: /projects/:id/forms (drafts)', () => { ]); }))))); + it('should request Enketo IDs when publishing for first time', testService(async (service, { env }) => { + const asAlice = await service.login('alice'); + + // Create a draft form. + global.enketo.state = 'error'; + const { body: draft } = await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + global.enketo.callCount.should.equal(1); + should.not.exist(draft.enketoId); + should.not.exist(draft.enketoOnceId); + + // Publish. + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200); + global.enketo.callCount.should.equal(2); + global.enketo.receivedUrl.should.equal(`${env.domain}/v1/projects/1`); + const { body: form } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + form.enketoId.should.equal('::abcdefgh'); + form.enketoOnceId.should.equal('::::abcdefgh'); + })); + + it('should return with success even if request to Enketo fails', testService(async (service) => { + const asAlice = await service.login('alice'); + await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + global.enketo.state = 'error'; + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200); + const { body: form } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + should.not.exist(form.enketoId); + should.not.exist(form.enketoOnceId); + })); + + it('should stop waiting for Enketo after 0.5 seconds @slow', testService(async (service) => { + const asAlice = await service.login('alice'); + await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + global.enketo.wait = (f) => { setTimeout(f, 501); }; + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200); + const { body: form } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + should.not.exist(form.enketoId); + should.not.exist(form.enketoOnceId); + })); + + it('should request Enketo IDs when republishing if they are missing', testService(async (service, { env }) => { + const asAlice = await service.login('alice'); + + // First publish + await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + global.enketo.state = 'error'; + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200); + const { body: v1 } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + should.not.exist(v1.enketoId); + should.not.exist(v1.enketoOnceId); + + // Republish + await asAlice.post('/v1/projects/1/forms/simple2/draft').expect(200); + global.enketo.callCount.should.equal(3); + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish?version=new') + .expect(200); + global.enketo.callCount.should.equal(4); + global.enketo.receivedUrl.should.equal(`${env.domain}/v1/projects/1`); + const { body: v2 } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + v2.enketoId.should.equal('::abcdefgh'); + v2.enketoOnceId.should.equal('::::abcdefgh'); + })); + + it('should not request Enketo IDs when republishing if they are present', testService(async (service) => { + const asAlice = await service.login('alice'); + + // First publish + await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200); + + // Republish + await asAlice.post('/v1/projects/1/forms/simple2/draft').expect(200); + global.enketo.callCount.should.equal(3); + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish?version=new') + .expect(200); + global.enketo.callCount.should.equal(3); + const { body: form } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + form.enketoId.should.equal('::abcdefgh'); + form.enketoOnceId.should.equal('::::abcdefgh'); + })); + + it('should request Enketo IDs from worker if request from endpoint fails', testService(async (service, container) => { + const asAlice = await service.login('alice'); + + // First request to Enketo, from endpoint + await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + global.enketo.state = 'error'; + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200); + const { body: beforeWorker } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + should.not.exist(beforeWorker.enketoId); + should.not.exist(beforeWorker.enketoOnceId); + + // Second request, from worker + global.enketo.callCount.should.equal(2); + await exhaust(container); + global.enketo.callCount.should.equal(3); + global.enketo.receivedUrl.should.equal(`${container.env.domain}/v1/projects/1`); + const { body: afterWorker } = await asAlice.get('/v1/projects/1/forms/simple2') + .expect(200); + afterWorker.enketoId.should.equal('::abcdefgh'); + afterWorker.enketoOnceId.should.equal('::::abcdefgh'); + })); + + it('should not request Enketo IDs from worker if request from endpoint succeeds', testService(async (service, container) => { + const asAlice = await service.login('alice'); + await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200); + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200); + global.enketo.callCount.should.equal(2); + await exhaust(container); + global.enketo.callCount.should.equal(2); + })); + it('should log the action in the audit log', testService((service, { Forms }) => service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms/simple/draft') diff --git a/test/integration/api/forms/forms.js b/test/integration/api/forms/forms.js index def7b9155..4486d9111 100644 --- a/test/integration/api/forms/forms.js +++ b/test/integration/api/forms/forms.js @@ -758,18 +758,16 @@ describe('api: /projects/:id/forms (create, read, update)', () => { body.submissions.should.equal(0); }))))); - it('should return the correct enketoId', testService((service, container) => + it('should return the correct enketoId', testService((service) => service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms?publish=true') .set('Content-Type', 'application/xml') .send(testData.forms.simple2) .expect(200) - .then(() => exhaust(container)) .then(() => { - global.enketo.token = '::ijklmnop'; + global.enketo.enketoId = '::ijklmnop'; return asAlice.post('/v1/projects/1/forms/simple2/draft') .expect(200) - .then(() => exhaust(container)) .then(() => asAlice.get('/v1/projects/1/forms/simple2') .set('X-Extended-Metadata', true) .expect(200) diff --git a/test/integration/fixtures/02-forms.js b/test/integration/fixtures/02-forms.js index 4efe06abe..1312c6c9c 100644 --- a/test/integration/fixtures/02-forms.js +++ b/test/integration/fixtures/02-forms.js @@ -1,13 +1,23 @@ - const appRoot = require('app-root-path'); -const { mapSequential } = require(appRoot + '/test/util/util'); const { Form } = require(appRoot + '/lib/model/frames'); const { simple, withrepeat } = require('../../data/xml').forms; const forms = [ simple, withrepeat ]; -module.exports = ({ Forms, Projects }) => - Projects.getById(1) - .then((option) => option.get()) - .then((project) => mapSequential(forms, (xml) => - Form.fromXml(xml).then((partial) => Forms.createNew(partial, project, true)))); +module.exports = async ({ Assignments, Forms, Projects, Roles }) => { + const project = (await Projects.getById(1)).get(); + const { id: formview } = (await Roles.getBySystemName('formview')).get(); + /* eslint-disable no-await-in-loop */ + for (const xml of forms) { + const partial = await Form.fromXml(xml); + // Create the form without Enketo IDs in order to maintain existing tests. + global.enketo.state = 'error'; + const { acteeId } = await Forms.createNew(partial, project, true); + + // Delete the assignment of the formview actor created by Forms.createNew() + // in order to maintain existing tests. + const [{ actorId }] = await Assignments.getByActeeAndRoleId(acteeId, formview); + await Assignments.revokeByActorId(actorId); + } + /* eslint-enable no-await-in-loop */ +}; diff --git a/test/integration/setup.js b/test/integration/setup.js index d33922037..d88d6cbdf 100644 --- a/test/integration/setup.js +++ b/test/integration/setup.js @@ -41,7 +41,11 @@ const bcrypt = require(appRoot + '/lib/util/crypto').password(_bcrypt); // set up our enketo mock. const { reset: resetEnketo, ...enketo } = require(appRoot + '/test/util/enketo'); -beforeEach(resetEnketo); +// Initialize the mock before other setup that uses the mock, then reset the +// mock after setup is complete and after each test. +before(resetEnketo); +after(resetEnketo); +afterEach(resetEnketo); // set up odk analytics mock. const { ODKAnalytics } = require(appRoot + '/test/util/odk-analytics-mock'); @@ -83,7 +87,7 @@ const initialize = async () => { await migrator.destroy(); } - return withDefaults({ db, bcrypt, context }).transacting(populate); + return withDefaults({ db, bcrypt, context, enketo, env }).transacting(populate); }; // eslint-disable-next-line func-names, space-before-function-paren diff --git a/test/integration/task/reap-sessions.js b/test/integration/task/reap-sessions.js index 2e249083d..7520be489 100644 --- a/test/integration/task/reap-sessions.js +++ b/test/integration/task/reap-sessions.js @@ -10,7 +10,9 @@ describe('task: reap-sessions', () => { .then((actor) => Promise.all([ 2000, 2001, 2002, 2003, 3000, 3001, 3002, 3003 ] .map((year) => Sessions.create(actor, new Date(`${year}-01-01`))))) .then(() => reapSessions()) - .then(() => oneFirst(sql`select count(*) from sessions`)) + .then(() => oneFirst(sql` +SELECT count(*) FROM sessions +JOIN actors ON actors.id = sessions."actorId" AND actors.type = 'actor'`)) .then((count) => { count.should.equal(4); }))); }); diff --git a/test/unit/external/enketo.js b/test/unit/external/enketo.js index 45cabdeb3..bcdd0b918 100644 --- a/test/unit/external/enketo.js +++ b/test/unit/external/enketo.js @@ -1,7 +1,7 @@ const appRoot = require('app-root-path'); const nock = require('nock'); const querystring = require('querystring'); -const enketo_ = require(appRoot + '/lib/external/enketo'); +const { init } = require(appRoot + '/lib/external/enketo'); const Problem = require(appRoot + '/lib/util/problem'); describe('external/enketo', () => { @@ -9,7 +9,7 @@ describe('external/enketo', () => { url: 'http://enketoHost:1234/enketoPath', apiKey: 'enketoApiKey' }; - const enketo = enketo_.init(enketoConfig); + const enketo = init(enketoConfig); const enketoNock = nock('http://enketoHost:1234'); const openRosaUrl = 'http://openRosaHost:5678/somePath'; const xmlFormId = 'wellPumps'; @@ -17,7 +17,7 @@ describe('external/enketo', () => { describe('preview', () => { it('should send a properly constructed request to Enketo', () => { enketoNock - .post('/enketoPath/api/v2/survey') + .post('/enketoPath/api/v2/survey/all') // eslint-disable-next-line space-before-function-paren, func-names .reply(201, function(uri, requestBody) { const base64Auth = Buffer.from('enketoApiKey:').toString('base64'); @@ -25,57 +25,58 @@ describe('external/enketo', () => { this.req.headers.authorization.should.equal(`Basic ${base64Auth}`); this.req.headers.cookie.should.equal('__Host-session=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); requestBody.should.equal(expectedQueryString); - return JSON.stringify({ url: 'http://enke.to/::stuvwxyz', code: 201 }); + return JSON.stringify({ + enketo_id: '::stuvwxyz', + single_once_url: 'http://enke.to/single/::::zyxwvuts', + code: 201 + }); }); return enketo.create(openRosaUrl, xmlFormId, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); }); - it('should return the Enketo survey ID', () => { + it('should return Enketo survey IDs', async () => { enketoNock - .post('/enketoPath/api/v2/survey') - .reply(201, { url: 'http://enke.to/::stuvwxyz', code: 201 }); - // eslint-disable-next-line no-trailing-spaces - - return enketo.create(openRosaUrl, xmlFormId, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa') - .then((result) => result.should.equal('::stuvwxyz')); - }); + .post('/enketoPath/api/v2/survey/all') + .reply(201, { + enketo_id: '::stuvwxyz', + single_once_url: 'http://enke.to/single/::::zyxwvuts', + code: 201 + }); - it('should return an Enketo single-survey ID', () => { - enketoNock - .post('/enketoPath/api/v2/survey/single/once') - .reply(201, { single_once_url: 'http://enke.to/single/::stuvwxyz', code: 201 }); - // eslint-disable-next-line no-trailing-spaces - - return enketo.createOnceToken(openRosaUrl, xmlFormId, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa') - .then((result) => result.should.equal('::stuvwxyz')); + const result = await enketo.create(openRosaUrl, xmlFormId, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); + result.should.eql({ + enketoId: '::stuvwxyz', + enketoOnceId: '::::zyxwvuts', + }); }); it('should throw a Problem if the Enketo response is not valid json', () => { enketoNock - .post('/enketoPath/api/v2/survey') + .post('/enketoPath/api/v2/survey/all') .reply(201, 'no json for you!'); - // eslint-disable-next-line no-trailing-spaces - + return enketo.create(openRosaUrl, xmlFormId, null) .should.be.rejectedWith(Problem.internal.enketoUnexpectedResponse('invalid JSON')); }); - it('should throw a Problem if the Enketo response url is unparseable', () => { + it('should throw a Problem if the single_once_url from Enketo is unparseable', () => { enketoNock - .post('/enketoPath/api/v2/survey') - .reply(201, { url: 'http://enke.to/$$', code: 201 }); - // eslint-disable-next-line no-trailing-spaces - + .post('/enketoPath/api/v2/survey/all') + .reply(201, { + enketoId: '::stuvwxyz', + single_once_url: 'http://enke.to/$$', + code: 201 + }); + return enketo.create(openRosaUrl, xmlFormId, null) - .should.be.rejectedWith(Problem.internal.enketoUnexpectedResponse('Could not parse token from enketo response url: http://enke.to/$$')); + .should.be.rejectedWith(Problem.internal.enketoUnexpectedResponse('Could not parse token from single_once_url: http://enke.to/$$')); }); it('should throw a Problem if the Enketo response code is unexpected', () => { enketoNock - .post('/enketoPath/api/v2/survey') + .post('/enketoPath/api/v2/survey/all') .reply(204, {}); - // eslint-disable-next-line no-trailing-spaces - + return enketo.create(openRosaUrl, xmlFormId, null) .should.be.rejectedWith(Problem.internal.enketoUnexpectedResponse('wrong status code')); }); diff --git a/test/util/enketo.js b/test/util/enketo.js index c84428159..6c9fdb649 100644 --- a/test/util/enketo.js +++ b/test/util/enketo.js @@ -7,25 +7,24 @@ const Problem = require(appRoot + '/lib/util/problem'); const { without } = require(appRoot + '/lib/util/util'); const defaults = { - // Properties that each test can set to determine the behavior of the mock + // Properties that can be set to change the behavior of the mock. These + // properties are reset after each mock request. // If `state` is set to 'error', the mock will pretend that Enketo has // misbehaved and will return a rejected promise for the next call. state: undefined, // Controls the timing of the Enketo response. wait: call, - // The enketoId for the create() or createOnceToken() method to return. By - // default, it is ::abcdefgh for create() and ::::abcdefgh for - // createOnceToken(). - token: undefined, + // The enketoId for the create() method to return (by default, ::abcdefgh). + enketoId: undefined, // Properties that the mock may update after being called. These properties // are how the mock communicates back to the test. - // The total number of times that the mock has been called during the test + // The number of times that the mock has been called during the test, that is, + // the number of requests that would be sent to Enketo callCount: 0, - // The OpenRosa URL that was passed to the create() or createOnceToken() - // method + // The OpenRosa URL that was passed to the create() method receivedUrl: undefined, // An object with a property for each argument passed to the edit() method editData: undefined @@ -40,7 +39,7 @@ const reset = () => { }; // Mocks a request to Enketo. -const request = (f) => { +const request = () => { global.enketo.callCount += 1; const options = { ...global.enketo }; Object.assign(global.enketo, without(['callCount'], defaults)); @@ -53,25 +52,22 @@ const request = (f) => { else if (options.state === 'error') reject(Problem.internal.enketoUnexpectedResponse('wrong status code')); else - resolve(f(options)); + resolve(options); }); }); }; -const _create = (prefix) => (openRosaUrl) => - request(({ token = `${prefix}abcdefgh` }) => { - global.enketo.receivedUrl = openRosaUrl; - return token; - }); - -const edit = (openRosaUrl, domain, form, logicalId, submissionDef, attachments, token) => - request(() => { - global.enketo.editData = { openRosaUrl, domain, form, logicalId, submissionDef, attachments, token }; - return 'https://enketo/edit/url'; - }); +const create = async (openRosaUrl) => { + const { enketoId = '::abcdefgh' } = await request(); + global.enketo.receivedUrl = openRosaUrl; + return { enketoId, enketoOnceId: '::::abcdefgh' }; +}; -module.exports = { - create: _create('::'), createOnceToken: _create('::::'), edit, - reset +const edit = async (openRosaUrl, domain, form, logicalId, submissionDef, attachments, token) => { + await request(); + global.enketo.editData = { openRosaUrl, domain, form, logicalId, submissionDef, attachments, token }; + return 'https://enketo/edit/url'; }; +module.exports = { create, edit, reset }; +