Skip to content

Commit 9d77596

Browse files
peombwapeombwa
authored
Merge pull request #378 from microsoftgraph/httpRequestClone
Adds cloning of HttpRequestMessage before retry
2 parents eb141e8 + ccf11c7 commit 9d77596

File tree

10 files changed

+92
-82
lines changed

10 files changed

+92
-82
lines changed

‎src/Microsoft.Graph.Core/Extensions/RequestExtensions.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
namespace Microsoft.Graph
66
{
77
using System.Net.Http;
8+
using System.Threading.Tasks;
9+
810
/// <summary>
911
/// Contains extension methods for <see cref="HttpRequestMessage"/>
1012
/// </summary>
@@ -26,5 +28,36 @@ internal static bool IsBuffered(this HttpRequestMessage httpRequestMessage)
2628
}
2729
return true;
2830
}
31+
32+
/// <summary>
33+
/// Create a new HTTP request by copying previous HTTP request's headers and properties from response's request message.
34+
/// </summary>
35+
/// <param name="originalRequest">The previous <see cref="HttpRequestMessage"/> needs to be copy.</param>
36+
/// <returns>The <see cref="HttpRequestMessage"/>.</returns>
37+
/// <remarks>
38+
/// Re-issue a new HTTP request with the previous request's headers and properities
39+
/// </remarks>
40+
internal static async Task<HttpRequestMessage> CloneAsync(this HttpRequestMessage originalRequest)
41+
{
42+
var newRequest = new HttpRequestMessage(originalRequest.Method, originalRequest.RequestUri);
43+
44+
foreach (var header in originalRequest.Headers)
45+
{
46+
newRequest.Headers.TryAddWithoutValidation(header.Key, header.Value);
47+
}
48+
49+
foreach (var property in originalRequest.Properties)
50+
{
51+
newRequest.Properties.Add(property);
52+
}
53+
54+
// Set Content if previous request contains
55+
if (originalRequest.Content != null && originalRequest.Content.Headers.ContentLength != 0)
56+
{
57+
newRequest.Content = new StreamContent(await originalRequest.Content.ReadAsStreamAsync());
58+
}
59+
60+
return newRequest;
61+
}
2962
}
3063
}

