From a5b395051ea92f2b92c3fa0a758e41322ddd62c4 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 Jan 2025 12:28:52 +0300 Subject: [PATCH 1/6] fix: fixes performance issue when using the `AddBatchRequestStepAsync` method due to the client trying to authenticate the request on conversion --- .github/workflows/validatePullRequest.yml | 4 +-- .../Microsoft.Graph.Core.csproj | 10 +++--- .../Requests/Content/BatchRequestContent.cs | 10 ++++-- .../Content/BatchRequestContentTests.cs | 33 ++++++++++++++++--- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/.github/workflows/validatePullRequest.yml b/.github/workflows/validatePullRequest.yml index 7311379a3..37bd58919 100644 --- a/.github/workflows/validatePullRequest.yml +++ b/.github/workflows/validatePullRequest.yml @@ -65,8 +65,8 @@ jobs: - name: Setup .NET uses: actions/setup-dotnet@v4 with: - dotnet-version: 8.x + dotnet-version: 9.x - name: Validate Trimming warnings - run: dotnet publish -c Release -r win-x64 /p:TreatWarningsAsErrors=true /warnaserror -f net8.0 + run: dotnet publish -c Release -r win-x64 /p:TreatWarningsAsErrors=true /warnaserror -f net9.0 working-directory: ./tests/Microsoft.Graph.DotnetCore.Core.Trimming \ No newline at end of file diff --git a/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj b/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj index 719f2d253..1e4fc0ece 100644 --- a/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj +++ b/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj @@ -62,16 +62,16 @@ - - + + - - + + - + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContent.cs b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContent.cs index 1679a9649..cc782d4a6 100644 --- a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContent.cs +++ b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContent.cs @@ -17,6 +17,7 @@ namespace Microsoft.Graph using System.Threading; using System.Threading.Tasks; using Microsoft.Kiota.Abstractions; + using Microsoft.Kiota.Abstractions.Authentication; /// /// A implementation to handle json batch requests. @@ -71,7 +72,7 @@ public BatchRequestContent(IRequestAdapter requestAdapter, params BatchRequestSt if (batchRequestSteps == null) throw new ArgumentNullException(nameof(batchRequestSteps)); - if (batchRequestSteps.Count() > CoreConstants.BatchRequest.MaxNumberOfRequests) + if (batchRequestSteps.Length > CoreConstants.BatchRequest.MaxNumberOfRequests) throw new ArgumentException(string.Format(ErrorConstants.Messages.MaximumValueExceeded, "Number of batch request steps", CoreConstants.BatchRequest.MaxNumberOfRequests)); this.Headers.ContentType = new MediaTypeHeaderValue(CoreConstants.MimeTypeNames.Application.Json); @@ -87,7 +88,12 @@ public BatchRequestContent(IRequestAdapter requestAdapter, params BatchRequestSt AddBatchRequestStep(requestStep); } - this.RequestAdapter = requestAdapter ?? throw new ArgumentNullException(nameof(requestAdapter)); + // as we only use the adapter to serialize request using the `ConvertToNativeRequestAsync` interface, + // we don't want to make extra request to the authentication provider as the request does not need the authentication header. + this.RequestAdapter = new BaseGraphRequestAdapter(new AnonymousAuthenticationProvider()) + { + BaseUrl = requestAdapter.BaseUrl + }; } /// diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs index fe798971e..9373c17bf 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs +++ b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs @@ -181,7 +181,7 @@ public void BatchRequestContent_RemoveBatchRequestStepWithIdForNonExistingId() Assert.False(isSuccess); Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(2)); - Assert.Same(batchRequestStep2.DependsOn.First(), batchRequestContent.BatchRequestSteps["2"].DependsOn.First()); + Assert.Same(batchRequestStep2.DependsOn[0], batchRequestContent.BatchRequestSteps["2"].DependsOn[0]); } [Fact] @@ -311,6 +311,29 @@ public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestCont Assert.Equal(expectedContent, requestContent); } + [Fact] + public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestContentFromRequestInformationDoesNotAddAuthHeaderAsync() + { + BatchRequestContent batchRequestContent = new BatchRequestContent(client); + RequestInformation requestInformation = new RequestInformation() { HttpMethod = Method.GET, UrlTemplate = REQUEST_URL }; + await batchRequestContent.AddBatchRequestStepAsync(requestInformation, "2"); + + string requestContent; + // We get the contents of the stream as string for comparison. + using (Stream requestStream = await batchRequestContent.GetBatchRequestContentAsync()) + using (StreamReader reader = new StreamReader(requestStream)) + { + requestContent = await reader.ReadToEndAsync(); + } + + string expectedContent = "{\"requests\":[{\"id\":\"2\",\"url\":\"/me\",\"method\":\"GET\"}]}"; //Auth Header Absent. + + Assert.NotNull(requestContent); + Assert.IsType(batchRequestContent.RequestAdapter); + Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(1)); + Assert.Equal(expectedContent, requestContent); + } + [Fact] public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestContentSupportsNonJsonPayloadAsync() { @@ -331,7 +354,7 @@ public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestCont string requestContent; // we do this to get a version of the json that is indented using (Stream requestStream = await batchRequestContent.GetBatchRequestContentAsync()) - using (JsonDocument jsonDocument = JsonDocument.Parse(requestStream)) + using (JsonDocument jsonDocument = await JsonDocument.ParseAsync(requestStream)) { requestContent = JsonSerializer.Serialize(jsonDocument.RootElement, new JsonSerializerOptions() { WriteIndented = true }); } @@ -409,7 +432,7 @@ public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestCont string requestContent; // we do this to get a version of the json that is indented using (Stream requestStream = await batchRequestContent.GetBatchRequestContentAsync()) - using (JsonDocument jsonDocument = JsonDocument.Parse(requestStream)) + using (JsonDocument jsonDocument = await JsonDocument.ParseAsync(requestStream)) { requestContent = JsonSerializer.Serialize(jsonDocument.RootElement, new JsonSerializerOptions() { WriteIndented = true }); } @@ -532,7 +555,7 @@ public void BatchRequestContent_AddBatchRequestStepWithHttpRequestMessageToBatch // Assert var exception = Assert.Throws(() => batchRequestContent.AddBatchRequestStep(extraHttpRequestMessage));//Assert we throw exception on excess add - //Assert.Equal(ErrorConstants.Codes.MaximumValueExceeded, exception.Error.Code); + Assert.Equal(ErrorConstants.Messages.MaximumValueExceeded, exception.Message); Assert.NotNull(batchRequestContent.BatchRequestSteps); Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(CoreConstants.BatchRequest.MaxNumberOfRequests)); } @@ -612,7 +635,7 @@ public async Task BatchRequestContent_AddBatchRequestStepWithBaseRequestToBatchR var exception = await Assert.ThrowsAsync(() => batchRequestContent.AddBatchRequestStepAsync(extraRequestInformation)); // Assert - //Assert.Equal(ErrorConstants.Codes.MaximumValueExceeded, exception.Error.Code); + Assert.Equal(ErrorConstants.Messages.MaximumValueExceeded, exception.Message); Assert.NotNull(batchRequestContent.BatchRequestSteps); Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(CoreConstants.BatchRequest.MaxNumberOfRequests)); } From 4d80a83ac4a0efd56373b2b66704f7bd65a15cd9 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 Jan 2025 12:32:23 +0300 Subject: [PATCH 2/6] chore: bump the trimming project version --- .../Microsoft.Graph.DotnetCore.Core.Trimming.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj index 928343674..3bfa7a49d 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj +++ b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj @@ -1,7 +1,7 @@ Exe - net8.0 + net9.0 enable enable true From 7c01730c593a425da1ae5c6bf4145d1ad363dbd5 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 Jan 2025 12:52:03 +0300 Subject: [PATCH 3/6] update project global file --- tests/Microsoft.Graph.DotnetCore.Core.Trimming/global.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/global.json b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/global.json index d251ac4e7..b81865078 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/global.json +++ b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "8.0.101", /* https://github.com/dotnet/maui/wiki/.NET-7-and-.NET-MAUI */ + "version": "9.0.102", /* https://github.com/dotnet/maui/wiki/.NET-7-and-.NET-MAUI */ "rollForward": "major" } } From 1d9446560480b989007810c1b0d0bc977ce7a9f0 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 Jan 2025 14:15:09 +0300 Subject: [PATCH 4/6] attempt to suppress warnings --- .../Microsoft.Graph.DotnetCore.Core.Trimming.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj index 3bfa7a49d..4a99cd482 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj +++ b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj @@ -10,6 +10,7 @@ true true IL3000 + true From dc6e44c839d7a50959d7732267dbfac6e4649247 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 Jan 2025 14:20:18 +0300 Subject: [PATCH 5/6] chore: move around supression --- src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj | 1 + .../Microsoft.Graph.DotnetCore.Core.Trimming.csproj | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj b/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj index 1e4fc0ece..465623ae7 100644 --- a/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj +++ b/src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj @@ -35,6 +35,7 @@ true true true + true True README.md NU5048;NETSDK1202 diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj index 4a99cd482..3bfa7a49d 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj +++ b/tests/Microsoft.Graph.DotnetCore.Core.Trimming/Microsoft.Graph.DotnetCore.Core.Trimming.csproj @@ -10,7 +10,6 @@ true true IL3000 - true From 14af57e9cc4baa7a08179e26e454065a6e7fe7a8 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 Jan 2025 14:41:08 +0300 Subject: [PATCH 6/6] chore: fix broken test --- .../Requests/Content/BatchRequestContentTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs index 9373c17bf..4ce0801fc 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs +++ b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs @@ -555,7 +555,7 @@ public void BatchRequestContent_AddBatchRequestStepWithHttpRequestMessageToBatch // Assert var exception = Assert.Throws(() => batchRequestContent.AddBatchRequestStep(extraHttpRequestMessage));//Assert we throw exception on excess add - Assert.Equal(ErrorConstants.Messages.MaximumValueExceeded, exception.Message); + Assert.Equal(string.Format(ErrorConstants.Messages.MaximumValueExceeded, "Number of batch request steps", CoreConstants.BatchRequest.MaxNumberOfRequests), exception.Message); Assert.NotNull(batchRequestContent.BatchRequestSteps); Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(CoreConstants.BatchRequest.MaxNumberOfRequests)); } @@ -635,7 +635,7 @@ public async Task BatchRequestContent_AddBatchRequestStepWithBaseRequestToBatchR var exception = await Assert.ThrowsAsync(() => batchRequestContent.AddBatchRequestStepAsync(extraRequestInformation)); // Assert - Assert.Equal(ErrorConstants.Messages.MaximumValueExceeded, exception.Message); + Assert.Equal(string.Format(ErrorConstants.Messages.MaximumValueExceeded, "Number of batch request steps", CoreConstants.BatchRequest.MaxNumberOfRequests), exception.Message); Assert.NotNull(batchRequestContent.BatchRequestSteps); Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(CoreConstants.BatchRequest.MaxNumberOfRequests)); }