Skip to content

Commit 6f77e19

Browse files
author
Andrew Omondi
committed
fix: fixes performance issue when using the AddBatchRequestStepAsync method due to the client trying to authenticate the request on conversion using the
1 parent 497fc19 commit 6f77e19

File tree

3 files changed

+40
-12
lines changed

3 files changed

+40
-12
lines changed

src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,16 @@
6262
</None>
6363
</ItemGroup>
6464
<ItemGroup>
65-
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.3.0" />
66-
<PackageReference Include="Microsoft.IdentityModel.Validators" Version="8.3.0" />
65+
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.3.1" />
66+
<PackageReference Include="Microsoft.IdentityModel.Validators" Version="8.3.1" />
6767
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
6868
<PackageReference Include="Microsoft.Kiota.Abstractions" Version="1.16.4" />
6969
<PackageReference Include="Microsoft.Kiota.Authentication.Azure" Version="1.16.4" />
7070
<PackageReference Include="Microsoft.Kiota.Serialization.Json" Version="1.16.4" />
71-
<PackageReference Include="Microsoft.Kiota.Serialization.Text" Version="1.16.3" />
72-
<PackageReference Include="Microsoft.Kiota.Serialization.Form" Version="1.16.3" />
71+
<PackageReference Include="Microsoft.Kiota.Serialization.Text" Version="1.16.4" />
72+
<PackageReference Include="Microsoft.Kiota.Serialization.Form" Version="1.16.4" />
7373
<PackageReference Include="Microsoft.Kiota.Http.HttpClientLibrary" Version="1.16.4" />
74-
<PackageReference Include="Microsoft.Kiota.Serialization.Multipart" Version="1.16.3" />
74+
<PackageReference Include="Microsoft.Kiota.Serialization.Multipart" Version="1.16.4" />
7575
<PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.12.19">
7676
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
7777
<PrivateAssets>all</PrivateAssets>

src/Microsoft.Graph.Core/Requests/Content/BatchRequestContent.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace Microsoft.Graph
1717
using System.Threading;
1818
using System.Threading.Tasks;
1919
using Microsoft.Kiota.Abstractions;
20+
using Microsoft.Kiota.Abstractions.Authentication;
2021

2122
/// <summary>
2223
/// A <see cref="HttpContent"/> implementation to handle json batch requests.
@@ -71,7 +72,7 @@ public BatchRequestContent(IRequestAdapter requestAdapter, params BatchRequestSt
7172
if (batchRequestSteps == null)
7273
throw new ArgumentNullException(nameof(batchRequestSteps));
7374

74-
if (batchRequestSteps.Count() > CoreConstants.BatchRequest.MaxNumberOfRequests)
75+
if (batchRequestSteps.Length > CoreConstants.BatchRequest.MaxNumberOfRequests)
7576
throw new ArgumentException(string.Format(ErrorConstants.Messages.MaximumValueExceeded, "Number of batch request steps", CoreConstants.BatchRequest.MaxNumberOfRequests));
7677

7778
this.Headers.ContentType = new MediaTypeHeaderValue(CoreConstants.MimeTypeNames.Application.Json);
@@ -87,7 +88,12 @@ public BatchRequestContent(IRequestAdapter requestAdapter, params BatchRequestSt
8788
AddBatchRequestStep(requestStep);
8889
}
8990

90-
this.RequestAdapter = requestAdapter ?? throw new ArgumentNullException(nameof(requestAdapter));
91+
// as we only use the adapter to serialize request using the `ConvertToNativeRequestAsync` interface,
92+
// we don't want to make extra request to the authentication provider as the request does not need the authentication header.
93+
this.RequestAdapter = new BaseGraphRequestAdapter(new AnonymousAuthenticationProvider())
94+
{
95+
BaseUrl = requestAdapter.BaseUrl
96+
};
9197
}
9298

9399
/// <summary>

tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public void BatchRequestContent_RemoveBatchRequestStepWithIdForNonExistingId()
181181

182182
Assert.False(isSuccess);
183183
Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(2));
184-
Assert.Same(batchRequestStep2.DependsOn.First(), batchRequestContent.BatchRequestSteps["2"].DependsOn.First());
184+
Assert.Same(batchRequestStep2.DependsOn[0], batchRequestContent.BatchRequestSteps["2"].DependsOn[0]);
185185
}
186186

