Skip to content

Resume attachment downloads #8940

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tylerjmchugh
Copy link
Contributor

Currently attachment downloads cannot be resumed. This means that if an attachment download is almost complete and an error is encountered, the entire download needs to be restarted. Depending on the size of the file this may be problematic and time consuming.

This PR aims to fix this issue by adding support for the Range header. This enables the browser to resume the download by specifying which bytes it still needs to download.
image

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

String originalFilename, int size) throws IOException {
// Set headers before processing image
String filename = originalFilename.substring(0, originalFilename.lastIndexOf('.')) + ".png";
response.setHeader(HttpHeaders.CONTENT_DISPOSITION, "inline; filename=\"" + filename + "\"");
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if inline is the best option here, probably it should use attachment?

Copy link
Contributor Author

@tylerjmchugh tylerjmchugh Jul 17, 2025

Choose a reason for hiding this comment

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

Do you also think all of the resource downloads should use attachment? I don't see why only the resized images would use attachment while all other resource downloads use inline.

The use of inline was pre-existent and I figured the reasoning was so that users could view images or other attachments that are viewable in the browser without needing to download it and open it manually in the browser or another viewer. If the browser does not support displaying the file in the browser it seems to revert to downloading it as an attachment so this seems to give the best of both worlds.

If others agree that all attachment downloads should be downloaded directly instead of displaying in the browser when possible I will update to attachment.

Copy link
Member

Choose a reason for hiding this comment

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

it seems fine to me @tylerjmchugh

Co-authored-by: Juan Luis Rodríguez Ponce <[email protected]>
Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

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

Tested to upload a big file and download with curl interrupting the download and continuing, and the file is

curl --limit-rate 1000k -O http://localhost:8080/geonetwork/srv/api/records/78f93047-74f8-4419-ac3d-fc62e4b0477b/attachments/bigfile.zip

CTRL+C

curl  --limit-rate 1000k  -C - -O http://localhost:8080/geonetwork/srv/api/records/78f93047-74f8-4419-ac3d-fc62e4b0477b/attachments/bigfile.zip

The download is resumed and the file is correct also.

Please check the small comment added.

Comment on lines +465 to +471
long skipped = 0;
while (skipped < start) {
long toSkip = start - skipped;
long actuallySkipped = bufferedStream.skip(toSkip);
if (actuallySkipped <= 0) break; // EOF
skipped += actuallySkipped;
}
Copy link
Member

Choose a reason for hiding this comment

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

Check indentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants