From c78b39d44e7e43054322ea124a35d380bcda6c79 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 10 Sep 2024 11:47:31 +0300 Subject: [PATCH 1/9] fix: resolved handling of larger batch request message --- .../Exceptions/ErrorConstants.cs | 2 + .../Extensions/IParseNodeExtensions.cs | 16 +++ .../Requests/BatchRequestBuilder.cs | 27 +++- .../Requests/Content/BatchResponseContent.cs | 4 +- .../Tasks/PageIterator.cs | 15 +-- .../Requests/BatchRequestBuilderTests.cs | 119 ++++++++++++++++++ 6 files changed, 169 insertions(+), 14 deletions(-) create mode 100644 src/Microsoft.Graph.Core/Extensions/IParseNodeExtensions.cs diff --git a/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs b/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs index 86e6af8d..80617e13 100644 --- a/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs +++ b/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs @@ -83,6 +83,8 @@ internal static class Messages internal static string PageIteratorRequestError = "Error occured when making a request with the page iterator. See inner exception for more details."; + internal static string BatchRequestError = "Error occured when making the batch request. See inner exception for more details."; + public static string InvalidProxyArgument = "Proxy cannot be set more once. Proxy can only be set on the proxy or defaultHttpHandler argument and not both."; } } diff --git a/src/Microsoft.Graph.Core/Extensions/IParseNodeExtensions.cs b/src/Microsoft.Graph.Core/Extensions/IParseNodeExtensions.cs new file mode 100644 index 00000000..cc7615dc --- /dev/null +++ b/src/Microsoft.Graph.Core/Extensions/IParseNodeExtensions.cs @@ -0,0 +1,16 @@ +using Microsoft.Kiota.Abstractions.Serialization; + +namespace Microsoft.Graph; + +/// +/// Extension helpers for the +/// +public static class ParseNodeExtensions +{ + internal static string GetErrorMessage(this IParseNode responseParseNode) + { + var errorParseNode = responseParseNode.GetChildNode("error"); + // concatenate the error code and message + return $"{errorParseNode?.GetChildNode("code")?.GetStringValue()} : {errorParseNode?.GetChildNode("message")?.GetStringValue()}"; + } +} diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index ae342fb5..084278b2 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -7,10 +7,12 @@ namespace Microsoft.Graph.Core.Requests using System; using System.Collections.Generic; using System.Net.Http; + using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.Kiota.Abstractions; using Microsoft.Kiota.Abstractions.Serialization; + using Microsoft.Kiota.Serialization.Json; /// /// The type BatchRequestBuilder @@ -58,7 +60,9 @@ public async Task PostAsync(BatchRequestContent batchReque var nativeResponseHandler = new NativeResponseHandler(); requestInfo.SetResponseHandler(nativeResponseHandler); await this.RequestAdapter.SendNoContentAsync(requestInfo, cancellationToken: cancellationToken); - return new BatchResponseContent(nativeResponseHandler.Value as HttpResponseMessage, errorMappings); + var httpResponseMessage = nativeResponseHandler.Value as HttpResponseMessage; + await ThrowIfFailedResponseAsync(httpResponseMessage); + return new BatchResponseContent(httpResponseMessage, errorMappings); } /// @@ -99,5 +103,26 @@ public async Task ToPostRequestInformationAsync(BatchRequest requestInfo.Headers.Add("Content-Type", CoreConstants.MimeTypeNames.Application.Json); return requestInfo; } + + private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpResponseMessage) + { + if (httpResponseMessage.IsSuccessStatusCode) return; + + if (CoreConstants.MimeTypeNames.Application.Json.Equals(httpResponseMessage.Content?.Headers?.ContentType?.MediaType, StringComparison.OrdinalIgnoreCase)) + { + using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false); + using var document = await JsonDocument.ParseAsync(responseContent).ConfigureAwait(false); + var parsable = new JsonParseNode(document.RootElement); + throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, new Exception(parsable.GetErrorMessage())); + } + + var responseStringContent = string.Empty; + if (httpResponseMessage.Content != null) + { + responseStringContent = await httpResponseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); + } + + throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, responseStringContent); + } } } diff --git a/src/Microsoft.Graph.Core/Requests/Content/BatchResponseContent.cs b/src/Microsoft.Graph.Core/Requests/Content/BatchResponseContent.cs index bcb3bb9c..b5f38ab8 100644 --- a/src/Microsoft.Graph.Core/Requests/Content/BatchResponseContent.cs +++ b/src/Microsoft.Graph.Core/Requests/Content/BatchResponseContent.cs @@ -32,7 +32,9 @@ public class BatchResponseContent public BatchResponseContent(HttpResponseMessage httpResponseMessage, Dictionary> errorMappings = null) { this.batchResponseMessage = httpResponseMessage ?? throw new ArgumentNullException(nameof(httpResponseMessage)); - this.apiErrorMappings = errorMappings ?? new(); + this.apiErrorMappings = errorMappings ?? new Dictionary>(StringComparer.OrdinalIgnoreCase) { + {"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.BatchRequestError, new Exception(parsable.GetErrorMessage())) } + }; } /// diff --git a/src/Microsoft.Graph.Core/Tasks/PageIterator.cs b/src/Microsoft.Graph.Core/Tasks/PageIterator.cs index f13bb116..f0792368 100644 --- a/src/Microsoft.Graph.Core/Tasks/PageIterator.cs +++ b/src/Microsoft.Graph.Core/Tasks/PageIterator.cs @@ -115,8 +115,7 @@ public static PageIterator CreatePageIterator(IRequest _processPageItemCallback = callback, _requestConfigurator = requestConfigurator, _errorMapping = errorMapping ?? new Dictionary>(StringComparer.OrdinalIgnoreCase) { - {"4XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) }, - {"5XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) } + {"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(parsable.GetErrorMessage())) } }, State = PagingState.NotStarted }; @@ -172,15 +171,14 @@ public static PageIterator CreatePageIterator(IRequest _asyncProcessPageItemCallback = asyncCallback, _requestConfigurator = requestConfigurator, _errorMapping = errorMapping ?? new Dictionary>(StringComparer.OrdinalIgnoreCase) { - {"4XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) }, - {"5XX", (parsable) =>new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) }, + {"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(parsable.GetErrorMessage())) } }, State = PagingState.NotStarted }; } /// - /// Iterate across the content of a a single results page with the callback. + /// Iterate across the content of a single results page with the callback. /// /// A boolean value that indicates whether the callback cancelled /// iterating across the page results or whether there are more pages to page. @@ -393,13 +391,6 @@ private static string ExtractNextLinkFromParsable(TCollectionPage parsableCollec // the next link property may not be defined in the response schema so we also check its presence in the additional data bag return parsableCollection.AdditionalData.TryGetValue(CoreConstants.OdataInstanceAnnotations.NextLink, out var nextLink) ? nextLink.ToString() : string.Empty; } - - private static string GetErrorMessageFromParsable(IParseNode responseParseNode) - { - var errorParseNode = responseParseNode.GetChildNode("error"); - // concatenate the error code and message - return $"{errorParseNode?.GetChildNode("code")?.GetStringValue()} : {errorParseNode?.GetChildNode("message")?.GetStringValue()}"; - } } /// diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/BatchRequestBuilderTests.cs b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/BatchRequestBuilderTests.cs index 2786b5f0..e074512c 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/BatchRequestBuilderTests.cs +++ b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/BatchRequestBuilderTests.cs @@ -6,10 +6,15 @@ namespace Microsoft.Graph.DotnetCore.Core.Test.Requests { using System.Collections.Generic; using System.Net.Http; + using System.Text; + using System.Threading; using System.Threading.Tasks; using Microsoft.Graph.Core.Requests; using Microsoft.Graph.DotnetCore.Core.Test.Mocks; + using Microsoft.Kiota.Abstractions; using Microsoft.Kiota.Abstractions.Authentication; + using Microsoft.Kiota.Abstractions.Serialization; + using Moq; using Xunit; public class BatchRequestBuilderTests @@ -42,5 +47,119 @@ public async Task BatchRequestBuilderAsync() Assert.Equal("{+baseurl}/$batch", requestInformation.UrlTemplate); Assert.Equal(baseClient.RequestAdapter, batchRequestBuilder.RequestAdapter); } + + + [Fact] + public async Task BatchRequestBuilderPostAsyncHandlesDoesNotThrowExceptionAsync() + { + // Arrange + var requestAdapter = new Mock(); + IBaseClient baseClient = new BaseClient(requestAdapter.Object); + + var errorResponseMessage = new HttpResponseMessage(System.Net.HttpStatusCode.OK) + { + Content = new StringContent("{}", Encoding.UTF8, "application/json"),//dummy content + }; + requestAdapter + .Setup(requestAdapter => requestAdapter.SendNoContentAsync(It.IsAny(), It.IsAny>>(), It.IsAny())) + .Callback((RequestInformation requestInfo, Dictionary> errorMapping, CancellationToken cancellationToken) => ((NativeResponseHandler)requestInfo.GetRequestOption().ResponseHandler).Value = errorResponseMessage) + .Returns(Task.FromResult(0)); + + // Act + var batchRequestBuilder = new BatchRequestBuilder(baseClient.RequestAdapter); + + // 4. Create batch request content to be sent out + // 4.1 Create HttpRequestMessages for the content + HttpRequestMessage httpRequestMessage1 = new HttpRequestMessage(HttpMethod.Get, "https://graph.microsoft.com/v1.0/me/"); + + // 4.2 Create batch request steps with request ids. + BatchRequestStep requestStep1 = new BatchRequestStep("1", httpRequestMessage1); + + // 4.3 Add batch request steps to BatchRequestContent. +#pragma warning disable CS0618 // Type or member is obsolete use the BatchRequestContentCollection for making batch requests + BatchRequestContent batchRequestContent = new BatchRequestContent(baseClient, requestStep1); +#pragma warning restore CS0618 // Type or member is obsolete use the BatchRequestContentCollection for making batch requests + var responseContent = await batchRequestBuilder.PostAsync(batchRequestContent); + + // Assert + Assert.NotNull(responseContent); + } + + [Fact] + public async Task BatchRequestBuilderPostAsyncHandlesNonSuccessStatusWithJsonResponseAsync() + { + // Arrange + var requestAdapter = new Mock(); + IBaseClient baseClient = new BaseClient(requestAdapter.Object); + + var errorResponseMessage = new HttpResponseMessage(System.Net.HttpStatusCode.Unauthorized) + { + Content = new StringContent("{\"error\": {\"code\": \"20117\",\"message\": \"An item with this name already exists in this location.\",\"innerError\":{\"request-id\": \"nothing1b13-45cd-new-92be873c5781\",\"date\": \"2019-03-22T23:17:50\"}}}", Encoding.UTF8, "application/json"), + }; + requestAdapter + .Setup(requestAdapter => requestAdapter.SendNoContentAsync(It.IsAny(), It.IsAny>>(), It.IsAny())) + .Callback((RequestInformation requestInfo, Dictionary> errorMapping, CancellationToken cancellationToken) => ((NativeResponseHandler)requestInfo.GetRequestOption().ResponseHandler).Value = errorResponseMessage) + .Returns(Task.FromResult(0)); + + // Act + var batchRequestBuilder = new BatchRequestBuilder(baseClient.RequestAdapter); + + // 4. Create batch request content to be sent out + // 4.1 Create HttpRequestMessages for the content + HttpRequestMessage httpRequestMessage1 = new HttpRequestMessage(HttpMethod.Get, "https://graph.microsoft.com/v1.0/me/"); + + // 4.2 Create batch request steps with request ids. + BatchRequestStep requestStep1 = new BatchRequestStep("1", httpRequestMessage1); + + // 4.3 Add batch request steps to BatchRequestContent. +#pragma warning disable CS0618 // Type or member is obsolete use the BatchRequestContentCollection for making batch requests + BatchRequestContent batchRequestContent = new BatchRequestContent(baseClient, requestStep1); +#pragma warning restore CS0618 // Type or member is obsolete use the BatchRequestContentCollection for making batch requests + var serviceException = await Assert.ThrowsAsync(async () => await batchRequestBuilder.PostAsync(batchRequestContent)); + + // Assert + Assert.Equal(ErrorConstants.Messages.BatchRequestError, serviceException.Message); + Assert.Equal(401, serviceException.ResponseStatusCode); + Assert.NotNull(serviceException.InnerException); + Assert.Equal("20117 : An item with this name already exists in this location.", serviceException.InnerException.Message); + } + + [Fact] + public async Task BatchRequestBuilderPostAsyncHandlesNonSuccessStatusWithNonJsonResponseAsync() + { + // Arrange + var requestAdapter = new Mock(); + IBaseClient baseClient = new BaseClient(requestAdapter.Object); + + var errorResponseMessage = new HttpResponseMessage(System.Net.HttpStatusCode.Conflict) + { + Content = new StringContent("This is random html", Encoding.UTF8, "text/plain"), + }; + requestAdapter + .Setup(requestAdapter => requestAdapter.SendNoContentAsync(It.IsAny(), It.IsAny>>(), It.IsAny())) + .Callback((RequestInformation requestInfo, Dictionary> errorMapping, CancellationToken cancellationToken) => ((NativeResponseHandler)requestInfo.GetRequestOption().ResponseHandler).Value = errorResponseMessage) + .Returns(Task.FromResult(0)); + + // Act + var batchRequestBuilder = new BatchRequestBuilder(baseClient.RequestAdapter); + + // 4. Create batch request content to be sent out + // 4.1 Create HttpRequestMessages for the content + HttpRequestMessage httpRequestMessage1 = new HttpRequestMessage(HttpMethod.Get, "https://graph.microsoft.com/v1.0/me/"); + + // 4.2 Create batch request steps with request ids. + BatchRequestStep requestStep1 = new BatchRequestStep("1", httpRequestMessage1); + + // 4.3 Add batch request steps to BatchRequestContent. +#pragma warning disable CS0618 // Type or member is obsolete use the BatchRequestContentCollection for making batch requests + BatchRequestContent batchRequestContent = new BatchRequestContent(baseClient, requestStep1); +#pragma warning restore CS0618 // Type or member is obsolete use the BatchRequestContentCollection for making batch requests + var serviceException = await Assert.ThrowsAsync(async () => await batchRequestBuilder.PostAsync(batchRequestContent)); + + // Assert + Assert.Equal(ErrorConstants.Messages.BatchRequestError, serviceException.Message); + Assert.Equal(409, serviceException.ResponseStatusCode); + Assert.Equal("This is random html", serviceException.RawResponseBody); + } } } From 7da16f725262d16444d928893a8e21f1e0198231 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 10 Sep 2024 12:19:16 +0300 Subject: [PATCH 2/9] cleanup a bunch of unused constants --- .../Exceptions/ErrorConstants.cs | 76 +++---------------- 1 file changed, 11 insertions(+), 65 deletions(-) diff --git a/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs b/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs index 80617e13..e0b833a3 100644 --- a/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs +++ b/src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs @@ -8,84 +8,30 @@ internal static class ErrorConstants { internal static class Codes { - internal static string GeneralException = "generalException"; - - internal static string InvalidRequest = "invalidRequest"; - - internal static string ItemNotFound = "itemNotFound"; - - internal static string NotAllowed = "notAllowed"; - - internal static string Timeout = "timeout"; - - internal static string TooManyRedirects = "tooManyRedirects"; - - internal static string TooManyRetries = "tooManyRetries"; - - internal static string MaximumValueExceeded = "MaximumValueExceeded"; - - internal static string InvalidArgument = "invalidArgument"; - - internal const string TemporarilyUnavailable = "temporarily_unavailable"; + internal const string GeneralException = "generalException"; } internal static class Messages { - internal static string AuthenticationProviderMissing = "Authentication provider is required before sending a request."; - - internal static string BaseUrlMissing = "Base URL cannot be null or empty."; - - internal static string InvalidTypeForDateConverter = "DateConverter can only serialize objects of type Date."; - - internal static string InvalidTypeForDateTimeOffsetConverter = "DateTimeOffsetConverter can only serialize objects of type DateTimeOffset."; - - internal static string LocationHeaderNotSetOnRedirect = "Location header not present in redirection response."; - - internal static string OverallTimeoutCannotBeSet = "Overall timeout cannot be set after the first request is sent."; - - internal static string RequestTimedOut = "The request timed out."; - - internal static string RequestUrlMissing = "Request URL is required to send a request."; - - internal static string TooManyRedirectsFormatString = "More than {0} redirects encountered while sending the request."; - - internal static string TooManyRetriesFormatString = "More than {0} retries encountered while sending the request."; - - internal static string UnableToCreateInstanceOfTypeFormatString = "Unable to create an instance of type {0}."; - - internal static string UnableToDeserializeDate = "Unable to deserialize the returned Date."; - - internal static string UnableToDeserializeDateTimeOffset = "Unable to deserialize the returned DateDateTimeOffset."; - - internal static string UnexpectedExceptionOnSend = "An error occurred sending the request."; - - internal static string UnexpectedExceptionResponse = "Unexpected exception returned from the service."; - - internal static string MaximumValueExceeded = "{0} exceeds the maximum value of {1}."; - - internal static string NullParameter = "{0} parameter cannot be null."; - - internal static string UnableToDeserializeContent = "Unable to deserialize content."; - - internal static string InvalidDependsOnRequestId = "Corresponding batch request id not found for the specified dependsOn relation."; + internal const string MaximumValueExceeded = "{0} exceeds the maximum value of {1}."; - internal static string ExpiredUploadSession = "Upload session expired. Upload cannot resume"; + internal const string NullParameter = "{0} parameter cannot be null."; - internal static string NoResponseForUpload = "No Response Received for upload."; + internal const string UnableToDeserializeContent = "Unable to deserialize content."; - internal static string NullValue = "{0} cannot be null."; + internal const string InvalidDependsOnRequestId = "Corresponding batch request id not found for the specified dependsOn relation."; - internal static string UnexpectedMsalException = "Unexpected exception returned from MSAL."; + internal const string ExpiredUploadSession = "Upload session expired. Upload cannot resume"; - internal static string UnexpectedException = "Unexpected exception occured while authenticating the request."; + internal const string NoResponseForUpload = "No Response Received for upload."; - internal static string MissingRetryAfterHeader = "Missing retry after header."; + internal const string MissingRetryAfterHeader = "Missing retry after header."; - internal static string PageIteratorRequestError = "Error occured when making a request with the page iterator. See inner exception for more details."; + internal const string PageIteratorRequestError = "Error occured when making a request with the page iterator. See inner exception for more details."; - internal static string BatchRequestError = "Error occured when making the batch request. See inner exception for more details."; + internal const string BatchRequestError = "Error occured when making the batch request. See inner exception for more details."; - public static string InvalidProxyArgument = "Proxy cannot be set more once. Proxy can only be set on the proxy or defaultHttpHandler argument and not both."; + internal const string InvalidProxyArgument = "Proxy cannot be set more once. Proxy can only be set on the proxy or defaultHttpHandler argument and not both."; } } } From f15167a4bfe2640bf89203788726f48269ef1c4e Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 10 Sep 2024 16:28:06 +0300 Subject: [PATCH 3/9] pr review feedback. --- src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index 084278b2..51eee0e7 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -108,7 +108,8 @@ private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpRes { if (httpResponseMessage.IsSuccessStatusCode) return; - if (CoreConstants.MimeTypeNames.Application.Json.Equals(httpResponseMessage.Content?.Headers?.ContentType?.MediaType, StringComparison.OrdinalIgnoreCase)) + var contentTypeString = httpResponseMessage.Content?.Headers?.ContentType?.MediaType is { } contentTypeMediaType && !string.IsNullOrEmpty(contentTypeMediaType) ? contentTypeMediaType : string.Empty; + if (httpResponseMessage.Content != null && contentTypeString.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) { using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false); using var document = await JsonDocument.ParseAsync(responseContent).ConfigureAwait(false); From 41517b26abdb6109f61c5a98442177409661ca61 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 10 Sep 2024 16:38:04 +0300 Subject: [PATCH 4/9] Update src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs Co-authored-by: Vincent Biret --- src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index 51eee0e7..800ef9e3 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -108,8 +108,7 @@ private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpRes { if (httpResponseMessage.IsSuccessStatusCode) return; - var contentTypeString = httpResponseMessage.Content?.Headers?.ContentType?.MediaType is { } contentTypeMediaType && !string.IsNullOrEmpty(contentTypeMediaType) ? contentTypeMediaType : string.Empty; - if (httpResponseMessage.Content != null && contentTypeString.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) + if (httpResponseMessage is {Content.Headers.ContentType.MediaType: string contentTypeMediaType} && !string.IsNullOrEmpty(contentTypeMediaType) && contentTypeString.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) { using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false); using var document = await JsonDocument.ParseAsync(responseContent).ConfigureAwait(false); From b92392ea0846cd515bad4a807b70b2e1894fef68 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 10 Sep 2024 16:48:44 +0300 Subject: [PATCH 5/9] resolve compile error. --- src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index 800ef9e3..4f331316 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -108,7 +108,7 @@ private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpRes { if (httpResponseMessage.IsSuccessStatusCode) return; - if (httpResponseMessage is {Content.Headers.ContentType.MediaType: string contentTypeMediaType} && !string.IsNullOrEmpty(contentTypeMediaType) && contentTypeString.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) + if (httpResponseMessage is { Content.Headers.ContentType.MediaType: string contentTypeMediaType } && !string.IsNullOrEmpty(contentTypeMediaType) && contentTypeMediaType.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) { using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false); using var document = await JsonDocument.ParseAsync(responseContent).ConfigureAwait(false); From c47970227c5303fdad8768432ba6fa4930e3885a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Sep 2024 09:51:01 -0400 Subject: [PATCH 6/9] chore: removes redundant check --- src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index 4f331316..0aab9333 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -108,7 +108,7 @@ private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpRes { if (httpResponseMessage.IsSuccessStatusCode) return; - if (httpResponseMessage is { Content.Headers.ContentType.MediaType: string contentTypeMediaType } && !string.IsNullOrEmpty(contentTypeMediaType) && contentTypeMediaType.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) + if (httpResponseMessage is { Content.Headers.ContentType.MediaType: string contentTypeMediaType } && contentTypeMediaType.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) { using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false); using var document = await JsonDocument.ParseAsync(responseContent).ConfigureAwait(false); From 85472de2b9ede0286c98dcf27db40fbaaf87b1b6 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 10 Sep 2024 17:34:50 +0300 Subject: [PATCH 7/9] pass cancellation token --- .../Requests/BatchRequestBuilder.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index 4f331316..b484679b 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -61,7 +61,7 @@ public async Task PostAsync(BatchRequestContent batchReque requestInfo.SetResponseHandler(nativeResponseHandler); await this.RequestAdapter.SendNoContentAsync(requestInfo, cancellationToken: cancellationToken); var httpResponseMessage = nativeResponseHandler.Value as HttpResponseMessage; - await ThrowIfFailedResponseAsync(httpResponseMessage); + await ThrowIfFailedResponseAsync(httpResponseMessage, cancellationToken); return new BatchResponseContent(httpResponseMessage, errorMappings); } @@ -104,14 +104,19 @@ public async Task ToPostRequestInformationAsync(BatchRequest return requestInfo; } - private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpResponseMessage) + private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpResponseMessage, CancellationToken cancellationToken) { if (httpResponseMessage.IsSuccessStatusCode) return; - if (httpResponseMessage is { Content.Headers.ContentType.MediaType: string contentTypeMediaType } && !string.IsNullOrEmpty(contentTypeMediaType) && contentTypeMediaType.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) + if (httpResponseMessage is { Content.Headers.ContentType.MediaType: string contentTypeMediaType } && contentTypeMediaType.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase)) { +#if NET5_0_OR_GREATER + using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + using var document = await JsonDocument.ParseAsync(responseContent, cancellationToken: cancellationToken).ConfigureAwait(false); +#else using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false); using var document = await JsonDocument.ParseAsync(responseContent).ConfigureAwait(false); +#endif var parsable = new JsonParseNode(document.RootElement); throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, new Exception(parsable.GetErrorMessage())); } From f29e4cd33bf6c1e3e541aa21614aabaa52570cc5 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Sep 2024 12:28:17 -0400 Subject: [PATCH 8/9] fix: moves parse async out of the condition since it always accepts a cancellation token Signed-off-by: Vincent Biret --- src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index b484679b..74e2a658 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -112,11 +112,10 @@ private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpRes { #if NET5_0_OR_GREATER using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - using var document = await JsonDocument.ParseAsync(responseContent, cancellationToken: cancellationToken).ConfigureAwait(false); #else using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false); - using var document = await JsonDocument.ParseAsync(responseContent).ConfigureAwait(false); #endif + using var document = await JsonDocument.ParseAsync(responseContent, cancellationToken: cancellationToken).ConfigureAwait(false); var parsable = new JsonParseNode(document.RootElement); throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, new Exception(parsable.GetErrorMessage())); } From 22133215fc2279a9b686f3ba81cbcd5c3bac54e9 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Sep 2024 12:30:52 -0400 Subject: [PATCH 9/9] fix: adds missing cancellation token parameter Signed-off-by: Vincent Biret --- src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs index 74e2a658..de4b8d7c 100644 --- a/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs +++ b/src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs @@ -123,7 +123,11 @@ private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpRes var responseStringContent = string.Empty; if (httpResponseMessage.Content != null) { +#if NET5_0_OR_GREATER + responseStringContent = await httpResponseMessage.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); +#else responseStringContent = await httpResponseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); +#endif } throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, responseStringContent);