POST submission attachments: reduce audit logging overhead

In the submission attachment POST endpoint and a few other submission-related endpoints, a full submission definition including XML is fetched from the database, where only one or two columns from that result are used.

This commit reduces the data fetched from the database to only those which are required.

lib/model/query/submission-attachments.js

@@ -107,12 +107,12 @@ const upsert = (def, files) => ({ Blobs, SubmissionAttachments }) =>
       const present = files.filter((file) => lookup.has(file.fieldname));
       return Promise.all(present
         .map((file) => Blobs.ensure(Blob.fromBuffer(file.buffer, file.mimetype))
-          .then((blobId) => SubmissionAttachments.attach(def, file.fieldname, blobId))));
+          .then((blobId) => SubmissionAttachments.attach(def.id, file.fieldname, blobId))));
     });
 
-const attach = (def, name, blobId) => ({ run }) => run(sql`
+const attach = (defId, name, blobId) => ({ run }) => run(sql`
 update submission_attachments set "blobId"=${blobId}
-where "submissionDefId"=${def.id} and name=${name}`);
+where "submissionDefId"=${defId} and name=${name}`);
 
 // TODO: this is currently audit logged in resource/submissions. probably deal w it here instead.
 

lib/model/query/submissions.js

@@ -18,6 +18,7 @@ const { unjoiner, extender, sqlEquals, page, updater, QueryOptions, insertMany,
 const { blankStringToNull, construct } = require('../../util/util');
 const Problem = require('../../util/problem');
 const { streamEncBlobs } = require('../../util/blob');
+const Option = require('../../util/option');
 
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -269,8 +270,8 @@ where submissions."instanceId"=${instanceId}
   and submission_defs.current=true`)
   .then(map((row) => new Submission(row, { def: new Submission.Def({ id: row.defId }) })));
 
-const getCurrentDefByIds = (projectId, xmlFormId, instanceId, draft) => ({ maybeOne }) => maybeOne(sql`
-select submission_defs.* from submission_defs
+const _buildGetCurrentSql = (cols, projectId, xmlFormId, instanceId, draft) => sql`
+select ${cols} from submission_defs
 inner join
   (select submissions.id, "instanceId" from submissions
     inner join
@@ -280,8 +281,19 @@ inner join
     where submissions."deletedAt" is null and draft=${draft}) as submissions
   on submissions.id=submission_defs."submissionId"
 where submissions."instanceId"=${instanceId} and current=true
-limit 1`)
-  .then(map(construct(Submission.Def)));
+limit 1`;
+
+const getCurrentDefColByIds = (col, projectId, xmlFormId, instanceId, draft) => ({ maybeOneFirst }) =>
+  maybeOneFirst(_buildGetCurrentSql(sql.identifier(['submission_defs', col]), projectId, xmlFormId, instanceId, draft))
+    .then(map(Option.of));
+
+const getCurrentDefColsByIds = (cols, projectId, xmlFormId, instanceId, draft) => ({ maybeOne }) =>
+  maybeOne(_buildGetCurrentSql(sql.join(cols.map(col => sql.identifier(['submission_defs', col])), sql`,`), projectId, xmlFormId, instanceId, draft))
+    .then(map(Option.of));
+
+const getCurrentDefByIds = (projectId, xmlFormId, instanceId, draft) => ({ maybeOne }) =>
+  maybeOne(_buildGetCurrentSql(sql`submission_defs.*`, projectId, xmlFormId, instanceId, draft))
+    .then(map(construct(Submission.Def)));
 
 const getDefById = (submissionDefId) => ({ maybeOne }) => maybeOne(sql`
 select submission_defs.* from submission_defs
@@ -480,7 +492,7 @@ module.exports = {
   setSelectMultipleValues, getSelectMultipleValuesForExport,
   getByIdsWithDef, getSubAndDefById,
   getByIds, getAllForFormByIds, getById, countByFormId, verifyVersion,
-  getDefById, getCurrentDefByIds, getAnyDefByFormAndInstanceId, getDefsByFormAndLogicalId, getDefBySubmissionAndInstanceId, getRootForInstanceId,
+  getDefById, getCurrentDefByIds, getCurrentDefColByIds, getCurrentDefColsByIds, getAnyDefByFormAndInstanceId, getDefsByFormAndLogicalId, getDefBySubmissionAndInstanceId, getRootForInstanceId,
   getDeleted,
   streamForExport, getForExport
 };

