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

Fix jcloud content length is inaccurate #8619

Merged

Conversation

tylerjmchugh
Copy link
Contributor

Currently the content length for uploading resources to jcloud is set using the InputStream.available function which is unreliable. This results in uploads ending pre-maturely.

This PR aims to fix this issue by passing the content length to the input stream so the jcloud store can use that value if it is known.

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

@ianwallen ianwallen requested a review from josegar74 January 25, 2025 20:26
@ianwallen ianwallen added this to the 4.4.7 milestone Jan 25, 2025
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.

I haven't test it with JCloud. but the code changes look fine.

@tylerjmchugh just to confirm before you merge it, this change only affects the JCloud store, correct?

@ianwallen
Copy link
Contributor

@josegar74
For all storages, they all receive LimitedInputStream which was done in PR #8562.

In this PR, there is simply a new fileSize field added to LimitedInputStream object which will not affect other storage. Jcloud is the only storage where the api's require the stream and the size to be supplied. All others seems to simply read the stream until no more data exists.

So I don't see how this change can possibly affect other storage.

@tylerjmchugh
Copy link
Contributor Author

@josegar74 Yes it should not affect any other stores. All the stores should have access to the new field now but only jcloud store needs it / takes advantage of it.

@josegar74
Copy link
Member

@tylerjmchugh thanks for the clarification. It looks good to merge it.

@ianwallen ianwallen merged commit 4dc61ea into geonetwork:main Jan 28, 2025
8 checks passed
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.

3 participants