diff --git a/lib/model/migrations/20200428-01-allow-string-downcast.js b/lib/model/migrations/20200428-01-allow-string-downcast.js new file mode 100644 index 000000000..4e9955f4e --- /dev/null +++ b/lib/model/migrations/20200428-01-allow-string-downcast.js @@ -0,0 +1,70 @@ +// Copyright 2020 ODK Central Developers +// See the NOTICE file at the top-level directory of this distribution and at +// https://github.com/opendatakit/central-backend/blob/master/NOTICE. +// This file is part of ODK Central. It is subject to the license terms in +// the LICENSE file found in the top-level directory of this distribution and at +// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central, +// including this file, may be copied, modified, propagated, or distributed +// except according to the terms contained in the LICENSE file. + + +const up = async (db) => { + await db.raw(` +create or replace function check_field_collisions() returns trigger as $check_field_collisions$ + declare extant int; + declare extformid int; + declare extpath text; + declare exttype text; + begin + -- factoring the repeated joins here into a CTE explodes the cost by 10x + + select count(distinct type), form_fields."formId", form_fields.path into extant, extformid, extpath + from form_fields + + -- finds canonical formDefIds (published, or active draft) + left outer join (select id from form_defs where "publishedAt" is not null) as form_defs + on form_defs.id = form_fields."formDefId" + left outer join (select id, "draftDefId" from forms) as forms + on forms."draftDefId" = form_fields."formDefId" + + -- weeds out paths whose latest def indicates they are a string. first figure + -- out latest def, then knock out latest strings from conflict detection. + inner join + (select form_fields."formId", max("formDefId") as "latestDefId" from form_fields + -- this is a repeat of the above canonical-def subquery + left outer join (select id from form_defs where "publishedAt" is not null) as ifds + on ifds.id = form_fields."formDefId" + left outer join (select id, "draftDefId" from forms) as ifs + on ifs."draftDefId" = form_fields."formDefId" + where ifs.id is not null or ifds.id is not null + group by form_fields."formId" + ) as tail + on tail."formId" = form_fields."formId" + inner join + (select "formDefId", path from form_fields where type != 'string') as nonstring + on "latestDefId" = nonstring."formDefId" and form_fields.path = nonstring.path + + where forms.id is not null or form_defs.id is not null + group by form_fields."formId", form_fields.path having count(distinct type) > 1; + + if extant > 0 then + select type into exttype + from form_fields + where "formId" = extformid and path = extpath + order by "formDefId" desc + limit 1 + offset 1; + + raise exception using message = format('ODK05:%s:%s', extpath, exttype); + end if; + + return NEW; + end; +$check_field_collisions$ language plpgsql; +`); +}; + +const down = () => {}; // no. would cause problems. + +module.exports = { up, down }; + diff --git a/test/integration/api/forms.js b/test/integration/api/forms.js index 9cf789edd..79b536181 100644 --- a/test/integration/api/forms.js +++ b/test/integration/api/forms.js @@ -1386,7 +1386,7 @@ describe('api: /projects/:id/forms', () => { it('should complain about field conflicts', testService((service) => service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms/simple/draft') - .send(testData.forms.simple.replace('type="int"', 'type="string"')) + .send(testData.forms.simple.replace('type="int"', 'type="date"')) .set('Content-Type', 'application/xml') .expect(400) .then(({ body }) => { @@ -1405,7 +1405,7 @@ describe('api: /projects/:id/forms', () => { .then(() => asAlice.post('/v1/projects/1/forms/simple/draft/publish') .expect(200)) .then(() => asAlice.post('/v1/projects/1/forms/simple/draft') - .send(testData.forms.simple.replace('type="int"', 'type="string"')) + .send(testData.forms.simple.replace('type="int"', 'type="date"')) .set('Content-Type', 'application/xml') .expect(400) .then(({ body }) => { @@ -1413,6 +1413,52 @@ describe('api: /projects/:id/forms', () => { body.details.should.eql({ path: '/age', type: 'int' }); }))))); + it('should not complain on downcast to string', testService((service) => + service.login('alice', (asAlice) => + asAlice.post('/v1/projects/1/forms/simple/draft') + .send(testData.forms.simple.replace('type="int"', 'type="string"')) + .set('Content-Type', 'application/xml') + .expect(200)))); + + it('should complain on upcast from string', testService((service) => + service.login('alice', (asAlice) => + asAlice.post('/v1/projects/1/forms/simple/draft') + .send(testData.forms.simple.replace('name" type="string"', 'name" type="int"')) + .set('Content-Type', 'application/xml') + .expect(400) + .then(({ body }) => { + body.code.should.equal(400.17); + body.details.should.eql({ path: '/name', type: 'string' }); + })))); + + it('should not complain on double-downcast to string', testService((service) => + service.login('alice', (asAlice) => + asAlice.post('/v1/projects/1/forms/simple/draft') + .send(testData.forms.simple.replace('type="int"', 'type="string"')) + .set('Content-Type', 'application/xml') + .expect(200) + .then(() => asAlice.post('/v1/projects/1/forms/simple/draft/publish?version=two') + .expect(200)) + .then(() => asAlice.post('/v1/projects/1/forms/simple/draft') + .expect(200))))); + + it('should complain on upcast once downcasted', testService((service) => + service.login('alice', (asAlice) => + asAlice.post('/v1/projects/1/forms/simple/draft') + .send(testData.forms.simple.replace('type="int"', 'type="string"')) + .set('Content-Type', 'application/xml') + .expect(200) + .then(() => asAlice.post('/v1/projects/1/forms/simple/draft/publish?version=two') + .expect(200)) + .then(() => asAlice.post('/v1/projects/1/forms/simple/draft') + .send(testData.forms.simple) + .set('Content-Type', 'application/xml') + .expect(400) + .then(({ body }) => { + body.code.should.equal(400.17); + body.details.should.eql({ path: '/age', type: 'string' }); + }))))); + it('should not complain about discarded draft field conflicts', testService((service) => service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms/simple/draft')