Skip to content

Commit 8e21629

Browse files
sadiqkhojasadiqkhoja
authored
Enhance: complex type / groups can be '$select'ed (#717)
* Enhance: complex type / groups can be '$select'ed * added a couple of unit tests
1 parent b885e22 commit 8e21629

File tree

4 files changed

+350
-24
lines changed

4 files changed

+350
-24
lines changed

‎lib/formats/odata.js

+80-20
Original file line numberDiff line numberDiff line change
@@ -440,38 +440,92 @@ const singleRowToOData = (fields, row, domain, originalUrl, query) => {
440440
});
441441
};
442442

443+
// Create a tree of form fields
444+
// Where each node has reference to its parent and children
445+
// Returns an object with key=path and value=fieldNode
446+
// so that we can quickly get the field node instead of traversing tree from top to bottom
447+
// Assumption: formFields are in order
448+
const getFieldTree = (formFields) => {
449+
const tree = {};
450+
451+
for (let i = 0; i < formFields.length; i += 1) {
452+
const node = { value: formFields[i], children: [], parent: null };
453+
tree[`${node.value.path.split('/').map(sanitizeOdataIdentifier).join('/')}`] = node;
454+
}
455+
456+
for (const i of Object.keys(tree)) {
457+
const node = tree[i];
458+
const parentPath = node.value.path.match(/(^.*)\//)[1].split('/').map(sanitizeOdataIdentifier).join('/');
459+
460+
if (tree[parentPath]) {
461+
node.parent = tree[parentPath];
462+
node.parent.children.push(node);
463+
}
464+
}
465+
466+
return tree;
467+
};
468+
469+
// Returns children recursively
470+
const getChildren = (field) => {
471+
const result = new Set();
472+
const stack = [];
473+
stack.push(field);
474+
475+
while (stack.length > 0) {
476+
const node = stack.pop();
477+
node.children.forEach(c => {
478+
if (c.value.type === 'structure') {
479+
stack.push(c);
480+
}
481+
result.add(c.value);
482+
});
483+
}
484+
return result;
485+
};
486+
443487
// Validates $select query parameter including metadata properties and returns list of FormFields
444488
const filterFields = (formFields, select, table) => {
445489
const filteredFields = new Set();
446-
const fieldMap = formFields.reduce((map, field) => ({ ...map, [`${field.path.split('/').map(sanitizeOdataIdentifier).join('/')}`]: field }), {});
490+
const fieldTree = getFieldTree(formFields);
491+
492+
let path = '';
493+
494+
// For subtables we have to include parents fields
495+
if (table !== 'Submissions') {
496+
for (const tableSegment of table.replace(/Submissions\./, '').split('.')) {
497+
path += `/${tableSegment}`;
498+
if (!fieldTree[path]) throw Problem.user.notFound();
499+
filteredFields.add(fieldTree[path].value);
500+
}
501+
}
502+
447503
for (const property of select.split(',').map(p => p.trim())) {
448504
// validate metadata properties. __system/.. properties are only valid for Submission table
449505
if (property.startsWith('__id') || property.startsWith('__system')) {
450506
if (!(property === '__id' || (table === 'Submissions' && systemFields.has(property))))
451507
throw Problem.user.propertyNotFound({ property });
452508
} else {
453509

454-
let path = '';
510+
const field = fieldTree[`${path}/${property}`];
511+
if (!field) throw Problem.user.propertyNotFound({ property });
455512

456-
// For subtables we have to include parents fields
457-
if (table !== 'Submissions') {
458-
for (const tableSegment of table.replace(/Submissions\./, '').split('.')) {
459-
path += `/${tableSegment}`;
460-
if (!fieldMap[path]) throw Problem.user.notFound();
461-
filteredFields.add(fieldMap[path]);
462-
}
463-
}
513+
filteredFields.add(field.value);
464514

465515
// we have to include parents fields in the result to handle grouped fields
466-
const propSegments = property.split('/');
467-
for (let i = 0; i < propSegments.length; i+=1) {
468-
path += `/${propSegments[i]}`;
469-
const field = fieldMap[path];
470-
if (!field) throw Problem.user.propertyNotFound({ property });
471-
// it's ok to include field with repeat type so that user can have navigation link
472-
// but child field of a repeat field is not supported
473-
if (field.type === 'repeat' && i < propSegments.length - 1) throw Problem.user.unsupportedODataSelectField({ property });
474-
filteredFields.add(field);
516+
let node = field.parent;
517+
while (node && !filteredFields.has(node.value)) { // filteredFields set already has the subtables
518+
// Child field of a repeat field is not supported
519+
if (node.value.type === 'repeat') throw Problem.user.unsupportedODataSelectField({ property });
520+
521+
filteredFields.add(node.value);
522+
node = node.parent;
523+
}
524+
525+
// Include the children of structure/group
526+
// Note: This doesn't expand 'repeat' fields
527+
if (field.value.type === 'structure') {
528+
getChildren(field).forEach(filteredFields.add, filteredFields);
475529
}
476530
}
477531
}
@@ -489,5 +543,11 @@ const filterFields = (formFields, select, table) => {
489543

490544
const selectFields = (query, table) => (fields) => (query.$select && query.$select !== '*' ? filterFields(fields, query.$select, table) : fields);
491545

492-
module.exports = { odataXmlError, serviceDocumentFor, edmxFor, rowStreamToOData, singleRowToOData, selectFields, getTableFromOriginalUrl };
546+
module.exports = {
547+
odataXmlError, serviceDocumentFor, edmxFor,
548+
rowStreamToOData, singleRowToOData,
549+
selectFields, getTableFromOriginalUrl,
550+
// exporting for unit tests
551+
getFieldTree, getChildren
552+
};
493553

‎test/data/xml.js

+139-1
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,109 @@ module.exports = {
354354
<bind nodeset="/data/hometown" type="string"/>
355355
</model>
356356
</h:head>
357-
</h:html>`
357+
</h:html>`,
358+
359+
groupRepeat: `<?xml version="1.0"?>
360+
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
361+
<h:head>
362+
<h:title>groupRepeat</h:title>
363+
<model odk:xforms-version="1.0.0">
364+
<instance>
365+
<data id="groupRepeat">
366+
<text/>
367+
<child_repeat jr:template="">
368+
<name/>
369+
<address>
370+
<city/>
371+
<country/>
372+
</address>
373+
</child_repeat>
374+
<child_repeat>
375+
<name/>
376+
<address>
377+
<city/>
378+
<country/>
379+
</address>
380+
</child_repeat>
381+
<meta>
382+
<instanceID/>
383+
</meta>
384+
</data>
385+
</instance>
386+
<bind nodeset="/data/text" type="string"/>
387+
<bind nodeset="/data/child_repeat/name" type="string"/>
388+
<bind nodeset="/data/child_repeat/address/city" type="string"/>
389+
<bind nodeset="/data/child_repeat/address/country" type="string"/>
390+
<bind jr:preload="uid" nodeset="/data/meta/instanceID" readonly="true()" type="string"/>
391+
</model>
392+
</h:head>
393+
<h:body>
394+
<input ref="/data/text">
395+
<label>text</label>
396+
</input>
397+
<group ref="/data/child_repeat">
398+
<label>Children</label>
399+
<repeat nodeset="/data/child_repeat">
400+
<input ref="/data/child_repeat/name">
401+
<label>Child's name</label>
402+
</input>
403+
<group ref="/data/child_repeat/address">
404+
<label>group</label>
405+
<input ref="/data/child_repeat/address/city">
406+
<label>City</label>
407+
</input>
408+
<input ref="/data/child_repeat/address/country">
409+
<label>Country</label>
410+
</input>
411+
</group>
412+
</repeat>
413+
</group>
414+
</h:body>
415+
</h:html>`,
416+
417+
nestedGroup: `<?xml version="1.0"?>
418+
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
419+
<h:head>
420+
<h:title>nestedGroup</h:title>
421+
<model odk:xforms-version="1.0.0">
422+
<instance>
423+
<data id="nestedGroup">
424+
<text/>
425+
<hospital>
426+
<name/>
427+
<hiv_medication>
428+
<have_hiv_medication/>
429+
</hiv_medication>
430+
</hospital>
431+
<meta>
432+
<instanceID/>
433+
</meta>
434+
</data>
435+
</instance>
436+
<bind nodeset="/data/text" type="string"/>
437+
<bind nodeset="/data/hospital/name" type="string"/>
438+
<bind nodeset="/data/hospital/hiv_medication/have_hiv_medication" type="string"/>
439+
<bind jr:preload="uid" nodeset="/data/meta/instanceID" readonly="true()" type="string"/>
440+
</model>
441+
</h:head>
442+
<h:body>
443+
<input ref="/data/text">
444+
<label>text</label>
445+
</input>
446+
<group ref="/data/hospital">
447+
<label>Hospital</label>
448+
<input ref="/data/hospital/name">
449+
<label>What is the name of this hospital?</label>
450+
</input>
451+
<group ref="/data/hospital/hiv_medication">
452+
<label>HIV Medication</label>
453+
<input ref="/data/hospital/hiv_medication/have_hiv_medication">
454+
<label>Does this hospital have HIV medication?</label>
455+
</input>
456+
</group>
457+
</group>
458+
</h:body>
459+
</h:html>`
358460
},
359461
instances: {
360462
simple: {
@@ -464,6 +566,42 @@ module.exports = {
464566
<name>John</name>
465567
<age>40</age>
466568
</data>`
569+
},
570+
groupRepeat: {
571+
one: `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" id="groupRepeat">
572+
<text>xyz</text>
573+
<child_repeat>
574+
<name>John</name>
575+
<address>
576+
<city>Toronto</city>
577+
<country>Canada</country>
578+
</address>
579+
</child_repeat>
580+
<child_repeat>
581+
<name>Jane</name>
582+
<address>
583+
<city>New York</city>
584+
<country>US</country>
585+
</address>
586+
</child_repeat>
587+
<meta>
588+
<instanceID>uuid:2be07915-2c9c-401a-93ea-1c8f3f8e68f6</instanceID>
589+
</meta>
590+
</data>`
591+
},
592+
nestedGroup: {
593+
one: `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" id="nestedGroup">
594+
<text>xyz</text>
595+
<hospital>
596+
<name>AKUH</name>
597+
<hiv_medication>
598+
<have_hiv_medication>Yes</have_hiv_medication>
599+
</hiv_medication>
600+
</hospital>
601+
<meta>
602+
<instanceID>uuid:f7908274-ef70-4169-90a0-e1389ab732ff</instanceID>
603+
</meta>
604+
</data>`
467605
}
468606
}
469607
};

‎test/integration/api/odata.js

+80-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const { testService } = require('../setup');
22
const { sql } = require('slonik');
33
const testData = require('../../data/xml');
4-
const { dissocPath } = require('ramda');
4+
const { dissocPath, identity } = require('ramda');
55

66
// NOTE: for the data output tests, we do not attempt to extensively determine if every
77
// internal case is covered; there are already two layers of tests below these, at
@@ -1251,6 +1251,56 @@ describe('api: /forms/:id.svc', () => {
12511251
});
12521252
}))));
12531253

1254+
it('should return toplevel rows with group properties', testService(async (service) => {
1255+
const asAlice = await withSubmissions(service, identity);
1256+
1257+
await asAlice.get('/v1/projects/1/forms/withrepeat.svc/Submissions?$select=meta')
1258+
.expect(200)
1259+
.then(({ body }) => {
1260+
body.should.eql({
1261+
'@odata.context': 'http://localhost:8989/v1/projects/1/forms/withrepeat.svc/$metadata#Submissions',
1262+
value: [
1263+
{ meta: { instanceID: 'rthree' } },
1264+
{ meta: { instanceID: 'rtwo' } },
1265+
{ meta: { instanceID: 'rone' } }
1266+
]
1267+
});
1268+
});
1269+
}));
1270+
1271+
it('should return toplevel row with nested group properties', testService(async (service) => {
1272+
const asAlice = await service.login('alice', identity);
1273+
1274+
await asAlice.post('/v1/projects/1/forms?publish=true')
1275+
.send(testData.forms.nestedGroup)
1276+
.set('Content-Type', 'text/xml')
1277+
.expect(200);
1278+
1279+
await asAlice.post('/v1/projects/1/forms/nestedGroup/submissions?deviceID=testid')
1280+
.send(testData.instances.nestedGroup.one)
1281+
.set('Content-Type', 'text/xml')
1282+
.expect(200);
1283+
1284+
await asAlice.get('/v1/projects/1/forms/nestedGroup.svc/Submissions?$select=text,hospital')
1285+
.expect(200)
1286+
.then(({ body }) => {
1287+
body.should.eql({
1288+
'@odata.context': 'http://localhost:8989/v1/projects/1/forms/nestedGroup.svc/$metadata#Submissions',
1289+
value: [
1290+
{
1291+
text: 'xyz',
1292+
hospital: {
1293+
name: 'AKUH',
1294+
hiv_medication: {
1295+
have_hiv_medication: 'Yes',
1296+
},
1297+
},
1298+
},
1299+
]
1300+
});
1301+
});
1302+
}));
1303+
12541304
it('should return subtable results with selected properties', testService((service) =>
12551305
withSubmissions(service, (asAlice) =>
12561306
asAlice.get('/v1/projects/1/forms/withrepeat.svc/Submissions.children.child?$select=__id,name')
@@ -1270,6 +1320,35 @@ describe('api: /forms/:id.svc', () => {
12701320
}]
12711321
});
12721322
}))));
1323+
1324+
it('should return subtable results with group properties', testService(async (service) => {
1325+
const asAlice = await service.login('alice', identity);
1326+
1327+
await asAlice.post('/v1/projects/1/forms?publish=true')
1328+
.send(testData.forms.groupRepeat)
1329+
.set('Content-Type', 'text/xml')
1330+
.expect(200);
1331+
1332+
await asAlice.post('/v1/projects/1/forms/groupRepeat/submissions?deviceID=testid')
1333+
.send(testData.instances.groupRepeat.one)
1334+
.set('Content-Type', 'text/xml')
1335+
.expect(200);
1336+
1337+
await asAlice.get('/v1/projects/1/forms/groupRepeat.svc/Submissions.child_repeat?$select=address')
1338+
.expect(200)
1339+
.then(({ body }) => {
1340+
body.should.eql({
1341+
'@odata.context': 'http://localhost:8989/v1/projects/1/forms/groupRepeat.svc/$metadata#Submissions.child_repeat',
1342+
value: [
1343+
{ address: { city: 'Toronto', country: 'Canada' } },
1344+
{ address: { city: 'New York', country: 'US' } }
1345+
]
1346+
});
1347+
});
1348+
}));
1349+
1350+
1351+
12731352
});
12741353

12751354
describe('/draft.svc', () => {

0 commit comments

Comments
 (0)