Skip to content

Commit

Permalink
code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
ktuite committed Nov 14, 2024
1 parent a9e3e05 commit 118ade1
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 22 deletions.
2 changes: 1 addition & 1 deletion lib/model/frames.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ Form.Attachment = class extends Frame.define(
const data = {
name: this.name,
type: this.type,
hash: this.blobHash,
hash: this.aux.blob?.md5,
exists: (this.blobId != null || this.datasetId != null),
blobExists: this.blobId != null,
datasetExists: this.datasetId != null
Expand Down
30 changes: 9 additions & 21 deletions lib/model/query/form-attachments.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,38 +131,26 @@ update.audit = (form, fa, blobId, datasetId = null) => (log) => log('form.attach
// GETTERS

// This unjoiner pulls md5 from blob table (if it exists) and adds it to attachment frame
const _unjoinMd5 = unjoiner(Form.Attachment, Frame.define(into('openRosa'), 'md5'));

// This function returns the blob's md5 hash if it exists, otherwise it returns null.
// This is used for listing the form attachments within Central and unlike `_chooseDynamicHash`
// below, doesn't need to look at the dataset timestamp to compute a dynamic hash.
const _chooseBlobHash = (attachment) => {
if (attachment.blobId) return attachment.with({ blobHash: attachment.aux.openRosa.md5 });
return attachment.with({ blobHash: null });
};
const _unjoinMd5 = unjoiner(Form.Attachment, Frame.define(into('blob'), 'md5'));

const getAllByFormDefId = (formDefId) => ({ all }) =>
all(sql`select ${_unjoinMd5.fields} from form_attachments
left outer join (select id, md5 from blobs) as blobs on form_attachments."blobId"=blobs.id
where "formDefId"=${formDefId} order by name asc`)
.then(map(_unjoinMd5))
.then(map(_chooseBlobHash));
.then(map(_unjoinMd5));

// Does not need to be joined with blobs table due to how it is used
const getByFormDefIdAndName = (formDefId, name) => ({ maybeOne }) => maybeOne(sql`
select ${_unjoinMd5.fields} from form_attachments
left outer join (select id, md5 from blobs) as blobs on form_attachments."blobId"=blobs.id
where "formDefId"=${formDefId} and "name"=${name}`)
.then(map(_unjoinMd5))
.then(map(_chooseBlobHash));
select * from form_attachments where "formDefId"=${formDefId} and "name"=${name}`)
.then(map(construct(Form.Attachment)));

// This function decides on the OpenRosa hash (functionally equivalent to an http Etag)
// It uses the blob md5 directly if it exists.
// If the attachment is actually an entity list, it looks up the last modified time
// in the database, which is computed from the latest dataset/entity audit timestamp.
// It is dynamic because it can change when a dataset's data is updated.
const _chooseDynamicHash = (attachment) => async ({ Datasets }) => {
const { md5 } = attachment.aux.openRosa;
if (attachment.blobId) return attachment.with({ openRosaHash: md5, md5 });
const _chooseOpenRosaHash = (attachment) => async ({ Datasets }) => {
if (attachment.blobId) return attachment.with({ openRosaHash: attachment.aux.blob.md5 });

if (attachment.datasetId) {
const lastTimestamp = await Datasets.getLastUpdateTimestamp(attachment.datasetId);
Expand All @@ -177,14 +165,14 @@ select ${_unjoinMd5.fields} from form_attachments
left outer join (select id, md5 from blobs) as blobs on form_attachments."blobId"=blobs.id
where "formDefId"=${formDefId}`)
.then(map(_unjoinMd5))
.then((attachments) => Promise.all(attachments.map(FormAttachments._chooseDynamicHash)));
.then((attachments) => Promise.all(attachments.map(FormAttachments._chooseOpenRosaHash)));


module.exports = {
createNew, createVersion,
update,
getAllByFormDefId, getByFormDefIdAndName,
getAllByFormDefIdForOpenRosa,
_chooseDynamicHash
_chooseOpenRosaHash
};

0 comments on commit 118ade1

Please sign in to comment.