search

dzikoysk / reposilite

Lightweight and easy-to-use repository management software dedicated for the Maven based artifacts in the JVM ecosystem 📦
https://reposilite.com
Apache License 2.0
1.36k stars 179 forks source link

GH-2113 Fix issue with sha hash generation #2114

Closed laszlof closed 5 months ago

laszlof commented 5 months ago

Fixes #2113

dzikoysk commented 5 months ago

Thanks for taking a look at the implementation of DigestUtils!

If we have to take care of those streams, I think it'd be fine to open the stream per each put file operation as you did before, but we should just wrap it within Kotlin's .use {} function that you've used before for copying the data to temp file. Thanks to that, we'll open just one stream at the time, and we will guarantee that no matter what, it'll always be closed - because currently, if any of those [...].inputStream() would fail, you can still leave a few open streams.

We have to be extremly careful with these open streams on files, because there's actually a hard limit of open files at OS level 😅

laszlof commented 5 months ago

Thanks for taking a look at the implementation of DigestUtils!

If we have to take care of those streams, I think it'd be fine to open the stream per each put file operation as you did before, but we should just wrap it within Kotlin's .use {} function that you've used before for copying the data to temp file. Thanks to that, we'll open just one stream at the time, and we will guarantee that no matter what, it'll always be closed - because currently, if any of those [...].inputStream() would fail, you can still leave a few open streams.

We have to be extremly careful with these open streams on files, because there's actually a hard limit of open files at OS level 😅

Does anything depend on the return of the writeFileChecksums function?

dzikoysk commented 5 months ago

If writeFileChecksums succeeds, it returns Unit (pretty much similar use-case to void in Java), so not really. It only carries the error if anything wrong happens.

laszlof commented 5 months ago

@dzikoysk PR updated. Let me know how it looks.

dzikoysk commented 5 months ago

Thanks! I'll be releasing the next version, so I decided to push some extra changes to speed up the process :) During CR I've come up with an idea to replace these manual checksum writes with an enum + added a test to actually check if the stored content matches the output.

laszlof commented 5 months ago

This approach seems a lot better/cleaner to me. Removes a lot of the duplicated code that was introduced.

dzikoysk commented 5 months ago

Thanks for help! The new release should be available within 1-2 hours :)