diff --git a/.github/workflows/api-level-lint.yml b/.github/workflows/api-level-lint.yml index ded918500..e28d53cbb 100644 --- a/.github/workflows/api-level-lint.yml +++ b/.github/workflows/api-level-lint.yml @@ -17,7 +17,7 @@ jobs: distribution: 'temurin' java-version: 20 - name: Setup Android SDK - uses: android-actions/setup-android@v3.2.0 + uses: android-actions/setup-android@v3.2.1 - name: Add execution right to the script run: chmod +x gradlew working-directory: ./android diff --git a/.github/workflows/build-and-publish.yml b/.github/workflows/build-and-publish.yml index 60228a467..5e54a94d4 100644 --- a/.github/workflows/build-and-publish.yml +++ b/.github/workflows/build-and-publish.yml @@ -111,7 +111,7 @@ jobs: with: tag: ${{ steps.GetVersion.outputs.tag }} - name: Queue Git Release - uses: benc-uk/workflow-dispatch@v121 + uses: benc-uk/workflow-dispatch@v1 with: workflow: Git Release token: ${{ secrets.PERSONAL_TOKEN }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f04f73ec..6e39392e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +## [3.1.8] + +### Added + +### Changed +- Changed chunkInputStream method in LargeFileUploadTask to resolve IndexOutOfBoundsException when uploading large files +- Fix Large File Upload bug where exception was thrown for completed successful uploads + ## [3.1.7] - 2024-03-28 ### Changed @@ -60,13 +68,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Version bump for Java SDK GA release. -- Bumps Kiota-Java abstractions, authentication, http, and serialization components for Java SDK 6.1.0 release. +- Bumps Kiota-Java abstractions, authentication, http, and serialization components for Java SDK 6.1.0 release. ## [3.0.12] - 2023-12-15 -### Fixed +### Fixed -- Fixes a bug where a null collection for allowedHosts would result in failure to initialize client. [#1411](https://github.com/microsoftgraph/msgraph-sdk-java-core/pull/1411) +- Fixes a bug where a null collection for allowedHosts would result in failure to initialize client. [#1411](https://github.com/microsoftgraph/msgraph-sdk-java-core/pull/1411) ## [3.0.11] - 2023-12-08 @@ -77,7 +85,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.0.10] - 2023-11-27 -### Changed +### Changed - Removed the usage of reflection for enum deserialization and reordered RequestAdapter method arguments. [Kiota-Java #840](https://github.com/microsoft/kiota-java/issues/840) @@ -100,13 +108,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed -- Removes 'SuppressFBWarnings' annotations and dependency. +- Removes 'SuppressFBWarnings' annotations and dependency. ## [3.0.7] - 2023-07-20 ### Added -- Adds graph-java-sdk implementation of the `UrlReplaceHandler` middleware including default replacement pairs. +- Adds graph-java-sdk implementation of the `UrlReplaceHandler` middleware including default replacement pairs. - Default replacement pair: '/users/TokenToReplace' -> '/me' ## [3.0.6] - 2023-07-11 @@ -123,11 +131,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.0.4] - 2023-05-03 -### Added - -- Added LargeFileUploadTask functionality for kiota generated service libraries. +### Added + +- Added LargeFileUploadTask functionality for kiota generated service libraries. -### Fixed +### Fixed - Fixes formatting used in the headers added by the telemetry handler to align with the [msGraph sdk spec.](https://github.com/microsoftgraph/msgraph-sdk-design/blob/master/middleware/TelemetryHandler.md) @@ -171,7 +179,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 private OkHttpClient createClient(@Nonnull final IAuthenticationProvider auth) { return HttpClients.createDefault(auth); } - + // then create the GraphServiceClient IAuthenticationProvider authenticationProvider = ...; GraphServiceClient @@ -183,16 +191,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [2.0.20] - 2023-10-23 -### Changed +### Changed -- Updates Okhttp3 to avoid transient vulnerabilty. [#1038](https://github.com/microsoftgraph/msgraph-sdk-java-core/issues/1038) +- Updates Okhttp3 to avoid transient vulnerabilty. [#1038](https://github.com/microsoftgraph/msgraph-sdk-java-core/issues/1038) ## [2.0.19] - 2023-06-20 ### Changed - Remove explicit logging of GraphServiceException in the CoreHttpProvider class. [#885](https://github.com/microsoftgraph/msgraph-sdk-java-core/issues/885) -- Thank you to @MaHa6543 for the contribution. +- Thank you to @MaHa6543 for the contribution. ## [2.0.18] - 2023-04-06 diff --git a/android/build.gradle b/android/build.gradle index d1b1d2133..28963e60a 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -5,8 +5,8 @@ buildscript { } dependencies { - classpath "com.gradle:gradle-enterprise-gradle-plugin:3.16.2" - classpath "com.android.tools.build:gradle:8.3.1" + classpath "com.gradle:gradle-enterprise-gradle-plugin:3.17.2" + classpath "com.android.tools.build:gradle:8.3.2" classpath "com.github.ben-manes:gradle-versions-plugin:0.51.0" } } diff --git a/build.gradle b/build.gradle index f9aef0af8..d0ed470cf 100644 --- a/build.gradle +++ b/build.gradle @@ -5,7 +5,7 @@ plugins { id 'maven-publish' id 'signing' id 'jacoco' - id 'com.github.spotbugs' version '6.0.9' + id 'com.github.spotbugs' version '6.0.12' id "org.sonarqube" version "5.0.0.4638" } diff --git a/gradle.properties b/gradle.properties index cc5f7bce9..e136b055f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -25,7 +25,7 @@ mavenGroupId = com.microsoft.graph mavenArtifactId = microsoft-graph-core mavenMajorVersion = 3 mavenMinorVersion = 1 -mavenPatchVersion = 7 +mavenPatchVersion = 8 mavenArtifactSuffix = #These values are used to run functional tests diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 2bd427eb2..c172fa73b 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -4,22 +4,21 @@ dependencies { testImplementation 'org.junit.jupiter:junit-jupiter-params:5.10.2' testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.10.2' testImplementation 'org.mockito:mockito-inline:5.2.0' - testImplementation 'io.opentelemetry:opentelemetry-api:1.36.0' - testImplementation 'io.opentelemetry:opentelemetry-context:1.36.0' - testImplementation 'io.github.std-uritemplate:std-uritemplate:0.0.55' - + testImplementation 'io.opentelemetry:opentelemetry-api:1.37.0' + testImplementation 'io.opentelemetry:opentelemetry-context:1.37.0' + testImplementation 'io.github.std-uritemplate:std-uritemplate:0.0.57' implementation 'com.google.code.gson:gson:2.10.1' - implementation 'jakarta.annotation:jakarta.annotation-api:2.1.1' + implementation 'jakarta.annotation:jakarta.annotation-api:3.0.0' api 'com.squareup.okhttp3:okhttp:4.12.0' - api 'com.azure:azure-core:1.47.0' + api 'com.azure:azure-core:1.48.0' - api 'com.microsoft.kiota:microsoft-kiota-abstractions:1.1.2' - implementation 'com.microsoft.kiota:microsoft-kiota-authentication-azure:1.1.2' - implementation 'com.microsoft.kiota:microsoft-kiota-http-okHttp:1.1.2' - implementation 'com.microsoft.kiota:microsoft-kiota-serialization-json:1.1.2' - implementation 'com.microsoft.kiota:microsoft-kiota-serialization-text:1.1.2' - implementation 'com.microsoft.kiota:microsoft-kiota-serialization-form:1.1.2' - implementation 'com.microsoft.kiota:microsoft-kiota-serialization-multipart:1.1.2' + api 'com.microsoft.kiota:microsoft-kiota-abstractions:1.1.6' + implementation 'com.microsoft.kiota:microsoft-kiota-authentication-azure:1.1.6' + implementation 'com.microsoft.kiota:microsoft-kiota-http-okHttp:1.1.6' + implementation 'com.microsoft.kiota:microsoft-kiota-serialization-json:1.1.6' + implementation 'com.microsoft.kiota:microsoft-kiota-serialization-text:1.1.6' + implementation 'com.microsoft.kiota:microsoft-kiota-serialization-form:1.1.6' + implementation 'com.microsoft.kiota:microsoft-kiota-serialization-multipart:1.1.6' } diff --git a/pom.xml b/pom.xml index c8a8da7dd..d82395394 100644 --- a/pom.xml +++ b/pom.xml @@ -30,7 +30,7 @@ com.azure azure-core - 1.47.0 + 1.48.0 org.junit.jupiter @@ -53,7 +53,7 @@ com.github.spotbugs spotbugs-annotations - 4.8.3 + 4.8.4 diff --git a/src/main/java/com/microsoft/graph/core/CoreConstants.java b/src/main/java/com/microsoft/graph/core/CoreConstants.java index 20549b19b..ce10f0ed6 100644 --- a/src/main/java/com/microsoft/graph/core/CoreConstants.java +++ b/src/main/java/com/microsoft/graph/core/CoreConstants.java @@ -14,7 +14,7 @@ private CoreConstants() {} private static class VersionValues { private static final int MAJOR = 3; private static final int MINOR = 1; - private static final int PATCH = 7; + private static final int PATCH = 8; } /** diff --git a/src/main/java/com/microsoft/graph/core/requests/upload/UploadResponseHandler.java b/src/main/java/com/microsoft/graph/core/requests/upload/UploadResponseHandler.java index 8e165599a..7a7b6ff5f 100644 --- a/src/main/java/com/microsoft/graph/core/requests/upload/UploadResponseHandler.java +++ b/src/main/java/com/microsoft/graph/core/requests/upload/UploadResponseHandler.java @@ -52,11 +52,25 @@ public UploadResult handleResponse(@Nonnull final Respon Objects.requireNonNull(response); Objects.requireNonNull(factory); try (final ResponseBody body = response.body()) { - if (Objects.isNull(body)) { + UploadResult uploadResult = new UploadResult<>(); + String contentLengthHeader = response.headers().get("content-length"); + // rely on content-type OR content-length headers to determine if response body is empty. + // Response body() may be non-null despite being empty in raw response https://square.github.io/okhttp/3.x/okhttp/okhttp3/Response.html#body-- + // content-length header is not always present in Graph responses. Content-type is more reliable + if (Objects.isNull(body) + || Objects.isNull(body.contentType()) + || (!Objects.isNull(contentLengthHeader) && Integer.parseInt(contentLengthHeader) == 0) + ) { + if (response.code() == HttpURLConnection.HTTP_CREATED) { + final String location = response.headers().get("location"); + if(!Objects.isNull(location) && !location.isEmpty()) { + uploadResult.location = new URI(location); + return uploadResult; + } + } throw new ApiException(ErrorConstants.Messages.NO_RESPONSE_FOR_UPLOAD); } try(final InputStream in = body.byteStream()){ - final String contentType = body.contentType().toString().split(";")[0]; //contentType.toString() returns in format ;, we only want the mediaType. if(!response.isSuccessful()) { throw new ApiExceptionBuilder() .withMessage(ErrorConstants.Codes.GENERAL_EXCEPTION) @@ -64,24 +78,20 @@ public UploadResult handleResponse(@Nonnull final Respon .withResponseHeaders(HeadersCompatibility.getResponseHeaders(response.headers())) .build(); } - UploadResult uploadResult = new UploadResult<>(); - if (response.code() == HttpURLConnection.HTTP_CREATED) { - if (body.contentLength() > 0) { - final ParseNode uploadTypeParseNode = parseNodeFactory.getParseNode(contentType, in); - uploadResult.itemResponse = uploadTypeParseNode.getObjectValue(factory); - } - final String location = response.headers().get("location"); - if(!Objects.isNull(location) && !location.isEmpty()) { - uploadResult.location = new URI(location); - } - } else { + boolean canBeParsed = (!Objects.isNull(contentLengthHeader) && Integer.parseInt(contentLengthHeader) > 0) || !Objects.isNull(body.contentType()); + String contentType = canBeParsed ? body.contentType().toString().split(";")[0] : null; //contentType.toString() returns in format ;, we only want the mediaType. + if (canBeParsed) { final ParseNode parseNode = parseNodeFactory.getParseNode(contentType, in); - final UploadSession uploadSession = parseNode.getObjectValue(UploadSession::createFromDiscriminatorValue); - final List nextExpectedRanges = uploadSession.getNextExpectedRanges(); - if (!(nextExpectedRanges == null || nextExpectedRanges.isEmpty())) { - uploadResult.uploadSession = uploadSession; - } else { + if (response.code() == HttpURLConnection.HTTP_CREATED) { uploadResult.itemResponse = parseNode.getObjectValue(factory); + } else { + final UploadSession uploadSession = parseNode.getObjectValue(UploadSession::createFromDiscriminatorValue); + final List nextExpectedRanges = uploadSession.getNextExpectedRanges(); + if (!(nextExpectedRanges == null || nextExpectedRanges.isEmpty())) { + uploadResult.uploadSession = uploadSession; + } else { + uploadResult.itemResponse = parseNode.getObjectValue(factory); + } } } return uploadResult; @@ -91,6 +101,7 @@ public UploadResult handleResponse(@Nonnull final Respon throw new RuntimeException(ex); } } + } diff --git a/src/main/java/com/microsoft/graph/core/tasks/LargeFileUploadTask.java b/src/main/java/com/microsoft/graph/core/tasks/LargeFileUploadTask.java index 9f02dfe27..566506d07 100644 --- a/src/main/java/com/microsoft/graph/core/tasks/LargeFileUploadTask.java +++ b/src/main/java/com/microsoft/graph/core/tasks/LargeFileUploadTask.java @@ -201,7 +201,7 @@ public IUploadSession updateSessionStatus() { return session; } private UploadResult uploadSlice(UploadSliceRequestBuilder uploadSliceRequestBuilder, ArrayList exceptionsList) throws IOException { - byte[] buffer = chunkInputStream(uploadStream,(int) uploadSliceRequestBuilder.getRangeBegin(), (int)uploadSliceRequestBuilder.getRangeLength()); + byte[] buffer = chunkInputStream(uploadStream, (int)uploadSliceRequestBuilder.getRangeLength()); ByteArrayInputStream chunkStream = new ByteArrayInputStream(buffer); try { return uploadSliceRequestBuilder.put(chunkStream); @@ -275,9 +275,9 @@ private long nextSliceSize(long rangeBegin, long rangeEnd) { long size = rangeEnd - rangeBegin + 1; return Math.min(size, this.maxSliceSize); } - private byte[] chunkInputStream(InputStream stream, int begin, int length) throws IOException { + private byte[] chunkInputStream(InputStream stream, int length) throws IOException { byte[] buffer = new byte[length]; - int lengthAssert = stream.read(buffer, begin, length); + int lengthAssert = stream.read(buffer); assert lengthAssert == length; return buffer; } diff --git a/src/test/java/com/microsoft/graph/core/requests/upload/UploadResponseHandlerTest.java b/src/test/java/com/microsoft/graph/core/requests/upload/UploadResponseHandlerTest.java index f0f4105f7..b15166d2e 100644 --- a/src/test/java/com/microsoft/graph/core/requests/upload/UploadResponseHandlerTest.java +++ b/src/test/java/com/microsoft/graph/core/requests/upload/UploadResponseHandlerTest.java @@ -11,11 +11,16 @@ import okhttp3.*; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.stubbing.Answer; import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static com.microsoft.kiota.serialization.ParseNodeFactoryRegistry.defaultInstance; +import java.io.IOException; import java.net.HttpURLConnection; import java.time.OffsetDateTime; @@ -24,7 +29,7 @@ class UploadResponseHandlerTest { ParseNodeFactoryRegistry registry = defaultInstance; @Test - void GetUploadItemOnCompletedUpload() { + void GetUploadItemOnCompletedUpload() throws IOException { registry.contentTypeAssociatedFactories.put(CoreConstants.MimeTypeNames.APPLICATION_JSON, new JsonParseNodeFactory()); UploadResponseHandler responseHandler = new UploadResponseHandler(null); @@ -41,8 +46,9 @@ void GetUploadItemOnCompletedUpload() { .body(body) .code(HttpURLConnection.HTTP_CREATED) .build(); + OkHttpClient mockHttpClient = getMockClient(response); UploadResult result = responseHandler - .handleResponse(response, TestDriveItem::createFromDiscriminatorValue); + .handleResponse(mockHttpClient.newCall(mock(Request.class)).execute(), TestDriveItem::createFromDiscriminatorValue); responseHandler.handleResponse(response, parseNode -> {return new TestDriveItem();}); TestDriveItem item = result.itemResponse; assertTrue(result.isUploadSuccessful()); @@ -53,7 +59,7 @@ void GetUploadItemOnCompletedUpload() { } @Test - void GetUploadItemOnCompletedUpdate() { + void GetUploadItemOnCompletedUpdate() throws IOException { registry.contentTypeAssociatedFactories.put(CoreConstants.MimeTypeNames.APPLICATION_JSON, new JsonParseNodeFactory()); UploadResponseHandler responseHandler = new UploadResponseHandler(null); @@ -70,8 +76,9 @@ void GetUploadItemOnCompletedUpdate() { .code(HttpURLConnection.HTTP_OK) .message("OK") .build(); + OkHttpClient mockHttpClient = getMockClient(response); UploadResult result = responseHandler - .handleResponse(response, TestDriveItem::createFromDiscriminatorValue); + .handleResponse(mockHttpClient.newCall(mock(Request.class)).execute(), TestDriveItem::createFromDiscriminatorValue); responseHandler.handleResponse(response, parseNode -> {return new TestDriveItem();}); TestDriveItem item = result.itemResponse; assertTrue(result.isUploadSuccessful()); @@ -82,7 +89,7 @@ void GetUploadItemOnCompletedUpdate() { } @Test - void getFileAttachmentLocationOnCompletedUpload() { + void getFileAttachmentLocationOnCompletedUpload() throws IOException { registry.contentTypeAssociatedFactories.put(CoreConstants.MimeTypeNames.APPLICATION_JSON, new JsonParseNodeFactory()); UploadResponseHandler responseHandler = new UploadResponseHandler(null); @@ -90,12 +97,13 @@ void getFileAttachmentLocationOnCompletedUpload() { .request(mock(Request.class)) .protocol(mock(Protocol.class)) .message("success") - .body(ResponseBody.create("", MediaType.parse(CoreConstants.MimeTypeNames.APPLICATION_JSON))) .code(HttpURLConnection.HTTP_CREATED) .header("location", "http://localhost") .build(); + OkHttpClient mockClient = getMockClient(response); UploadResult result = responseHandler - .handleResponse(response,TestDriveItem::createFromDiscriminatorValue); + .handleResponse(mockClient.newCall(mock(Request.class)).execute() + ,TestDriveItem::createFromDiscriminatorValue); TestDriveItem item = result.itemResponse; assertTrue(result.isUploadSuccessful()); @@ -103,7 +111,7 @@ void getFileAttachmentLocationOnCompletedUpload() { assertEquals("http://localhost", result.location.toString()); } @Test - void getUploadSessionOnProgressingUpload() { + void getUploadSessionOnProgressingUpload() throws IOException { registry.contentTypeAssociatedFactories.put(CoreConstants.MimeTypeNames.APPLICATION_JSON, new JsonParseNodeFactory()); UploadResponseHandler responseHandler = new UploadResponseHandler(null); @@ -123,8 +131,9 @@ void getUploadSessionOnProgressingUpload() { .body(body) .code(HttpURLConnection.HTTP_ACCEPTED) .build(); + OkHttpClient mockHttpClient = getMockClient(response); UploadResult result = responseHandler - .handleResponse(response, TestDriveItem::createFromDiscriminatorValue); + .handleResponse(mockHttpClient.newCall(mock(Request.class)).execute(), TestDriveItem::createFromDiscriminatorValue); UploadSession session = (UploadSession) result.uploadSession; assertFalse(result.isUploadSuccessful()); @@ -137,7 +146,7 @@ void getUploadSessionOnProgressingUpload() { } @Test - void throwsServiceExceptionOnErrorResponse() { + void throwsServiceExceptionOnErrorResponse() throws IOException { UploadResponseHandler responseHandler = new UploadResponseHandler(null); ResponseBody body = ResponseBody.create("{\n" + " \"error\": {\n"+ @@ -157,17 +166,18 @@ void throwsServiceExceptionOnErrorResponse() { .body(body) .code(HttpURLConnection.HTTP_UNAUTHORIZED) .build(); + OkHttpClient mockHttpClient = getMockClient(response); try { responseHandler - .handleResponse(response, TestDriveItem::createFromDiscriminatorValue); + .handleResponse(mockHttpClient.newCall(mock(Request.class)).execute(), TestDriveItem::createFromDiscriminatorValue); } catch (ApiException ex) { Assertions.assertEquals(ErrorConstants.Codes.GENERAL_EXCEPTION, ex.getMessage()); assertEquals(HttpURLConnection.HTTP_UNAUTHORIZED, ex.getResponseStatusCode()); } } @Test - void throwsSerializationErrorOnInvalidJson() { + void throwsSerializationErrorOnInvalidJson() throws IOException { UploadResponseHandler responseHandler = new UploadResponseHandler(null); String malformedResponse = " \"error\": {\n"+ @@ -188,11 +198,31 @@ void throwsSerializationErrorOnInvalidJson() { .body(body) .code(HttpURLConnection.HTTP_UNAUTHORIZED) .build(); + OkHttpClient mockHttpClient = getMockClient(response); try { responseHandler - .handleResponse(response, TestDriveItem::createFromDiscriminatorValue); + .handleResponse(mockHttpClient.newCall(mock(Request.class)).execute(), TestDriveItem::createFromDiscriminatorValue); } catch (ApiException ex) { assertEquals(ErrorConstants.Codes.GENERAL_EXCEPTION, ex.getMessage()); } } + + public static OkHttpClient getMockClient(final Response response) throws IOException { + final OkHttpClient mockClient = mock(OkHttpClient.class); + final Call remoteCall = mock(Call.class); + final Dispatcher dispatcher = new Dispatcher(); + when(remoteCall.execute()).thenReturn(response); + doAnswer( + (Answer) + invocation -> { + Callback callback = invocation.getArgument(0); + callback.onResponse(null, response); + return null; + }) + .when(remoteCall) + .enqueue(any(Callback.class)); + when(mockClient.dispatcher()).thenReturn(dispatcher); + when(mockClient.newCall(any())).thenReturn(remoteCall); + return mockClient; + } }