lib/resources/submissions.js

@@ -206,7 +206,7 @@ module.exports = (service, endpoint) => {
         const deprecatedId = partial.deprecatedId.orElseGet(() => { throw Problem.user.expectedDeprecation(); });
         return Promise.all([
           // TODO/PERF: a bespoke query here could save some round-trips
-          Submissions.getCurrentDefByIds(params.projectId, params.formId, params.instanceId, draft)
+          Submissions.getCurrentDefColsByIds(['instanceId', 'submissionId'], params.projectId, params.formId, params.instanceId, draft)
             .then(getOrNotFound)
             .then(rejectIf(((current) => current.instanceId !== deprecatedId),
               () => Problem.user.deprecatingOldSubmission(({ deprecatedId })))),
@@ -413,9 +413,9 @@ module.exports = (service, endpoint) => {
 
     service.get(`${base}/:instanceId.xml`, endpoint(({ Forms, Submissions }, context) =>
       getOrRedirect(Forms, Submissions, context)
-        .then(() => Submissions.getCurrentDefByIds(context.params.projectId, context.params.formId, context.params.instanceId, draft))
+        .then(() => Submissions.getCurrentDefColByIds('xml', context.params.projectId, context.params.formId, context.params.instanceId, draft))
         .then(getOrNotFound)
-        .then((def) => xml(def.xml))));
+        .then((defXml) => xml(defXml))));
 
     service.get(`${base}/:instanceId`, endpoint(({ Forms, Submissions }, context) =>
       getOrRedirect(Forms, Submissions, context)
@@ -434,25 +434,24 @@ module.exports = (service, endpoint) => {
         .then(getOrNotFound)
         .then((blob) => blobResponse(s3, context.params.name, blob))));
 
-    // TODO: wow audit-logging this is expensive.
     service.post(
       `${base}/:instanceId/attachments/:name`,
       endpoint(({ Audits, Blobs, Forms, SubmissionAttachments, Submissions }, { params, headers, auth }, request) =>
         Promise.all([
           getForm(params, Forms)
             .then((form) => auth.canOrReject('submission.update', form))
-            .then((form) => Submissions.getCurrentDefByIds(form.projectId, form.xmlFormId, params.instanceId, draft)
+            .then((form) => Submissions.getCurrentDefColByIds('id', form.projectId, form.xmlFormId, params.instanceId, draft)
               .then(getOrNotFound)
-              .then((def) => SubmissionAttachments.getBySubmissionDefIdAndName(def.id, params.name) // just for audit logging
+              .then((defId) => SubmissionAttachments.getBySubmissionDefIdAndName(defId, params.name) // just for audit logging
                 .then(getOrNotFound)
-                .then((oldAttachment) => [ form, def, oldAttachment ]))),
+                .then((oldAttachment) => [ form, defId, oldAttachment ]))),
           Blob.fromStream(request, headers['content-type']).then(Blobs.ensure)
         ])
-          .then(([ [ form, def, oldAttachment ], blobId ]) => Promise.all([
-            SubmissionAttachments.attach(def, params.name, blobId),
+          .then(([ [ form, defId, oldAttachment ], blobId ]) => Promise.all([
+            SubmissionAttachments.attach(defId, params.name, blobId),
             Audits.log(auth.actor, 'submission.attachment.update', form, {
               instanceId: params.instanceId,
-              submissionDefId: def.id,
+              submissionDefId: defId,
               name: params.name,
               oldBlobId: oldAttachment.blobId,
               newBlobId: blobId
@@ -469,9 +468,9 @@ module.exports = (service, endpoint) => {
       endpoint(({ Forms, SubmissionAttachments, Submissions }, { params, auth }) =>
         getForm(params, Forms)
           .then((form) => auth.canOrReject('submission.update', form))
-          .then((form) => Submissions.getCurrentDefByIds(form.projectId, form.xmlFormId, params.instanceId, draft)
+          .then((form) => Submissions.getCurrentDefColByIds('id', form.projectId, form.xmlFormId, params.instanceId, draft)
             .then(getOrNotFound)
-            .then((def) => SubmissionAttachments.getBySubmissionDefIdAndName(def.id, params.name))
+            .then((defId) => SubmissionAttachments.getBySubmissionDefIdAndName(defId, params.name))
             .then(getOrNotFound)
             .then((attachment) => SubmissionAttachments.clear(attachment, form, params.instanceId)))
           .then(success))