Skip to content

Commit fd2c6ea

Browse files
ktuitektuite
authored
Rename getter functions in Dataset query module (getodk#807)
* Refactoring dataset getters in query module * Etag fix * Change dataset metadata function args and name * Rename get list of datasets function * Rename name dataset get function * More simplifying dataset names * Changed a test
1 parent 8ca3317 commit fd2c6ea

File tree

8 files changed

+242
-290
lines changed

8 files changed

+242
-290
lines changed

‎lib/model/query/datasets.js‎

Lines changed: 181 additions & 218 deletions
Large diffs are not rendered by default.

‎lib/model/query/form-attachments.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ const createNew = (xml, form, itemsets) => ({ Blobs, Datasets, run }) =>
5858
.then((expected) => [expected])
5959
.then(itemsetsHack(Blobs, itemsets))
6060
.then((expected) =>
61-
Datasets.getAllByProjectId(form.projectId)
61+
Datasets.getList(form.projectId)
6262
.then(datasets => {
6363
const dsHashtable = datasets.reduce((acc, ds) => Object.assign(acc, { [`${ds.name}.csv`]: ds }), {});
6464
const ids = { formId: form.id, formDefId: form.def.id };
@@ -77,7 +77,7 @@ const createVersion = (xml, form, savedDef, itemsets, publish = false) => ({ Blo
7777
])
7878
.then(ignoringResult(itemsetsHack(Blobs, itemsets)))
7979
.then(([expecteds, extants]) =>
80-
Datasets.getAllByProjectId(form.projectId)
80+
Datasets.getList(form.projectId)
8181
.then(datasets => {
8282
const dsHashtable = datasets.reduce((acc, ds) => Object.assign(acc, { [`${ds.name}.csv`]: ds }), {});
8383
// deal with attachments. match up extant ones with now-expected ones,

‎lib/resources/datasets.js‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,24 @@ module.exports = (service, endpoint) => {
1818
Projects.getById(params.id, queryOptions)
1919
.then(getOrNotFound)
2020
.then((project) => auth.canOrReject('dataset.list', project))
21-
.then(() => Datasets.getAllByProjectId(params.id, queryOptions))));
21+
.then(() => Datasets.getList(params.id, queryOptions))));
2222

23-
service.get('/projects/:projectId/datasets/:name', endpoint(({ Datasets, Projects }, { params, auth }) =>
24-
Projects.getById(params.projectId)
23+
service.get('/projects/:projectId/datasets/:name', endpoint(({ Datasets }, { params, auth }) =>
24+
Datasets.get(params.projectId, params.name)
2525
.then(getOrNotFound)
26-
.then((project) => auth.canOrReject('dataset.read', project))
27-
.then(() => Datasets.getDatasetMetadata(params.name, params.projectId)
28-
.then(getOrNotFound))));
26+
.then((dataset) => auth.canOrReject('dataset.read', dataset)
27+
.then(() => Datasets.getMetadata(dataset)))));
2928

3029
service.get('/projects/:projectId/datasets/:name/entities.csv', endpoint(async ({ Datasets, Entities, Projects }, { params, auth }, request, response) => {
3130
const project = await Projects.getById(params.projectId).then(getOrNotFound);
3231
await auth.canOrReject('entity.list', project);
3332

34-
const dataset = await Datasets.getByName(params.name, params.projectId).then(getOrNotFound);
33+
const dataset = await Datasets.get(params.projectId, params.name, true, true).then(getOrNotFound);
34+
const properties = await Datasets.getProperties(dataset.id);
35+
const { lastEntity } = dataset.forApi();
3536

3637
// Etag logic inspired from https://stackoverflow.com/questions/72334843/custom-computed-etag-for-express-js/72335674#72335674
37-
const serverEtag = `"${md5sum(dataset.lastEntity?.toISOString() ?? '1970-01-01')}"`;
38+
const serverEtag = `"${md5sum(lastEntity?.toISOString() ?? '1970-01-01')}"`;
3839
const clientEtag = request.get('If-None-Match');
3940
if (clientEtag?.includes(serverEtag)) { // nginx weakens Etag when gzip is used, so clientEtag is like W/"4e9f0c7e9a8240..."
4041
response.status(304);
@@ -46,7 +47,7 @@ module.exports = (service, endpoint) => {
4647
response.append('Content-Disposition', contentDisposition(`${filename}.${extension}`));
4748
response.append('Content-Type', 'text/csv');
4849
response.set('ETag', serverEtag);
49-
return streamEntityCsv(entities, dataset.properties);
50+
return streamEntityCsv(entities, properties);
5051

5152
}));
5253
};

‎lib/resources/forms.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ module.exports = (service, endpoint) => {
166166
.then(([form]) => ensureDef(form))
167167
.then((form) => (form.aux.def.keyId
168168
? [] // return empty array if encryption is enabled
169-
: Datasets.getDatasetDiff(params.projectId, params.id, true)))));
169+
: Datasets.getDiff(params.projectId, params.id, true)))));
170170