‎src/Microsoft.Graph.Core/Requests/AuthenticationHandler.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,16 @@ private async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage httpR
6464
int retryAttempt = 0;
6565
while (retryAttempt < MaxRetry)
6666
{
67-
var originalRequest = httpResponseMessage.RequestMessage;
67+
// general clone request with internal CloneAsync (see CloneAsync for details) extension method
68+
var newRequest = await httpResponseMessage.RequestMessage.CloneAsync();
6869

6970
// Authenticate request using AuthenticationProvider
70-
await AuthenticationProvider.AuthenticateRequestAsync(originalRequest);
71-
httpResponseMessage = await base.SendAsync(originalRequest, cancellationToken);
71+
await AuthenticationProvider.AuthenticateRequestAsync(newRequest);
72+
httpResponseMessage = await base.SendAsync(newRequest, cancellationToken);
7273

7374
retryAttempt++;
7475

75-
if (!IsUnauthorized(httpResponseMessage) || !originalRequest.IsBuffered())
76+
if (!IsUnauthorized(httpResponseMessage) || !newRequest.IsBuffered())
7677
{
7778
// Re-issue the request to get a new access token
7879
return httpResponseMessage;

‎src/Microsoft.Graph.Core/Requests/RedirectHandler.cs

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
6565

6666
while (redirectCount < maxRedirects)
6767
{
68-
// general copy request with internal CopyRequest(see copyRequest for details) method
69-
var newRequest = await CopyRequest(response.RequestMessage);
68+
// general clone request with internal CloneAsync (see CloneAsync for details) extension method
69+
var newRequest = await response.RequestMessage.CloneAsync();
7070

7171
// status code == 303: change request method from post to get and content to be null
7272
if (response.StatusCode == HttpStatusCode.SeeOther)
@@ -107,37 +107,6 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
107107

108108
}
109109

110-
/// <summary>
111-
/// Create a new HTTP request by copying previous HTTP request's headers and properties from response's request message.
112-
/// </summary>
113-
/// <param name="originalRequest">The previous <see cref="HttpRequestMessage"/> needs to be copy.</param>
114-
/// <returns>The <see cref="HttpRequestMessage"/>.</returns>
115-
/// <remarks>
116-
/// Re-issue a new HTTP request with the previous request's headers and properities
117-
/// </remarks>
118-
internal async Task<HttpRequestMessage> CopyRequest(HttpRequestMessage originalRequest)
119-
{
120-
var newRequest = new HttpRequestMessage(originalRequest.Method, originalRequest.RequestUri);
121-
122-
foreach (var header in originalRequest.Headers)
123-
{
124-
newRequest.Headers.TryAddWithoutValidation(header.Key, header.Value);
125-
}
126-
127-
foreach (var property in originalRequest.Properties)
128-
{
129-
newRequest.Properties.Add(property);
130-
}
131-
132-
// Set Content if previous request contains
133-
if (originalRequest.Content != null && originalRequest.Content.Headers.ContentLength != 0)
134-
{
135-
newRequest.Content = new StreamContent(await originalRequest.Content.ReadAsStreamAsync());
136-
}
137-
138-
return newRequest;
139-
}
140-
141110

142111
/// <summary>
143112
/// Checks whether <see cref="HttpStatusCode"/> is redirected

‎src/Microsoft.Graph.Core/Requests/RetryHandler.cs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,15 @@ protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage
7171
/// <returns></returns>
7272
public async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage response, CancellationToken cancellationToken)
7373
{
74-
75-
7674
int retryCount = 0;
7775

78-
7976
while (retryCount < MaxRetry)
8077
{
81-
8278
// Call Delay method to get delay time from response's Retry-After header or by exponential backoff
8379
Task delay = Delay(response, retryCount, cancellationToken);
8480

85-
// Get the original request
86-
var request = response.RequestMessage;
81+
// general clone request with internal CloneAsync (see CloneAsync for details) extension method
82+
var request = await response.RequestMessage.CloneAsync();
8783

8884
// Increase retryCount and then update Retry-Attempt in request header
8985
retryCount++;
@@ -110,8 +106,6 @@ public async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage respon
110106

111107
}
112108

113-
114-
115109
/// <summary>
116110
/// Check the HTTP response's status to determine whether it should be retried or not.
117111
/// </summary>
@@ -127,7 +121,6 @@ public bool IsRetry(HttpResponseMessage response)
127121
return false;
128122
}
129123

130-
131124
/// <summary>
132125
/// Update Retry-Attempt header in the HTTP request
133126
/// </summary>

‎tests/Microsoft.Graph.Core.Test/Requests/AuthenticationHandlerTests.cs

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ public async Task AuthHandler_NonUnauthorizedStatusShouldPassThrough(HttpStatusC
7171

7272
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
7373

74-
Assert.AreSame(response, expectedResponse, "Doesn't return a successful response");
75-
Assert.AreSame(response.RequestMessage, httpRequestMessage, "Http response message sets wrong request message");
74+
Assert.AreSame(response, expectedResponse, "Doesn't return a successful response.");
75+
Assert.AreSame(response.RequestMessage, httpRequestMessage, "Reissued a new http request.");
7676
}
7777

7878
[TestMethod]
@@ -86,9 +86,9 @@ public async Task AuthHandler_ShouldRetryUnauthorizedGetRequest()
8686

8787
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
8888

89-
Assert.AreSame(response.RequestMessage, httpRequestMessage, "Http response message sets wrong request message");
90-
Assert.AreSame(response, expectedResponse, "Retry didn't happen");
91-
Assert.IsNull(response.RequestMessage.Content, "Content is not null");
89+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage, "Doesn't reissue a new http request.");
90+
Assert.AreSame(response, expectedResponse, "Retry didn't happen.");
91+
Assert.IsNull(response.RequestMessage.Content, "Content is not null.");
9292
}
9393

9494
[TestMethod]
@@ -102,9 +102,9 @@ public async Task AuthHandler_ShouldRetryUnauthorizedPostRequestWithNoContent()
102102

103103
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
104104

105-
Assert.AreSame(response.RequestMessage, httpRequestMessage, "Http response message sets wrong request message");
106-
Assert.AreSame(response, expectedResponse, "Retry didn't happen");
107-
Assert.IsNull(response.RequestMessage.Content, "Content is not null");
105+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage, "Doesn't reissue a new http request.");
106+
Assert.AreSame(response, expectedResponse, "Retry didn't happen.");
107+
Assert.IsNull(response.RequestMessage.Content, "Content is not null.");
108108
}
109109

110110
[TestMethod]
@@ -120,10 +120,11 @@ public async Task AuthHandler_ShouldRetryUnauthorizedPostRequestWithBufferConten
120120

121121
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
122122

123-
Assert.AreSame(response, okResponse, "Retry didn't happen");
124-
Assert.AreNotSame(response, unauthorizedResponse, "Retry didn't happen");
125-
Assert.IsNotNull(response.RequestMessage.Content, "The request content is null");
126-
Assert.AreEqual(response.RequestMessage.Content.ReadAsStringAsync().Result, "Hello World!", "Content changed");
123+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage, "Doesn't reissue a new http request.");
124+
Assert.AreSame(response, okResponse, "Retry didn't happen.");
125+
Assert.AreNotSame(response, unauthorizedResponse, "Retry didn't happen.");
126+
Assert.IsNotNull(response.RequestMessage.Content, "The request content is null.");
127+
Assert.AreEqual(response.RequestMessage.Content.ReadAsStringAsync().Result, "Hello World!", "Content changed.");
127128
}
128129

