kiota-java icon indicating copy to clipboard operation
kiota-java copied to clipboard

Update request body write to retain inputstream buffer

Open Ndiritu opened this issue 1 year ago • 2 comments

We currently override the Request body's writeTo method which was clearing the buffer by doing a writeAll.

When a logging interceptor is used e.g. HttpLoggingInterceptor the request body when the network call is being made is empty.

closes https://github.com/microsoftgraph/msgraph-sdk-java/issues/2037

In progress:

  • Unit tests

Ndiritu avatar Aug 19 '24 15:08 Ndiritu

for others who might be following along:

  • the logging interceptor is draining the stream
  • when the transport picks it up, it's already drained
  • the main reason it's most likely not doing a buffered copy is because they instead leverage the "isOneShot" method on the body to avoid draining the request body if it shouldn't.
  • Our implementation needs to implement that method in the request adapter, based on whether the underlying stream is markable or not
  • additionally, when the underlying stream is markable, in the writeTo method, we need to reset/mark it to zero
  • This option is much better in terms of performance than buffering everything for this edge case.
  • It's also much better in terms of UX than giving a setting on the request adapter to buffer ONLY when requested.

baywet avatar Aug 21 '24 19:08 baywet

Thank you for making the changes! A version bump, a changelog entry and we should be good to go!

Ah no release please yet :) On it.

Ndiritu avatar Aug 22 '24 12:08 Ndiritu

@Ndiritu the tag also needs to be manually created on this repo for the time being. I'll let you take care of that :)

baywet avatar Aug 22 '24 12:08 baywet

@Ndiritu @baywet Thanks a lot guys! ❤️

mkomko avatar Aug 22 '24 12:08 mkomko