ETag for entities.csv (getodk#776)

* fix: it should return md5 of timestamp of latest entity

* fix: coalesce of updatedAt and createdAt for lastEntity timestamp

* Enhance: Return etag in entity.csv response

Author: sadiqkhoja

lib/formats/openrosa.js

@@ -76,7 +76,7 @@ const formManifestTemplate = template(200, `<?xml version="1.0" encoding="UTF-8"
 const formManifest = (data) => formManifestTemplate(mergeRight(data, {
   attachments: data.attachments.map((attachment) =>
     attachment.with({ hasSource: attachment.blobId || attachment.datasetId,
-      md5: attachment.blobId ? attachment.aux.openRosa.md5 : md5sum(attachment.aux.openRosa.dsUpdatedAt ?? '1970-01-01'),
+      md5: attachment.blobId ? attachment.aux.openRosa.md5 : md5sum(attachment.aux.openRosa.dsUpdatedAt?.toISOString() ?? '1970-01-01'),
       urlName: encodeURIComponent(attachment.name) }))
 }));
 

lib/model/query/datasets.js

@@ -24,7 +24,7 @@ const pickFrameFields = (frame, obj) => compose(
 )(obj);
 
 const makeHierarchy = reduce((result, item) => {
-  const dataset = new Dataset(pickFrameFields(Dataset, item));
+  const dataset = new Dataset(pickFrameFields(Dataset, item)).with({ lastEntity: item.lastEntity });
   const property = new Dataset.Property(pickFrameFields(Dataset.Property, item));
 
   return {
@@ -50,7 +50,7 @@ const _getAllByProjectId = (fields, extend, options) => sql`
   SELECT ${fields} FROM datasets
   ${extend|| sql`
   LEFT JOIN (
-    SELECT "datasetId", count(1) "entities", max("createdAt") "lastEntity" 
+    SELECT "datasetId", COUNT(1) "entities", MAX(COALESCE("updatedAt", "createdAt")) "lastEntity" 
     FROM entities e 
     GROUP BY "datasetId" 
   ) stats on stats."datasetId" = datasets.id`}
@@ -130,6 +130,11 @@ const _getByNameSql = ((fields, datasetName, projectId, includeForms) => sql`
         ${fields}
     FROM
         datasets
+    LEFT OUTER JOIN (
+      SELECT "datasetId", COUNT(1) "entities", MAX(COALESCE("updatedAt", "createdAt")) "lastEntity" 
+      FROM entities e 
+      GROUP BY "datasetId" 
+    ) stats on stats."datasetId" = datasets.id
     LEFT OUTER JOIN ds_properties ON
         datasets.id = ds_properties."datasetId" AND ds_properties."publishedAt" IS NOT NULL
     ${includeForms ? sql`
@@ -198,7 +203,7 @@ const getById = (datasetId) => ({ all }) =>
 
 // Returns only published dataset with its published properties
 const getByName = (datasetName, projectId) => ({ all }) =>
-  all(_getByNameSql(unjoiner(Dataset, Dataset.Property).fields, datasetName, projectId))
+  all(_getByNameSql(unjoiner(Dataset, Dataset.Property, Dataset.Extended).fields, datasetName, projectId))
     .then(makeHierarchy)
     .then(asArray)
     .then(nth(0))

lib/resources/datasets.js

@@ -11,6 +11,7 @@ const sanitize = require('sanitize-filename');
 const { getOrNotFound } = require('../util/promise');
 const { streamEntityCsv } = require('../data/entity');
 const { contentDisposition } = require('../util/http');
+const { md5sum } = require('../util/crypto');
 
 module.exports = (service, endpoint) => {
   service.get('/projects/:id/datasets', endpoint(({ Projects, Datasets }, { auth, params, queryOptions }) =>
@@ -26,18 +27,26 @@ module.exports = (service, endpoint) => {
       .then(() => Datasets.getDatasetMetadata(params.name, params.projectId)
         .then(getOrNotFound))));
 
-  service.get('/projects/:projectId/datasets/:name/entities.csv', endpoint(({ Datasets, Entities, Projects }, { params, auth }, _, response) =>
-    Projects.getById(params.projectId)
-      .then(getOrNotFound)
-      .then((project) => auth.canOrReject('entity.list', project))
-      .then(() => Datasets.getByName(params.name, params.projectId)
-        .then(getOrNotFound)
-        .then((dataset) => Entities.streamForExport(dataset.id)
-          .then((entities) => {
-            const filename = sanitize(dataset.name);
-            const extension = 'csv';
-            response.append('Content-Disposition', contentDisposition(`${filename}.${extension}`));
-            response.append('Content-Type', 'text/csv');
-            return streamEntityCsv(entities, dataset.properties);
-          })))));
+  service.get('/projects/:projectId/datasets/:name/entities.csv', endpoint(async ({ Datasets, Entities, Projects }, { params, auth }, request, response) => {
+    const project = await Projects.getById(params.projectId).then(getOrNotFound);
+    await auth.canOrReject('entity.list', project);
+
+    const dataset = await Datasets.getByName(params.name, params.projectId).then(getOrNotFound);
+
+    // Etag logic inspired from https://stackoverflow.com/questions/72334843/custom-computed-etag-for-express-js/72335674#72335674
+    const serverEtag = `"${md5sum(dataset.lastEntity?.toISOString() ?? '1970-01-01')}"`;
+    const clientEtag = request.get('If-None-Match');
+    if (clientEtag?.includes(serverEtag)) { // nginx weakens Etag when gzip is used, so clientEtag is like W/"4e9f0c7e9a8240..."
+      response.status(304);
+      return;
+    }
+    const entities = await Entities.streamForExport(dataset.id);
+    const filename = sanitize(dataset.name);
+    const extension = 'csv';
+    response.append('Content-Disposition', contentDisposition(`${filename}.${extension}`));
+    response.append('Content-Type', 'text/csv');
+    response.set('ETag', serverEtag);
+    return streamEntityCsv(entities, dataset.properties);
+
+  }));
 };

test/integration/api/datasets.js

@@ -327,6 +327,71 @@ describe('datasets and entities', () => {
             .expect(200)
             .then(() => asAlice.get('/v1/projects/1/datasets/people/entities.csv')
               .expect(404)))));
+
+      it('should return csv file with data', testService(async (service, container) => {
+        const asAlice = await service.login('alice');
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.simpleEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms/simpleEntity/submissions')
+          .send(testData.instances.simpleEntity.one)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.patch('/v1/projects/1/forms/simpleEntity/submissions/one')
+          .send({ reviewState: 'approved' })
+          .expect(200);
+
+        await exhaust(container);
+
+        const result = await asAlice.get('/v1/projects/1/datasets/people/entities.csv')
+          .expect(200)
+          .then(r => r.text);
+
+        result.should.be.eql(
+          'name,label,first_name,age\n' +
+          '12345678-1234-4123-8234-123456789abc,Alice (88),Alice,88\n'
+        );
+
+      }));
+
+      it('should return 304 content not changed if ETag matches', testService(async (service, container) => {
+        const asAlice = await service.login('alice');
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.simpleEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms/simpleEntity/submissions')
+          .send(testData.instances.simpleEntity.one)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.patch('/v1/projects/1/forms/simpleEntity/submissions/one')
+          .send({ reviewState: 'approved' })
+          .expect(200);
+
+        await exhaust(container);
+
+        const result = await asAlice.get('/v1/projects/1/datasets/people/entities.csv')
+          .expect(200);
+
+        result.text.should.be.eql(
+          'name,label,first_name,age\n' +
+          '12345678-1234-4123-8234-123456789abc,Alice (88),Alice,88\n'
+        );
+
+        const etag = result.get('ETag');
+
+        await asAlice.get('/v1/projects/1/datasets/people/entities.csv')
+          .set('If-None-Match', etag)
+          .expect(304);
+      }));
+
     });
 
     describe('projects/:id/datasets/:name GET', () => {
@@ -499,6 +564,52 @@ describe('datasets and entities', () => {
   </manifest>`);
               })))));
 
+      it('should return md5 of last Entity timestamp in the manifest', testService(async (service, container) => {
+        const asAlice = await service.login('alice');
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.simpleEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms/simpleEntity/submissions')
+          .send(testData.instances.simpleEntity.one)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.patch('/v1/projects/1/forms/simpleEntity/submissions/one')
+          .send({ reviewState: 'approved' })
+          .expect(200);
+
+        await exhaust(container);
+
+        const result = await asAlice.get('/v1/projects/1/datasets/people/entities.csv')
+          .expect(200);
+
+        const etag = result.get('ETag');
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.withAttachments.replace(/goodone/g, 'people'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.get('/v1/projects/1/forms/withAttachments/manifest')
+          .set('X-OpenRosa-Version', '1.0')
+          .expect(200)
+          .then(({ text }) => {
+            const domain = config.get('default.env.domain');
+            text.should.be.eql(`<?xml version="1.0" encoding="UTF-8"?>
+  <manifest xmlns="http://openrosa.org/xforms/xformsManifest">
+    <mediaFile>
+      <filename>people.csv</filename>
+      <hash>md5:${etag.replace(/"/g, '')}</hash>
+      <downloadUrl>${domain}/v1/projects/1/forms/withAttachments/attachments/people.csv</downloadUrl>
+    </mediaFile>
+  </manifest>`);
+          });
+
+      }));
+
       it('should override blob and link dataset', testService((service, { Forms, FormAttachments, Audits, Datasets }) =>
         service.login('alice', (asAlice) =>
           asAlice.post('/v1/projects/1/forms')