Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose md5 hash of file attachments through api #1292

Merged
merged 3 commits into from
Nov 14, 2024
Merged

Expose md5 hash of file attachments through api #1292

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5818e7
Expose md5 hash of file attachments through api
ktuite Nov 12, 2024
a9e3e05
updated names of functions and hash/md5 attribute
ktuite Nov 12, 2024
118ade1
code review fixes
ktuite Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/model/frames.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,14 @@ Form.Attachment = class extends Frame.define(
fieldTypes(['int4', 'int4', 'int4', 'int4', 'text', 'text', 'timestamptz'])
) {
forApi() {
const data = { name: this.name, type: this.type, exists: (this.blobId != null || this.datasetId != null), blobExists: this.blobId != null, datasetExists: this.datasetId != null };
const data = {
name: this.name,
type: this.type,
hash: this.blobHash,
exists: (this.blobId != null || this.datasetId != null),
blobExists: this.blobId != null,
datasetExists: this.datasetId != null
};
if (this.updatedAt != null) data.updatedAt = this.updatedAt;
return data;
}
Expand Down
40 changes: 28 additions & 12 deletions lib/model/query/form-attachments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,23 +130,39 @@ 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'));
ktuite marked this conversation as resolved.
Show resolved Hide resolved

// 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 });
ktuite marked this conversation as resolved.
Show resolved Hide resolved
ktuite marked this conversation as resolved.
Show resolved Hide resolved
return attachment.with({ blobHash: null });
};

const getAllByFormDefId = (formDefId) => ({ all }) =>
all(sql`select * from form_attachments where "formDefId"=${formDefId} order by name asc`)
.then(map(construct(Form.Attachment)));
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));

const getByFormDefIdAndName = (formDefId, name) => ({ maybeOne }) => maybeOne(sql`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test that hits this function? All the tests below seem to involve arrays of form attachments, which I'd guess hit getAllByFormDefId(), but not this function. How about adding a test of the /v1/projects/:projectId/forms/:id/attachments/:name endpoint that verifies that hash is added to the response?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This endpoint returns the contents of the file, so it doesn't have the hash. This means that I might not need this particular unjoiner for getByFormDefIdAndName though.

select * from form_attachments where "formDefId"=${formDefId} and "name"=${name}`)
.then(map(construct(Form.Attachment)));

// 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'));
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));

// This function decides on the openrosa hash (functionally equivalent to an http Etag)
// 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.
const _chooseHash = (attachment) => async ({ Datasets }) => {
if (attachment.blobId) return attachment.with({ openRosaHash: attachment.aux.openRosa.md5 });
// It is dynamic because it can change when a dataset's data is updated.
const _chooseDynamicHash = (attachment) => async ({ Datasets }) => {
ktuite marked this conversation as resolved.
Show resolved Hide resolved
const { md5 } = attachment.aux.openRosa;
if (attachment.blobId) return attachment.with({ openRosaHash: md5, md5 });
ktuite marked this conversation as resolved.
Show resolved Hide resolved

if (attachment.datasetId) {
const lastTimestamp = await Datasets.getLastUpdateTimestamp(attachment.datasetId);
Expand All @@ -161,14 +177,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._chooseHash)));
.then((attachments) => Promise.all(attachments.map(FormAttachments._chooseDynamicHash)));


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

16 changes: 8 additions & 8 deletions test/integration/api/forms/draft.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,8 @@ describe('api: /projects/:id/forms (drafts)', () => {
.expect(200)
.then(({ body }) => {
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: false, blobExists: false, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: false, blobExists: false, datasetExists: false, hash: null },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
})))));

Expand Down Expand Up @@ -572,8 +572,8 @@ describe('api: /projects/:id/forms (drafts)', () => {
// eslint-disable-next-line no-param-reassign
delete body[0].updatedAt;
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'greattwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2af2751b79eccfaa8f452331e76e679e' },
{ name: 'greattwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
})))));

Expand Down Expand Up @@ -604,8 +604,8 @@ describe('api: /projects/:id/forms (drafts)', () => {
// eslint-disable-next-line no-param-reassign
delete body[0].updatedAt;
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'greattwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2af2751b79eccfaa8f452331e76e679e' },
{ name: 'greattwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
})))));

Expand Down Expand Up @@ -1139,8 +1139,8 @@ describe('api: /projects/:id/forms (drafts)', () => {
// eslint-disable-next-line no-param-reassign
delete body[0].updatedAt;
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2af2751b79eccfaa8f452331e76e679e' },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
})))));

Expand Down
8 changes: 4 additions & 4 deletions test/integration/api/forms/forms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,8 +1226,8 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
.expect(200)
.then(({ body }) => {
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: false, blobExists: false, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: false, blobExists: false, datasetExists: false, hash: null },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
})))));

Expand All @@ -1252,8 +1252,8 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
delete body[0].updatedAt;

body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2241de57bbec8144c8ad387e69b3a3ba' },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
})))));

Expand Down
4 changes: 2 additions & 2 deletions test/integration/api/forms/versions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,8 @@ describe('api: /projects/:id/forms (versions)', () => {
delete body[0].updatedAt;

body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2af2751b79eccfaa8f452331e76e679e' },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
})))));

Expand Down
12 changes: 6 additions & 6 deletions test/integration/other/form-entities-version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,8 @@ describe('Update / migrate entities-version within form', () => {
// eslint-disable-next-line no-param-reassign
delete body[0].updatedAt;
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2241de57bbec8144c8ad387e69b3a3ba' },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
});

Expand Down Expand Up @@ -318,8 +318,8 @@ describe('Update / migrate entities-version within form', () => {
// eslint-disable-next-line no-param-reassign
delete body[0].updatedAt;
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2241de57bbec8144c8ad387e69b3a3ba' },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
});

Expand All @@ -329,8 +329,8 @@ describe('Update / migrate entities-version within form', () => {
// eslint-disable-next-line no-param-reassign
delete body[0].updatedAt;
body.should.eql([
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false }
{ name: 'goodone.csv', type: 'file', exists: true, blobExists: true, datasetExists: false, hash: '2241de57bbec8144c8ad387e69b3a3ba' },
{ name: 'goodtwo.mp3', type: 'audio', exists: false, blobExists: false, datasetExists: false, hash: null }
]);
});
}));
Expand Down