Skip to content

Commit 82ea6cb

Browse files
matthew-whitematthew-white
committed
Request enketoId during request to create new form draft
Addresses the "new draft is created" case of getodk/central#385.
1 parent 41510ca commit 82ea6cb

File tree

3 files changed

+97
-32
lines changed

3 files changed

+97
-32
lines changed

‎lib/model/query/forms.js‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const { getFormFields, merge, compare } = require('../../data/schema');
1616
const { getDataset } = require('../../data/dataset');
1717
const { generateToken } = require('../../util/crypto');
1818
const { unjoiner, extender, updater, equals, insert, insertMany, markDeleted, markUndeleted, QueryOptions } = require('../../util/db');
19-
const { resolve, reject } = require('../../util/promise');
19+
const { resolve, reject, timebound } = require('../../util/promise');
2020
const { splitStream } = require('../../util/stream');
2121
const { construct, noop, pickAll } = require('../../util/util');
2222
const Option = require('../../util/option');
@@ -123,7 +123,7 @@ createNew.audit.withResult = true;
123123
// Inserts a new form def into the database for createVersion() below, setting
124124
// fields on the def according to whether the def will be the current def or the
125125
// draft def.
126-
const _createNewDef = (partial, form, publish, data) => ({ one }) => {
126+
const _createNewDef = (partial, form, publish, data) => async ({ one, enketo, env }) => {
127127
const insertWith = (moreData) => one(insert(partial.def.with({
128128
formId: form.id,
129129
xlsBlobId: partial.xls.xlsBlobId,
@@ -135,9 +135,19 @@ const _createNewDef = (partial, form, publish, data) => ({ one }) => {
135135
if (publish === true) return insertWith({ publishedAt: new Date() });
136136

137137
// Check whether there is an existing draft that we have access to. If not,
138-
// generate a draft token.
139-
if (form.def.id == null || form.def.id !== form.draftDefId)
140-
return insertWith({ draftToken: generateToken() });
138+
// generate a draft token and enketoId.
139+
if (form.def.id == null || form.def.id !== form.draftDefId) {
140+
const draftToken = generateToken();
141+
142+
// Try to push the draft to Enketo. If doing so fails or is too slow, then
143+
// the worker will try again later.
144+
const encodedId = encodeURIComponent(form.xmlFormId);
145+
const path = `/v1/test/${draftToken}/projects/${form.projectId}/forms/${encodedId}/draft`;
146+
const request = enketo.create(`${env.domain}${path}`, form.xmlFormId);
147+
const enketoId = await timebound(request, 0.5).catch(() => null);
148+
149+
return insertWith({ draftToken, enketoId });
150+
}
141151

142152
// Copy forward fields from the existing draft.
143153
return insertWith(pickAll(['draftToken', 'enketoId'], form.def));

‎lib/worker/form.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ const pushDraftToEnketo = ({ Forms, enketo, env }, event) =>
1515
// if there was no draft or this form isn't the draft anymore just bail.
1616
if ((form.def.id == null) || (form.draftDefId !== form.def.id)) return;
1717

18-
// if the enketoId has been carried forward from the previous draft, then
19-
// bail.
18+
// if the enketoId was received during the request to create the draft, or
19+
// if it was carried forward from the previous draft, then bail.
2020
if (form.def.enketoId != null) return;
2121

2222
// if this form doesn't have a draft testing key something is broken

‎test/integration/api/forms/draft.js‎

Lines changed: 80 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe('api: /projects/:id/forms (drafts)', () => {
6161
body.version.should.equal('drafty');
6262
})))));
6363

64-
it('should create a new draft token setting a new draft version', testService((service) =>
64+
it('should create a new draft token while setting a new draft', testService((service) =>
6565
service.login('alice', (asAlice) =>
6666
asAlice.post('/v1/projects/1/forms/simple/draft')
6767
.expect(200)
@@ -83,22 +83,81 @@ describe('api: /projects/:id/forms (drafts)', () => {
8383
}));
8484
})))));
8585

86-
it('should worker-process the draft form over to enketo', testService((service, container) =>
87-
service.login('alice', (asAlice) =>
88-
asAlice.post('/v1/projects/1/forms/simple/draft')
89-
.expect(200)
90-
.then(({ body }) => {
91-
should.not.exist(body.enketoId);
92-
})
93-
.then(() => exhaust(container))
94-
.then(() => asAlice.get('/v1/projects/1/forms/simple/draft')
95-
.expect(200)
96-
.then(({ body }) => {
97-
body.enketoId.should.equal('::abcdefgh');
98-
should.not.exist(body.enketoOnceId);
99-
global.enketo.receivedUrl.startsWith(container.env.domain).should.equal(true);
100-
global.enketo.receivedUrl.should.match(/\/v1\/test\/[a-z0-9$!]{64}\/projects\/1\/forms\/simple\/draft/i);
101-
})))));
86+
it('should request an enketoId while setting a new draft', testService(async (service, { env }) => {
87+
const asAlice = await service.login('alice');
88+
global.enketo.token = '::ijklmnop';
89+
await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
90+
global.enketo.callCount.should.equal(1);
91+
global.enketo.receivedUrl.startsWith(env.domain).should.be.true();
92+
const match = global.enketo.receivedUrl.match(/\/v1\/test\/([a-z0-9$!]{64})\/projects\/1\/forms\/simple\/draft$/i);
93+
should.exist(match);
94+
const { body } = await asAlice.get('/v1/projects/1/forms/simple/draft')
95+
.expect(200);
96+
match[1].should.equal(body.draftToken);
97+
body.enketoId.should.equal('::ijklmnop');
98+
}));
99+
100+
it('should request a new enketoId while setting each new draft', testService(async (service, { env }) => {
101+
const asAlice = await service.login('alice');
102+
await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
103+
await asAlice.post('/v1/projects/1/forms/simple/draft/publish?version=two')
104+
.expect(200);
105+
global.enketo.callCount.should.equal(1);
106+
global.enketo.token = '::ijklmnop';
107+
await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
108+
global.enketo.callCount.should.equal(2);
109+
global.enketo.receivedUrl.startsWith(env.domain).should.be.true();
110+
const match = global.enketo.receivedUrl.match(/\/v1\/test\/([a-z0-9$!]{64})\/projects\/1\/forms\/simple\/draft$/i);
111+
should.exist(match);
112+
const { body } = await asAlice.get('/v1/projects/1/forms/simple/draft')
113+
.expect(200);
114+
match[1].should.equal(body.draftToken);
115+
body.enketoId.should.equal('::ijklmnop');
116+
}));
117+
118+
it('should return with success even if the request to Enketo fails', testService(async (service) => {
119+
const asAlice = await service.login('alice');
120+
global.enketo.state = 'error';
121+
await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
122+
const { body } = await asAlice.get('/v1/projects/1/forms/simple/draft')
123+
.expect(200);
124+
should.not.exist(body.enketoId);
125+
}));
126+
127+
it('should stop waiting for Enketo after 0.5 seconds @slow', testService(async (service) => {
128+
const asAlice = await service.login('alice');
129+
global.enketo.wait = (f) => { setTimeout(f, 501); };
130+
await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
131+
const { body } = await asAlice.get('/v1/projects/1/forms/simple/draft')
132+
.expect(200);
133+
should.not.exist(body.enketoId);
134+
}));
135+
136+
it('should request an enketoId from the worker if the request from the endpoint fails', testService(async (service, container) => {
137+
const asAlice = await service.login('alice');
138+
global.enketo.state = 'error';
139+
await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
140+
global.enketo.callCount.should.equal(1);
141+
global.enketo.token = '::ijklmnop';
142+
await exhaust(container);
143+
global.enketo.callCount.should.equal(2);
144+
global.enketo.receivedUrl.startsWith(container.env.domain).should.be.true();
145+
const match = global.enketo.receivedUrl.match(/\/v1\/test\/([a-z0-9$!]{64})\/projects\/1\/forms\/simple\/draft$/i);
146+
should.exist(match);
147+
const { body } = await asAlice.get('/v1/projects/1/forms/simple/draft')
148+
.expect(200);
149+
match[1].should.equal(body.draftToken);
150+
body.enketoId.should.equal('::ijklmnop');
151+
should.not.exist(body.enketoOnceId);
152+
}));
153+
154+
it('should not request an enketoId from the worker if the request from the endpoint succeeds', testService(async (service, container) => {
155+
const asAlice = await service.login('alice');
156+
await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
157+
global.enketo.callCount.should.equal(1);
158+
await exhaust(container);
159+
global.enketo.callCount.should.equal(1);
160+
}));
102161

103162
it('should manage draft/published enketo tokens separately', testService((service, container) =>
104163
service.login('alice', (asAlice) =>
@@ -111,7 +170,6 @@ describe('api: /projects/:id/forms (drafts)', () => {
111170
global.enketo.token = '::ijklmnop';
112171
return asAlice.post('/v1/projects/1/forms/simple2/draft')
113172
.expect(200)
114-
.then(() => exhaust(container))
115173
.then(() => Promise.all([
116174
asAlice.get('/v1/projects/1/forms/simple2')
117175
.expect(200)
@@ -167,14 +225,15 @@ describe('api: /projects/:id/forms (drafts)', () => {
167225
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty"'))
168226
.set('Content-Type', 'application/xml')
169227
.expect(200);
170-
await exhaust(container);
228+
global.enketo.callCount.should.equal(1);
171229
const { body: draft1 } = await asAlice.get('/v1/projects/1/forms/simple/draft')
172230
.expect(200);
173231
draft1.enketoId.should.equal('::abcdefgh');
174232
await asAlice.post('/v1/projects/1/forms/simple/draft')
175233
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty2"'))
176234
.set('Content-Type', 'application/xml')
177235
.expect(200);
236+
global.enketo.callCount.should.equal(1);
178237
const { body: draft2 } = await asAlice.get('/v1/projects/1/forms/simple/draft')
179238
.expect(200);
180239
draft2.enketoId.should.equal('::abcdefgh');
@@ -186,16 +245,13 @@ describe('api: /projects/:id/forms (drafts)', () => {
186245
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty"'))
187246
.set('Content-Type', 'application/xml')
188247
.expect(200);
189-
await exhaust(container);
248+
global.enketo.callCount.should.equal(1);
190249
await asAlice.post('/v1/projects/1/forms/simple/draft')
191250
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty2"'))
192251
.set('Content-Type', 'application/xml')
193252
.expect(200);
194-
global.enketoToken = '::ijklmnop';
195253
await exhaust(container);
196-
const { body: draft } = await asAlice.get('/v1/projects/1/forms/simple/draft')
197-
.expect(200);
198-
draft.enketoId.should.equal('::abcdefgh');
254+
global.enketo.callCount.should.equal(1);
199255
}));
200256

201257
it('should copy the published form definition if not given one', testService((service) =>
@@ -854,7 +910,6 @@ describe('api: /projects/:id/forms (drafts)', () => {
854910
global.enketo.token = '::ijklmnop';
855911
return asAlice.post('/v1/projects/1/forms/simple2/draft')
856912
.expect(200)
857-
.then(() => exhaust(container))
858913
.then(() => asAlice.get('/v1/projects/1/forms/simple2/draft')
859914
.set('X-Extended-Metadata', true)
860915
.expect(200)

0 commit comments

Comments
 (0)