187187
[Fact]
@@ -311,6 +311,28 @@ public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestCont
311311
Assert.Equal(expectedContent, requestContent);
312312
}
313313

314+
[Fact]
315+
public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestContentFromRequestInformationDoesNotAddAuthHeaderAsync()
316+
{
317+
BatchRequestContent batchRequestContent = new BatchRequestContent(client);
318+
RequestInformation requestInformation = new RequestInformation() { HttpMethod = Method.GET, UrlTemplate = REQUEST_URL };
319+
await batchRequestContent.AddBatchRequestStepAsync(requestInformation, "2");
320+
321+
string requestContent;
322+
// We get the contents of the stream as string for comparison.
323+
using (Stream requestStream = await batchRequestContent.GetBatchRequestContentAsync())
324+
using (StreamReader reader = new StreamReader(requestStream))
325+
{
326+
requestContent = await reader.ReadToEndAsync();
327+
}
328+
329+
string expectedContent = "{\"requests\":[{\"id\":\"2\",\"url\":\"/me\",\"method\":\"GET\"}]}";
330+
331+
Assert.NotNull(requestContent);
332+
Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(1));
333+
Assert.Equal(expectedContent, requestContent);
334+
}
335+
314336
[Fact]
315337
public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestContentSupportsNonJsonPayloadAsync()
316338
{
@@ -331,7 +353,7 @@ public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestCont
331353
string requestContent;
332354
// we do this to get a version of the json that is indented
333355
using (Stream requestStream = await batchRequestContent.GetBatchRequestContentAsync())
334-
using (JsonDocument jsonDocument = JsonDocument.Parse(requestStream))
356+
using (JsonDocument jsonDocument = await JsonDocument.ParseAsync(requestStream))
335357
{
336358
requestContent = JsonSerializer.Serialize(jsonDocument.RootElement, new JsonSerializerOptions() { WriteIndented = true });
337359
}
@@ -409,7 +431,7 @@ public async System.Threading.Tasks.Task BatchRequestContent_GetBatchRequestCont
409431
string requestContent;
410432
// we do this to get a version of the json that is indented
411433
using (Stream requestStream = await batchRequestContent.GetBatchRequestContentAsync())
412-
using (JsonDocument jsonDocument = JsonDocument.Parse(requestStream))
434+
using (JsonDocument jsonDocument = await JsonDocument.ParseAsync(requestStream))
413435
{
414436
requestContent = JsonSerializer.Serialize(jsonDocument.RootElement, new JsonSerializerOptions() { WriteIndented = true });
415437
}
@@ -532,7 +554,7 @@ public void BatchRequestContent_AddBatchRequestStepWithHttpRequestMessageToBatch
532554

533555
// Assert
534556
var exception = Assert.Throws<ArgumentException>(() => batchRequestContent.AddBatchRequestStep(extraHttpRequestMessage));//Assert we throw exception on excess add
535-
//Assert.Equal(ErrorConstants.Codes.MaximumValueExceeded, exception.Error.Code);
557+
Assert.Equal(ErrorConstants.Messages.MaximumValueExceeded, exception.Message);
536558
Assert.NotNull(batchRequestContent.BatchRequestSteps);
537559
Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(CoreConstants.BatchRequest.MaxNumberOfRequests));
538560
}
@@ -612,7 +634,7 @@ public async Task BatchRequestContent_AddBatchRequestStepWithBaseRequestToBatchR
612634
var exception = await Assert.ThrowsAsync<ArgumentException>(() => batchRequestContent.AddBatchRequestStepAsync(extraRequestInformation));
613635

614636
// Assert
615-
//Assert.Equal(ErrorConstants.Codes.MaximumValueExceeded, exception.Error.Code);
637+
Assert.Equal(ErrorConstants.Messages.MaximumValueExceeded, exception.Message);
616638
Assert.NotNull(batchRequestContent.BatchRequestSteps);
617639
Assert.True(batchRequestContent.BatchRequestSteps.Count.Equals(CoreConstants.BatchRequest.MaxNumberOfRequests));
618640
}

0 commit comments

Comments
 (0)