Skip to content

Commit

Permalink
Merge pull request #1088 from microsoft/rsh/contentLengthUploadFix
Browse files Browse the repository at this point in the history
Content Length Missing Fix
  • Loading branch information
ramsessanchez authored Feb 29, 2024
2 parents 4fb8d13 + 826ad00 commit d2103b3
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 26 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

## [1.0.5] - 2023-02-28

### Changed

- Added contentLength property to RequestInformation to facilitate in setting the content length of the Okhttp3 RequestBody object within the OkhttpRequestAdapter.

## [1.0.4] - 2024-02-26

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import okio.BufferedSink;
import okio.Okio;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.math.BigDecimal;
Expand All @@ -53,6 +54,7 @@
/** RequestAdapter implementation for OkHttp */
public class OkHttpRequestAdapter implements com.microsoft.kiota.RequestAdapter {
private static final String contentTypeHeaderKey = "Content-Type";
private static final String contentLengthHeaderKey = "Content-Length";
@Nonnull private final Call.Factory client;
@Nonnull private final AuthenticationProvider authProvider;
@Nonnull private final ObservabilityOptions obsOptions;
Expand Down Expand Up @@ -188,7 +190,7 @@ public void enableBackingStore(@Nullable final BackingStoreFactory backingStoreF
Objects.requireNonNull(requestInfo, nullRequestInfoParameter);
Objects.requireNonNull(factory, nullFactoryParameter);

final Span span = startSpan(requestInfo, "sendCollectionAsync");
final Span span = startSpan(requestInfo, "sendCollection");
try (final Scope scope = span.makeCurrent()) {
Response response = this.getHttpResponseMessage(requestInfo, span, span, null);
final ResponseHandler responseHandler = getResponseHandler(requestInfo);
Expand Down Expand Up @@ -269,7 +271,7 @@ private Span startSpan(
Objects.requireNonNull(requestInfo, nullRequestInfoParameter);
Objects.requireNonNull(factory, nullFactoryParameter);

final Span span = startSpan(requestInfo, "sendAsync");
final Span span = startSpan(requestInfo, "send");
try (final Scope scope = span.makeCurrent()) {
Response response = this.getHttpResponseMessage(requestInfo, span, span, null);
final ResponseHandler responseHandler = getResponseHandler(requestInfo);
Expand Down Expand Up @@ -331,7 +333,7 @@ private void closeResponse(boolean closeResponse, Response response) {
@Nonnull final Class<ModelType> targetClass) {
Objects.requireNonNull(requestInfo, nullRequestInfoParameter);
Objects.requireNonNull(targetClass, "parameter targetClass cannot be null");
final Span span = startSpan(requestInfo, "sendPrimitiveAsync");
final Span span = startSpan(requestInfo, "sendPrimitive");
try (final Scope scope = span.makeCurrent()) {
Response response = this.getHttpResponseMessage(requestInfo, span, span, null);
final ResponseHandler responseHandler = getResponseHandler(requestInfo);
Expand Down Expand Up @@ -425,7 +427,7 @@ private void closeResponse(boolean closeResponse, Response response) {
@Nonnull final ValuedEnumParser<ModelType> enumParser) {
Objects.requireNonNull(requestInfo, nullRequestInfoParameter);
Objects.requireNonNull(enumParser, nullEnumParserParameter);
final Span span = startSpan(requestInfo, "sendEnumAsync");
final Span span = startSpan(requestInfo, "sendEnum");
try (final Scope scope = span.makeCurrent()) {
Response response = this.getHttpResponseMessage(requestInfo, span, span, null);
final ResponseHandler responseHandler = getResponseHandler(requestInfo);
Expand Down Expand Up @@ -471,7 +473,7 @@ private void closeResponse(boolean closeResponse, Response response) {
@Nonnull final ValuedEnumParser<ModelType> enumParser) {
Objects.requireNonNull(requestInfo, nullRequestInfoParameter);
Objects.requireNonNull(enumParser, nullEnumParserParameter);
final Span span = startSpan(requestInfo, "sendEnumCollectionAsync");
final Span span = startSpan(requestInfo, "sendEnumCollection");
try (final Scope scope = span.makeCurrent()) {
Response response = this.getHttpResponseMessage(requestInfo, span, span, null);
final ResponseHandler responseHandler = getResponseHandler(requestInfo);
Expand Down Expand Up @@ -518,7 +520,7 @@ private void closeResponse(boolean closeResponse, Response response) {
@Nonnull final Class<ModelType> targetClass) {
Objects.requireNonNull(requestInfo, nullRequestInfoParameter);

final Span span = startSpan(requestInfo, "sendPrimitiveCollectionAsync");
final Span span = startSpan(requestInfo, "sendPrimitiveCollection");
try (final Scope scope = span.makeCurrent()) {
Response response = getHttpResponseMessage(requestInfo, span, span, null);
final ResponseHandler responseHandler = getResponseHandler(requestInfo);
Expand Down Expand Up @@ -835,7 +837,7 @@ private void setBaseUrlForRequestInformation(@Nonnull final RequestInformation r
@SuppressWarnings("unchecked")
@Nonnull public <T> T convertToNativeRequest(@Nonnull final RequestInformation requestInfo) {
Objects.requireNonNull(requestInfo, nullRequestInfoParameter);
final Span span = startSpan(requestInfo, "convertToNativeRequestAsync");
final Span span = startSpan(requestInfo, "convertToNativeRequest");
try (final Scope scope = span.makeCurrent()) {
this.authProvider.authenticateRequest(requestInfo, null);
return (T) getRequestFromRequestInformation(requestInfo, span, span);
Expand Down Expand Up @@ -885,9 +887,8 @@ private void setBaseUrlForRequestInformation(@Nonnull final RequestInformation r
@Override
public MediaType contentType() {
final Set<String> contentTypes =
requestInfo.headers.containsKey(contentTypeHeaderKey)
? requestInfo.headers.get(contentTypeHeaderKey)
: new HashSet<>();
requestInfo.headers.getOrDefault(
contentTypeHeaderKey, new HashSet<>());
if (contentTypes.isEmpty()) {
return null;
} else {
Expand All @@ -899,6 +900,30 @@ public MediaType contentType() {
}
}

@Override
public long contentLength() {
long length;
final Set<String> contentLength =
requestInfo.headers.getOrDefault(
contentLengthHeaderKey, new HashSet<>());
if (contentLength.isEmpty()
&& requestInfo.content
instanceof ByteArrayInputStream) {
final ByteArrayInputStream contentStream =
(ByteArrayInputStream) requestInfo.content;
length = contentStream.available();
} else {
length =
Long.parseLong(
contentLength.toArray(new String[] {})[0]);
}
if (length > 0) {
spanForAttributes.setAttribute(
SemanticAttributes.HTTP_REQUEST_BODY_SIZE, length);
}
return length;
}

@Override
public void writeTo(@Nonnull BufferedSink sink) throws IOException {
sink.writeAll(Okio.source(requestInfo.content));
Expand Down Expand Up @@ -933,17 +958,7 @@ public void writeTo(@Nonnull BufferedSink sink) throws IOException {
requestBuilder.tag(obsOptions.getType(), obsOptions);
}
requestBuilder.tag(Span.class, parentSpan);
final Request request = requestBuilder.build();
final List<String> contentLengthHeader = request.headers().values("Content-Length");
if (contentLengthHeader != null && !contentLengthHeader.isEmpty()) {
final String firstEntryValue = contentLengthHeader.get(0);
if (firstEntryValue != null && !firstEntryValue.isEmpty()) {
spanForAttributes.setAttribute(
SemanticAttributes.HTTP_REQUEST_BODY_SIZE,
Long.parseLong(firstEntryValue));
}
}
return request;
return requestBuilder.build();
} finally {
span.end();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public UserAgentHandlerOption() {}

private boolean enabled = true;
@Nonnull private String productName = "kiota-java";
@Nonnull private String productVersion = "1.0.0";
@Nonnull private String productVersion = "1.0.5";

/**
* Gets the product name to be used in the user agent header
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,15 @@

import okio.Okio;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.stubbing.Answer;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.*;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
Expand Down Expand Up @@ -293,6 +292,60 @@ void throwsAPIException(
assertTrue(exception.getResponseHeaders().containsKey("request-id"));
}

@Test
void getRequestFromRequestInformationHasCorrectContentLength_JsonPayload() throws Exception {
final var authenticationProviderMock = mock(AuthenticationProvider.class);
final var requestInformation = new RequestInformation();
requestInformation.setUri(new URI("https://localhost"));
ByteArrayInputStream content =
new ByteArrayInputStream(
"{\"name\":\"value\",\"array\":[\"1\",\"2\",\"3\"]}"
.getBytes(StandardCharsets.UTF_8));
requestInformation.setStreamContent(content, "application/octet-stream");
requestInformation.httpMethod = HttpMethod.PUT;
requestInformation.headers.tryAdd("Content-Length", String.valueOf(content.available()));

final var adapter = new OkHttpRequestAdapter(authenticationProviderMock);
final var request =
adapter.getRequestFromRequestInformation(
requestInformation, mock(Span.class), mock(Span.class));

assertEquals(
String.valueOf(requestInformation.content.available()),
request.headers().get("Content-Length"));
assertEquals("application/octet-stream", request.headers().get("Content-Type"));
assertNotNull(request.body());
assertEquals(request.body().contentLength(), requestInformation.content.available());
assertEquals(request.body().contentType(), MediaType.parse("application/octet-stream"));
}

@Test
void getRequestFromRequestInformationIncludesContentLength_FilePayload() throws Exception {
final var authenticationProviderMock = mock(AuthenticationProvider.class);
final var testFile = new File("./src/test/resources/helloWorld.txt");
final var requestInformation = new RequestInformation();

requestInformation.setUri(new URI("https://localhost"));
requestInformation.httpMethod = HttpMethod.PUT;
requestInformation.headers.add("Content-Length", String.valueOf(testFile.length()));
try (FileInputStream content = new FileInputStream(testFile)) {
requestInformation.setStreamContent(content, "application/octet-stream");

final var adapter = new OkHttpRequestAdapter(authenticationProviderMock);
final var request =
adapter.getRequestFromRequestInformation(
requestInformation, mock(Span.class), mock(Span.class));

assertEquals(
String.valueOf(requestInformation.content.available()),
request.headers().get("Content-Length"));
assertEquals("application/octet-stream", request.headers().get("Content-Type"));
assertNotNull(request.body());
assertEquals(request.body().contentLength(), requestInformation.content.available());
assertEquals(request.body().contentType(), MediaType.parse("application/octet-stream"));
}
}

public static OkHttpClient getMockClient(final Response response) throws IOException {
final OkHttpClient mockClient = mock(OkHttpClient.class);
final Call remoteCall = mock(Call.class);
Expand Down
25 changes: 25 additions & 0 deletions components/http/okHttp/src/test/resources/helloWorld.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
HelloWorld!
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ org.gradle.caching=true
mavenGroupId = com.microsoft.kiota
mavenMajorVersion = 1
mavenMinorVersion = 0
mavenPatchVersion = 4
mavenPatchVersion = 5
mavenArtifactSuffix =

#These values are used to run functional tests
Expand Down

0 comments on commit d2103b3

Please sign in to comment.