Skip to content

Commit

Permalink
fix: fixes performance issue when using the `AddBatchRequestStepAsync…
Browse files Browse the repository at this point in the history
…` method due to the client trying to authenticate the request on conversion
  • Loading branch information
andrueastman committed Jan 28, 2025
1 parent 497fc19 commit a5b3950
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 14 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/validatePullRequest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 5 additions & 5 deletions src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,16 @@
</None>
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.3.0" />
<PackageReference Include="Microsoft.IdentityModel.Validators" Version="8.3.0" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.3.1" />
<PackageReference Include="Microsoft.IdentityModel.Validators" Version="8.3.1" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
<PackageReference Include="Microsoft.Kiota.Abstractions" Version="1.16.4" />
<PackageReference Include="Microsoft.Kiota.Authentication.Azure" Version="1.16.4" />
<PackageReference Include="Microsoft.Kiota.Serialization.Json" Version="1.16.4" />
<PackageReference Include="Microsoft.Kiota.Serialization.Text" Version="1.16.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Form" Version="1.16.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Text" Version="1.16.4" />
<PackageReference Include="Microsoft.Kiota.Serialization.Form" Version="1.16.4" />
<PackageReference Include="Microsoft.Kiota.Http.HttpClientLibrary" Version="1.16.4" />
<PackageReference Include="Microsoft.Kiota.Serialization.Multipart" Version="1.16.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Multipart" Version="1.16.4" />
<PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.12.19">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
Expand Down
10 changes: 8 additions & 2 deletions src/Microsoft.Graph.Core/Requests/Content/BatchRequestContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Microsoft.Graph
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Kiota.Abstractions;
using Microsoft.Kiota.Abstractions.Authentication;

/// <summary>
/// A <see cref="HttpContent"/> implementation to handle json batch requests.
Expand Down Expand Up @@ -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);
Expand All @@ -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
};
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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<BaseGraphRequestAdapter>(batchRequestContent.RequestAdapter);
Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(1));
Assert.Equal(expectedContent, requestContent);
}

[Fact]
public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestContentSupportsNonJsonPayloadAsync()
{
Expand All @@ -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 });
}
Expand Down Expand Up @@ -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 });
}
Expand Down Expand Up @@ -532,7 +555,7 @@ public void BatchRequestContent_AddBatchRequestStepWithHttpRequestMessageToBatch

// Assert
var exception = Assert.Throws<ArgumentException>(() => 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));
}
Expand Down Expand Up @@ -612,7 +635,7 @@ public async Task BatchRequestContent_AddBatchRequestStepWithBaseRequestToBatchR
var exception = await Assert.ThrowsAsync<ArgumentException>(() => 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));
}
Expand Down

0 comments on commit a5b3950

Please sign in to comment.