171171
service.get('/projects/:projectId/forms/:id/dataset-diff', endpoint(({ Forms, Datasets, Projects }, { params, auth }) =>
172172
Promise.all([
@@ -186,14 +186,14 @@ module.exports = (service, endpoint) => {
186186
.then(([form]) => ensureDef(form))
187187
.then((form) => (form.aux.def.keyId
188188
? [] // return empty array if encryption is enabled
189-
: Datasets.getDatasetDiff(params.projectId, params.id, false)))));
189+
: Datasets.getDiff(params.projectId, params.id, false)))));
190190

191191
service.patch('/projects/:projectId/forms/:id/draft/attachments/:name', endpoint(({ Datasets, FormAttachments, Forms }, { auth, params, body }) =>
192192
Forms.getByProjectAndXmlFormId(params.projectId, params.id, false, Form.DraftVersion)
193193
.then(getOrNotFound)
194194
.then((form) => auth.canOrReject('form.update', form))
195195
.then((form) => Promise.all([
196-
Datasets.getByProjectAndName(params.projectId, params.name.replace(/\.csv$/i, ''), true)
196+
Datasets.get(params.projectId, params.name.replace(/\.csv$/i, ''), true)
197197
.then(getOrNotFound)
198198
.then((dataset) => auth.canOrReject('entity.list', dataset)),
199199
FormAttachments.getByFormDefIdAndName(form.draftDefId, params.name).then(getOrNotFound)
@@ -283,12 +283,12 @@ module.exports = (service, endpoint) => {
283283
? Blobs.getById(attachment.blobId)
284284
.then(getOrNotFound)
285285
.then((blob) => binary(blob.contentType, attachment.name, blob.content))
286-
: Datasets.getById(attachment.datasetId, params.projectId)
287-
.then((dataset) => Entities.streamForExport(attachment.datasetId)
286+
: Datasets.getProperties(attachment.datasetId)
287+
.then((properties) => Entities.streamForExport(attachment.datasetId)
288288
.then((entities) => {
289289
response.append('Content-Disposition', contentDisposition(`${attachment.name}`));
290290
response.append('Content-Type', 'text/csv');
291-
return streamEntityCsv(entities, dataset.properties);
291+
return streamEntityCsv(entities, properties);
292292
}))))))));
293293
};
294294

