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.

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

Conversation

ktuite
Copy link
Member

@ktuite ktuite commented Nov 12, 2024

Closes getodk/central#758

Exposes the property hash on form attachments, which is set to:

  • null if the attachment is a dataset or if the attachment blob is empty
  • the md5 hash value of the attachment blob file

Unlike the OpenRosa hash for the form manifest, this value is set to null for dataset attachments and doesn't change when a dataset has more entities added/updated.

What has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Does this change require updates to the API documentation? If so, please update docs/api.yaml as part of this PR.

yes, need to add to docs.

Before submitting this PR, please make sure you have:

  • run make test and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code from external sources are properly credited in comments or that everything is internally sourced

@ktuite ktuite mentioned this pull request Oct 15, 2024
3 tasks
lib/model/query/form-attachments.js Outdated Show resolved Hide resolved
lib/model/query/form-attachments.js Outdated Show resolved Hide resolved
lib/model/query/form-attachments.js Outdated Show resolved Hide resolved
lib/model/query/form-attachments.js Outdated Show resolved Hide resolved
lib/model/query/form-attachments.js Outdated Show resolved Hide resolved
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.

@ktuite ktuite merged commit 0c3ec3d into master Nov 14, 2024
7 checks passed
@ktuite ktuite deleted the ktuite/form_attach_hash branch November 14, 2024 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return MD5 hash for form attachments
2 participants