Skip to content

Commit

Permalink
Merge pull request #268 from getodk/issa/string-downcast
Browse files Browse the repository at this point in the history
improve: allow fields to be downcast to string.
  • Loading branch information
issa-tseng authored Apr 29, 2020
2 parents 0b9c5a7 + 2f46dd1 commit f6502a0
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 2 deletions.
70 changes: 70 additions & 0 deletions lib/model/migrations/20200428-01-allow-string-downcast.js
Original file line number Diff line number Diff line change
@@ -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 };

50 changes: 48 additions & 2 deletions test/integration/api/forms.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand All @@ -1405,14 +1405,60 @@ 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 }) => {
body.code.should.equal(400.17);
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')
Expand Down

0 comments on commit f6502a0

Please sign in to comment.