diff --git a/lib/model/query/forms.js b/lib/model/query/forms.js index 0a75df8a2..a09f36b86 100644 --- a/lib/model/query/forms.js +++ b/lib/model/query/forms.js @@ -58,34 +58,56 @@ const pushDraftToEnketo = ({ projectId, xmlFormId, def, draftToken = def?.draftT return (await timeboundEnketo(enketo.create(path, xmlFormId), bound)).enketoId; }; +// 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([ @@ -100,8 +122,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 }; @@ -269,12 +315,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) ]) @@ -728,7 +780,7 @@ const _newSchema = () => ({ one }) => .then((s) => s.id); module.exports = { - fromXls, pushDraftToEnketo, _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 b40d156e8..c08372f7c 100644 --- a/lib/worker/form.js +++ b/lib/worker/form.js @@ -7,7 +7,7 @@ // 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 }, event) => Forms.getByActeeIdForUpdate(event.acteeId, undefined, Form.DraftVersion) @@ -27,24 +27,15 @@ const pushDraftToEnketo = ({ Forms }, event) => .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((enketoIds) => Forms.update(form, new Form(enketoIds)))); + return Forms.pushFormToEnketo(form) + .then((enketoIds) => Forms.update(form, new Form(enketoIds))); }).orNull()); const create = pushDraftToEnketo; diff --git a/test/integration/api/forms/draft.js b/test/integration/api/forms/draft.js index 740eb0671..0498764be 100644 --- a/test/integration/api/forms/draft.js +++ b/test/integration/api/forms/draft.js @@ -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.callCount.should.equal(2); global.enketo.enketoId = '::ijklmnop'; await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200); - global.enketo.callCount.should.equal(2); + 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); @@ -158,13 +158,12 @@ 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.enketoId = '::ijklmnop'; return asAlice.post('/v1/projects/1/forms/simple2/draft') @@ -898,13 +897,12 @@ 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.enketoId = '::ijklmnop'; return asAlice.post('/v1/projects/1/forms/simple2/draft') @@ -1144,6 +1142,151 @@ 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 + 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 d9a362fc5..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.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..38e0d2ef0 100644 --- a/test/integration/fixtures/02-forms.js +++ b/test/integration/fixtures/02-forms.js @@ -1,13 +1,22 @@ - const appRoot = require('app-root-path'); -const { mapSequential } = require(appRoot + '/test/util/util'); const { Form } = require(appRoot + '/lib/model/frames'); +const { QueryOptions } = require(appRoot + '/lib/util/db'); 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 ({ Actors, Assignments, Forms, Projects, Roles }) => { + const project = (await Projects.getById(1)).get(); + /* eslint-disable no-await-in-loop */ + for (const xml of forms) { + const partial = await Form.fromXml(xml); + const { acteeId } = await Forms.createNew(partial, project, true); + + // Delete the formview actor created by Forms.createNew() in order to + // maintain existing tests. + const { id: roleId } = (await Roles.getBySystemName('formview')).get(); + const [{ actor }] = await Assignments.getByActeeAndRoleId(acteeId, roleId, QueryOptions.extended); + await Actors.del(actor); + } + /* eslint-enable no-await-in-loop */ +}; diff --git a/test/integration/setup.js b/test/integration/setup.js index d33922037..7ce454368 100644 --- a/test/integration/setup.js +++ b/test/integration/setup.js @@ -41,7 +41,8 @@ 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); +before(resetEnketo); +afterEach(resetEnketo); // set up odk analytics mock. const { ODKAnalytics } = require(appRoot + '/test/util/odk-analytics-mock'); @@ -83,7 +84,12 @@ const initialize = async () => { await migrator.destroy(); } - return withDefaults({ db, bcrypt, context }).transacting(populate); + // When creating fixtures, create forms without Enketo IDs in order to + // maintain existing tests. + global.enketo.state = 'error'; + return withDefaults({ db, bcrypt, context, enketo, env }) + .transacting(populate) + .finally(resetEnketo); }; // eslint-disable-next-line func-names, space-before-function-paren