‎lib/resources/odata-entities.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,27 +26,29 @@ module.exports = (service, endpoint) => {
2626
service.get('/projects/:projectId/datasets/:name.svc', endpoint.odata.json(async ({ Datasets, Projects, env }, { auth, params, originalUrl }) => {
2727
const project = await Projects.getById(params.projectId).then(getOrNotFound);
2828
await auth.canOrReject('entity.list', project);
29-
await Datasets.getByName(params.name, params.projectId).then(getOrNotFound);
29+
await Datasets.get(params.projectId, params.name, true).then(getOrNotFound);
3030
return contentType('application/json; odata.metadata=minimal')(serviceDocument(env.domain, originalUrl));
3131
}));
3232

3333
// serves a metadata document describing the properties in the Dataset
3434
service.get('/projects/:projectId/datasets/:name.svc/([$])metadata', endpoint.odata.xml(async ({ Datasets, Projects }, { auth, params }) => {
3535
const project = await Projects.getById(params.projectId).then(getOrNotFound);
3636
await auth.canOrReject('entity.list', project);
37-
const dataset = await Datasets.getByName(params.name, params.projectId).then(getOrNotFound);
38-
return xml(edmxForEntities(params.name, dataset.properties));
37+
const dataset = await Datasets.get(params.projectId, params.name, true).then(getOrNotFound);
38+
const properties = await Datasets.getProperties(dataset.id);
39+
return xml(edmxForEntities(params.name, properties));
3940
}));
4041

4142
// serves the odata feed for the entities
4243
service.get('/projects/:projectId/datasets/:name.svc/Entities', endpoint.odata.json(async ({ Datasets, Entities, Projects, env }, { auth, params, originalUrl, query }) => {
4344
const project = await Projects.getById(params.projectId).then(getOrNotFound);
4445
await auth.canOrReject('entity.list', project);
45-
const dataset = await Datasets.getByName(params.name, params.projectId).then(getOrNotFound);
46+
const dataset = await Datasets.get(params.projectId, params.name, true).then(getOrNotFound);
47+
const properties = await Datasets.getProperties(dataset.id);
4648
const options = QueryOptions.fromODataRequestEntities(query);
4749
const count = await Entities.countByDatasetId(dataset.id, options);
4850
const entities = await Entities.streamForExport(dataset.id, options);
49-
return json(streamEntityOdata(entities, dataset.properties, env.domain, originalUrl, query, count));
51+
return json(streamEntityOdata(entities, properties, env.domain, originalUrl, query, count));
5052
}));
5153

5254

‎test/assertions.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -356,10 +356,8 @@ should.Assertion.add('Dataset', function assertDataset() {
356356
should.Assertion.add('ExtendedDataset', function assertExtendedDataset() {
357357
this.params = { operator: 'to be an extended Dataset' };
358358

359-
Object.keys(this.obj).should.containDeep([ 'projectId', 'name', 'createdAt', 'entities', 'lastEntity' ]);
360-
this.obj.projectId.should.be.a.Number();
361-
this.obj.name.should.be.a.String();
362-
this.obj.createdAt.should.be.an.isoDate();
359+
this.obj.should.be.a.Dataset();
360+
Object.keys(this.obj).should.containDeep([ 'entities', 'lastEntity' ]);
363361
this.obj.entities.should.be.a.Number();
364362
if (this.obj.lastEntity != null) this.obj.lastEntity.should.be.an.isoDate();
365363
});

‎test/integration/api/datasets.js‎

Lines changed: 30 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,7 @@ describe('datasets and entities', () => {
832832
.expect(200))
833833
.then(() => Promise.all([
834834
Forms.getByProjectAndXmlFormId(1, 'withAttachments', false, Form.DraftVersion).then(getOrNotFound),
835-
Datasets.getByProjectAndName(1, 'goodone').then(getOrNotFound)
835+
Datasets.get(1, 'goodone').then(getOrNotFound)
836836
]))
837837
.then(([form, dataset]) => FormAttachments.getByFormDefIdAndName(form.draftDefId, 'goodone.csv').then(getOrNotFound)
838838
.then(attachment => asAlice.patch('/v1/projects/1/forms/withAttachments/draft/attachments/goodone.csv')
@@ -1096,7 +1096,7 @@ describe('datasets and entities', () => {
10961096
.expect(200))
10971097
.then(() => Promise.all([
10981098
Forms.getByProjectAndXmlFormId(1, 'withAttachments', false, Form.DraftVersion).then(getOrNotFound),
1099-
Datasets.getByProjectAndName(1, 'goodone').then(getOrNotFound)
1099+
Datasets.get(1, 'goodone').then(getOrNotFound)
11001100
]))
11011101
.then(([form, dataset]) => FormAttachments.getByFormDefIdAndName(form.draftDefId, 'goodone.csv').then(getOrNotFound)
11021102
.then((attachment) => FormAttachments.update(form, attachment, 1, dataset.id)
@@ -1114,7 +1114,7 @@ describe('datasets and entities', () => {
11141114
.send(testData.forms.simpleEntity))
11151115
.then(() => Promise.all([
11161116
Forms.getByProjectAndXmlFormId(1, 'withAttachments', false, Form.DraftVersion).then(getOrNotFound),
1117-
Datasets.getByProjectAndName(1, 'people').then(getOrNotFound)
1117+
Datasets.get(1, 'people').then(getOrNotFound)
11181118
]))
11191119
.then(([form, dataset]) => FormAttachments.getByFormDefIdAndName(form.draftDefId, 'goodtwo.mp3').then(getOrNotFound)
11201120
.then((attachment) => FormAttachments.update(form, attachment, null, dataset.id)
@@ -1580,51 +1580,39 @@ describe('datasets and entities', () => {
15801580
}));
15811581
}));
15821582

1583-
it('should update a dataset with new form draft', testService(async (service, { Datasets }) => {
1583+
it('should update a dataset with a new draft and be able to upload multiple drafts', testService(async (service) => {
1584+
const asAlice = await service.login('alice');
1585+
15841586
// Upload a form and then create a new draft version
1585-
await service.login('alice', (asAlice) =>
1586-
asAlice.post('/v1/projects/1/forms?publish=true')
1587-
.send(testData.forms.simpleEntity)
1588-
.set('Content-Type', 'application/xml')
1587+
await asAlice.post('/v1/projects/1/forms?publish=true')
1588+
.send(testData.forms.simpleEntity)
1589+
.set('Content-Type', 'application/xml')
1590+
.expect(200)
1591+
.then(() => asAlice.post('/v1/projects/1/forms/simpleEntity/draft')
15891592
.expect(200)
15901593
.then(() => asAlice.post('/v1/projects/1/forms/simpleEntity/draft')
1594+
.send(testData.forms.simpleEntity)
1595+
.set('Content-Type', 'application/xml')
1596+
.expect(200))
1597+
.then(() => asAlice.get('/v1/projects/1/forms/simpleEntity/draft')
1598+
.set('X-Extended-Metadata', 'true')
15911599
.expect(200)
1592-
.then(() => asAlice.get('/v1/projects/1/forms/simpleEntity/draft')
1593-
.set('X-Extended-Metadata', 'true')
1594-
.expect(200)
1595-
.then(({ body }) => {
1596-
body.entityRelated.should.equal(true);
1597-
}))));
1598-
1599-
// Get all datasets by projectId
1600-
const datasetId = await Datasets.getAllByProjectId(1)
1601-
.then(result => result[0].id);
1600+
.then(({ body }) => {
1601+
body.entityRelated.should.equal(true);
1602+
})));
16021603

1603-
await Datasets.getById(datasetId)
1604-
.then(result => {
1605-
result.properties.length.should.be.eql(2);
1604+
await asAlice.get('/v1/projects/1/datasets')
1605+
.expect(200)
1606+
.then(({ body }) => {
1607+
body[0].name.should.be.eql('people');
16061608
});
1607-
}));
16081609

1609-
it('should be able to upload multiple drafts', testService(async (service) => {
1610-
// Upload a form and then create a new draft version
1611-
await service.login('alice', (asAlice) =>
1612-
asAlice.post('/v1/projects/1/forms?publish=true')
1613-
.send(testData.forms.simpleEntity)
1614-
.set('Content-Type', 'application/xml')
1615-
.expect(200)
1616-
.then(() => asAlice.post('/v1/projects/1/forms/simpleEntity/draft')
1617-
.expect(200)
1618-
.then(() => asAlice.post('/v1/projects/1/forms/simpleEntity/draft')
1619-
.send(testData.forms.simpleEntity)
1620-
.set('Content-Type', 'application/xml')
1621-
.expect(200))
1622-
.then(() => asAlice.get('/v1/projects/1/forms/simpleEntity/draft')
1623-
.set('X-Extended-Metadata', 'true')
1624-
.expect(200)
1625-
.then(({ body }) => {
1626-
body.entityRelated.should.equal(true);
1627-
}))));
1610+
await asAlice.get('/v1/projects/1/datasets/people')
1611+
.expect(200)
1612+
.then(({ body }) => {
1613+
body.name.should.be.eql('people');
1614+
body.properties.length.should.be.eql(2);
1615+
});
16281616
}));
16291617

16301618
it('should not let multiple fields to be mapped to a single property', testService(async (service) => {
@@ -1712,7 +1700,7 @@ describe('datasets and entities', () => {
17121700

17131701
await Audits.getLatestByAction('dataset.update.publish')
17141702
.then(o => o.get())
1715-
.then(audit => audit.details.should.eql({ properties: ['age', 'first_name'] }));
1703+
.then(audit => audit.details.should.eql({ properties: ['first_name', 'age'] }));
17161704

17171705
await asAlice.post('/v1/projects/1/forms')
17181706
.send(testData.forms.simpleEntity

‎test/integration/worker/entity.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -451,10 +451,10 @@ describe('worker: entity', () => {
451451
const { text } = await asAlice.get('/v1/projects/1/datasets/foo/entities.csv');
452452

453453
const csv = text.split('\n');
454-
csv[0].includes('name,label,b_q1,d_q2,a_q3,c_q4,f_q1,e_q2').should.equal(true);
455-
csv[1].includes(',one,,,y,z,w,x').should.equal(true);
456-
csv[2].includes(',two,a,b,c,d,,').should.equal(true);
457-
csv[3].includes(',one,w,x,y,z,,').should.equal(true);
454+
csv[0].includes('name,label,f_q1,e_q2,a_q3,c_q4,b_q1,d_q2').should.equal(true);
455+
csv[1].includes(',one,w,x,y,z,,').should.equal(true);
456+
csv[2].includes(',two,,,c,d,a,b').should.equal(true);
457+
csv[3].includes(',one,,,y,z,w,x').should.equal(true);
458458
}));
459459
});
460460
});

0 commit comments

Comments
 (0)