129130
[TestMethod]
@@ -139,10 +140,11 @@ public async Task AuthHandler_ShouldRetryUnauthorizedPatchRequestWithBufferConte
139140

140141
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
141142

142-
Assert.AreSame(response, okResponse, "Retry didn't happen");
143-
Assert.AreNotSame(response, unauthorizedResponse, "Retry didn't happen");
144-
Assert.IsNotNull(response.RequestMessage.Content, "The request content is null");
145-
Assert.AreEqual(response.RequestMessage.Content.ReadAsStringAsync().Result, "Hello World!", "Content changed");
143+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage, "Doesn't reissue a new http request.");
144+
Assert.AreSame(response, okResponse, "Retry didn't happen.");
145+
Assert.AreNotSame(response, unauthorizedResponse, "Retry didn't happen.");
146+
Assert.IsNotNull(response.RequestMessage.Content, "The request content is null.");
147+
Assert.AreEqual(response.RequestMessage.Content.ReadAsStringAsync().Result, "Hello World!", "Content changed.");
146148
}
147149

148150
[TestMethod]
@@ -159,10 +161,11 @@ public async Task AuthHandler_ShouldNotRetryUnauthorizedPutRequestWithStreamCont
159161

160162
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
161163

162-
Assert.AreNotSame(response, okResponse, "Unexpected retry");
163-
Assert.AreSame(response, unauthorizedResponse, "Unexpected retry");
164-
Assert.IsNotNull(response.RequestMessage.Content, "Request content is null");
165-
Assert.AreEqual(response.RequestMessage.Content.Headers.ContentLength, -1, "Request content length changed");
164+
Assert.AreSame(response.RequestMessage, httpRequestMessage, "Reissued a new http request.");
165+
Assert.AreNotSame(response, okResponse, "Unexpected retry.");
166+
Assert.AreSame(response, unauthorizedResponse, "Unexpected retry.");
167+
Assert.IsNotNull(response.RequestMessage.Content, "Request content is null.");
168+
Assert.AreEqual(response.RequestMessage.Content.Headers.ContentLength, -1, "Request content length changed.");
166169
}
167170

168171
[TestMethod]
@@ -179,10 +182,11 @@ public async Task AuthHandler_ShouldNotRetryUnauthorizedPatchRequestWithStreamCo
179182

180183
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
181184

182-
Assert.AreNotSame(response, okResponse, "Unexpected retry");
183-
Assert.AreSame(response, unauthorizedResponse, "Unexpected retry");
184-
Assert.IsNotNull(response.RequestMessage.Content, "Request content is null");
185-
Assert.AreEqual(response.RequestMessage.Content.Headers.ContentLength, -1, "Request content length changed");
185+
Assert.AreSame(response.RequestMessage, httpRequestMessage, "Reissued a new http request.");
186+
Assert.AreNotSame(response, okResponse, "Unexpected retry.");
187+
Assert.AreSame(response, unauthorizedResponse, "Unexpected retry.");
188+
Assert.IsNotNull(response.RequestMessage.Content, "Request content is null.");
189+
Assert.AreEqual(response.RequestMessage.Content.Headers.ContentLength, -1, "Request content length changed.");
186190
}
187191

188192
[TestMethod]
@@ -197,7 +201,8 @@ public async Task AuthHandler_ShouldReturnUnauthorizedRequestWithDefaultMaxRetry
197201

198202
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
199203

200-
Assert.AreSame(response, expectedResponse, "Unexpected code returned");
204+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage, "Doesn't reissued a new http request.");
205+
Assert.AreSame(response, expectedResponse, "Unexpected code returned.");
201206
Assert.AreEqual(response.RequestMessage.Content.ReadAsStringAsync().Result, "Hello Mars!");
202207
}
203208
}

‎tests/Microsoft.Graph.Core.Test/Requests/GraphClientFactoryTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,10 @@ public async Task SendRequest_Retry()
206206
var response = await client.SendAsync(httpRequestMessage, new CancellationToken());
207207
Assert.AreSame(response, response_2);
208208
IEnumerable<string> values;
209-
Assert.IsTrue(httpRequestMessage.Headers.TryGetValues("Retry-Attempt", out values), "Don't set Retry-Attemp Header");
209+
Assert.IsTrue(response.RequestMessage.Headers.TryGetValues("Retry-Attempt", out values), "Don't set Retry-Attemp Header");
210210
Assert.AreEqual(values.Count(), 1);
211211
Assert.AreEqual(values.First(), 1.ToString());
212+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage);
212213
}
213214
}
214215

