msgraph-sdk-java-core
msgraph-sdk-java-core copied to clipboard
** fix ** fix for LargFileUploadTask constructor
LargeFileUploadTask's constructor does not check anymore whether the available() method of InputStream returns a positive number; as stated by the javadoc this method just makes an estimate of the available bytes and can return 0 even if the stream contains bytes to read.
Fixes #1621
Changes proposed in this pull request
- In
LargeFileUploadTaskthe check on theInputStream's bytes availability is removed. - Updated patch version
- Updated CHANGELOG
@baywet not a great git user, but I think I managed to obtain what you requested, although by reverting and recommitting. :$
Thanks for making the update. @ramsessanchez Do you remember why this check might have been added back then? https://github.com/microsoftgraph/msgraph-sdk-java-core/pull/781
@baywet not a great git user, but I think I managed to obtain what you requested, although by reverting and recommitting. :$
no worries, I have set the auto-merge to squash, which will result in a single commit without the undesired changes
I am actually making a mess... I updated another file, for which I wanted to create a different PR, but it seems that everything i commit to my fork's dev branch ends up in this PR. I will stop altering my branch. I promise.
Thanks for making the update. @ramsessanchez Do you remember why this check might have been added back then? #781
Documentation for available is meant to return the number of bytes that can be read before blocking, however, upon a second check it appears to always return 0 in the case of InputStream , but returns a positive value in the case of BufferedInputStream, or when bytes of the stream have been stored in memory. In our case we use InputStream so we should certainly remove this check and find a better way to ensure that InputStream is not empty. Approving the change as well.
https://stackoverflow.com/questions/3695372/what-does-inputstream-available-do-in-java
@baywet , removed auto merge, please give final approval.
Just wanted to be sure you noticed you also approved and merged the change on settings.json to forbid autoformatting, in this project, a source file modified by somebody (like me) who has autoformatting enabled at global level
Thanks for the callout #1627
@baywet you're welcome. Just out of curiosity, but... why reverting it? Do not misunderstand me, please. I am not offended by the revert, but I can't help but asking: removing it, don't you risk that if somebody deletes one line, you get tons of modifications to the file because of a possible autoformatting set by the user?
I added it because I use google code style at a workspace level so when I saved LargeFileUploadTask.java, all the file was reformatted generating several "different" lines between the original and the new one (may more than the three I deleted).
Please, do not misunderstand me. I am just wondering whether there are better ways to get the modified file not corrupted by user autoformatting rules.
Just asking for learning. Thanks for the patience.
For context we have continuous integration validation that checks for formatting so if any user is not set up properly we will know about that during the pull request validation. Or at least that was my assumption until you called that out. It seems this repository is not set up for linting validation just yet but here is an example PR for another repository and if you want to set it up for this repository in a full request will be more than happy to review and merge this. https://github.com/microsoft/kiota-java/pull/823
Thanks for explaining. About integrating spotless in this project as well, why not during spare time.