@@ -251,7 +252,7 @@ public async Task SendRequest_UnauthorizedWithAuthenticationProvider()
251252
{
252253
var response = await client.SendAsync(httpRequestMessage, new CancellationToken());
253254
Assert.AreSame(response, okResponse);
254-
Assert.AreSame(response.RequestMessage, httpRequestMessage);
255+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage);
255256
}
256257
}
257258

‎tests/Microsoft.Graph.Core.Test/Requests/RetryHandlerTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public async Task ShouldRetryWithAddRetryAttemptHeader(HttpStatusCode statusCode
9090
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
9191

9292
Assert.AreSame(response, response_2, "Retry failed.");
93-
Assert.AreSame(response.RequestMessage, httpRequestMessage, "The request is set wrong.");
93+
Assert.AreNotSame(response.RequestMessage, httpRequestMessage, "The request is set wrong.");
9494
Assert.IsNotNull(response.RequestMessage.Headers, "The request header is null");
9595
Assert.IsTrue(response.RequestMessage.Headers.Contains(RETRY_ATTEMPT), "Doesn't set Retry-Attemp header to request");
9696
IEnumerable<string> values;
@@ -227,7 +227,7 @@ public async Task ShouldRetrytBasedOnRetryAfter(HttpStatusCode statusCode)
227227

228228
Assert.AreSame(response, response_2);
229229
IEnumerable<string> values;
230-
Assert.IsTrue(httpRequestMessage.Headers.TryGetValues(RETRY_ATTEMPT, out values), "Don't set Retry-Attemp Header");
230+
Assert.IsTrue(response.RequestMessage.Headers.TryGetValues(RETRY_ATTEMPT, out values), "Don't set Retry-Attemp Header");
231231
Assert.AreEqual(values.Count(), 1);
232232
Assert.AreEqual(values.First(), 1.ToString());
233233

‎tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/AuthenticationHandlerTests.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public async Task AuthHandler_ShouldRetryUnauthorizedGetRequest()
8383
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
8484

8585
Assert.Same(response, expectedResponse);
86-
Assert.Same(response.RequestMessage, httpRequestMessage);
86+
Assert.NotSame(response.RequestMessage, httpRequestMessage);
8787
Assert.Null(response.RequestMessage.Content);
8888
}
8989

@@ -98,6 +98,7 @@ public async Task AuthHandler_ShouldRetryUnauthorizedPostRequestWithNoContent()
9898

9999
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
100100

101+
Assert.NotSame(response.RequestMessage, httpRequestMessage);
101102
Assert.Same(response, expectedResponse);
102103
Assert.NotSame(response, unauthorizedResponse);
103104
Assert.Null(response.RequestMessage.Content);
@@ -116,6 +117,7 @@ public async Task AuthHandler_ShouldRetryUnauthorizedPostRequestWithBufferConten
116117

117118
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
118119

120+
Assert.NotSame(response.RequestMessage, httpRequestMessage);
119121
Assert.Same(response, okResponse);
120122
Assert.NotSame(response, unauthorizedResponse);
121123
Assert.NotNull(response.RequestMessage.Content);
@@ -135,6 +137,7 @@ public async Task AuthHandler_ShouldRetryUnauthorizedPatchRequestWithBufferConte
135137

136138
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
137139

140+
Assert.NotSame(response.RequestMessage, httpRequestMessage);
138141
Assert.Same(response, okResponse);
139142
Assert.NotSame(response, unauthorizedResponse);
140143
Assert.NotNull(response.RequestMessage.Content);
@@ -155,6 +158,7 @@ public async Task AuthHandler_ShouldNotRetryUnauthorizedPutRequestWithStreamCont
155158

156159
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
157160

161+
Assert.Same(response.RequestMessage, httpRequestMessage);
158162
Assert.NotSame(response, okResponse);
159163
Assert.Same(response, unauthorizedResponse);
160164
Assert.NotNull(response.RequestMessage.Content);
@@ -175,6 +179,7 @@ public async Task AuthHandler_ShouldNotRetryUnauthorizedPatchRequestWithStreamCo
175179

176180
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
177181

182+
Assert.Same(response.RequestMessage, httpRequestMessage);
178183
Assert.NotSame(response, okResponse);
179184
Assert.Same(response, unauthorizedResponse);
180185
Assert.NotNull(response.RequestMessage.Content);
@@ -194,6 +199,7 @@ public async Task AuthHandler_ShouldReturnUnauthorizedRequestWithDefaultMaxRetry
194199

195200
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
196201

202+
Assert.NotSame(response.RequestMessage, httpRequestMessage);
197203
Assert.Same(response, expectedResponse);
198204
Assert.Equal(response.RequestMessage.Content.ReadAsStringAsync().Result, "Hello Mars!");
199205
}

0 commit comments

Comments
 (0)