Skip to content

Commit e812738

Browse files
iwatemikepizzo
iwate
and
mikepizzo
authored
Enhance batch accept header decision (#2231)
* add test cases * modify CreateODataBatchResponseAsync logic * fix typo * Added handling for default q-value of 1. * Added logic and tests to AspNet classic. Co-authored-by: mikepizzo <[email protected]>
1 parent ebe2a09 commit e812738

File tree

4 files changed

+202
-25
lines changed

4 files changed

+202
-25
lines changed

‎src/Microsoft.AspNet.OData/Batch/ODataBatchHttpRequestMessageExtensions.cs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -198,29 +198,33 @@ internal static Task<HttpResponseMessage> CreateODataBatchResponseAsync(this Htt
198198
HttpHeaderValueCollection<MediaTypeWithQualityHeaderValue> acceptHeaderValues = request.Headers.Accept;
199199

200200
MediaTypeHeaderValue responseContentType = null;
201-
if (!acceptHeaderValues.Any())
201+
foreach (MediaTypeWithQualityHeaderValue acceptHeader in acceptHeaderValues.OrderByDescending(h => h.Quality == null ? 1 : h.Quality))
202+
{
203+
if (acceptHeader.MediaType.Equals(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeMime, StringComparison.OrdinalIgnoreCase))
204+
{
205+
responseContentType = MediaTypeHeaderValue.Parse(
206+
String.Format(CultureInfo.InvariantCulture, "multipart/mixed;boundary=batchresponse_{0}",
207+
Guid.NewGuid()));
208+
break;
209+
}
210+
else if (acceptHeader.MediaType.Equals(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeJson, StringComparison.OrdinalIgnoreCase))
211+
{
212+
responseContentType = MediaTypeHeaderValue.Parse(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeJson);
213+
break;
214+
}
215+
}
216+
217+
if (responseContentType == null)
202218
{
203219
// In absence of accept, if request was JSON then default response to be JSON.
204220
// Note that, if responseContentType is not set, then it will default to multipart/mixed
205221
// when constructing the BatchContent, so we don't need to handle that case here
206-
if (request.Content != null && request.Content.Headers.Any(h=>String.Equals(h.Key, ODataBatchHttpRequestMessageExtensions.ContentType, StringComparison.OrdinalIgnoreCase)
207-
&& h.Value.Any(v=>v.IndexOf(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeJson, StringComparison.OrdinalIgnoreCase) > -1)))
222+
if (request.Content != null && request.Content.Headers.Any(h => String.Equals(h.Key, ODataBatchHttpRequestMessageExtensions.ContentType, StringComparison.OrdinalIgnoreCase)
223+
&& h.Value.Any(v => v.IndexOf(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeJson, StringComparison.OrdinalIgnoreCase) > -1)))
208224
{
209225
responseContentType = MediaTypeHeaderValue.Parse(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeJson);
210226
}
211227
}
212-
else if (acceptHeaderValues.Any(
213-
t => t.MediaType.Equals(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeMime, StringComparison.OrdinalIgnoreCase)))
214-
{
215-
responseContentType = MediaTypeHeaderValue.Parse(
216-
String.Format(CultureInfo.InvariantCulture, "multipart/mixed;boundary=batchresponse_{0}",
217-
Guid.NewGuid()));
218-
}
219-
else if (acceptHeaderValues.Any(
220-
t => t.MediaType.Equals(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeJson, StringComparison.OrdinalIgnoreCase)))
221-
{
222-
responseContentType = MediaTypeHeaderValue.Parse(ODataBatchHttpRequestMessageExtensions.BatchMediaTypeJson);
223-
}
224228

225229
HttpResponseMessage response = request.CreateResponse(HttpStatusCode.OK);
226230
response.Content = new ODataBatchContent(responses, requestContainer, responseContentType);

‎src/Microsoft.AspNetCore.OData/Batch/ODataBatchHttpRequestMessageExtensions.cs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,23 @@ internal static Task CreateODataBatchResponseAsync(this HttpRequest request, IEn
195195

196196
HttpResponse response = request.HttpContext.Response;
197197

198-
StringValues acceptHeader = request.Headers["Accept"];
198+
IEnumerable<MediaTypeHeaderValue> acceptHeaders = MediaTypeHeaderValue.ParseList(request.Headers.GetCommaSeparatedValues("Accept"));
199199
string responseContentType = null;
200-
if (StringValues.IsNullOrEmpty(acceptHeader))
200+
201+
foreach (MediaTypeHeaderValue acceptHeader in acceptHeaders.OrderByDescending(h => h.Quality == null ? 1 : h.Quality))
202+
{
203+
if (acceptHeader.MediaType.Equals(ODataBatchHttpRequestExtensions.BatchMediaTypeMime, StringComparison.OrdinalIgnoreCase))
204+
{
205+
responseContentType = String.Format(CultureInfo.InvariantCulture, "multipart/mixed;boundary=batchresponse_{0}", Guid.NewGuid());
206+
break;
207+
}
208+
else if (acceptHeader.MediaType.Equals(ODataBatchHttpRequestExtensions.BatchMediaTypeJson, StringComparison.OrdinalIgnoreCase))
209+
{
210+
responseContentType = ODataBatchHttpRequestExtensions.BatchMediaTypeJson;
211+
break;
212+
}
213+
}
214+
if (responseContentType == null)
201215
{
202216
// In absence of accept, if request was JSON then default response to be JSON.
203217
// Note that, if responseContentType is not set, then it will default to multipart/mixed
@@ -208,14 +222,6 @@ internal static Task CreateODataBatchResponseAsync(this HttpRequest request, IEn
208222
responseContentType = ODataBatchHttpRequestExtensions.BatchMediaTypeJson;
209223
}
210224
}
211-
else if (acceptHeader.Any(h => h.Equals(ODataBatchHttpRequestExtensions.BatchMediaTypeMime, StringComparison.OrdinalIgnoreCase)))
212-
{
213-
responseContentType = String.Format(CultureInfo.InvariantCulture, "multipart/mixed;boundary=batchresponse_{0}", Guid.NewGuid());
214-
}
215-
else if (acceptHeader.Any(h => h.IndexOf(ODataBatchHttpRequestExtensions.BatchMediaTypeJson, StringComparison.OrdinalIgnoreCase) > -1))
216-
{
217-
responseContentType = ODataBatchHttpRequestExtensions.BatchMediaTypeJson;
218-
}
219225

220226
response.StatusCode = (int)HttpStatusCode.OK;
221227
ODataBatchContent batchContent = new ODataBatchContent(responses, requestContainer, responseContentType);

‎test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Batch/ODataBatchReaderExtensionsTest.cs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33

44
#if !NETCORE // TODO #939: Enable these test on AspNetCore.
55
using System;
6+
using System.Linq;
67
using System.Net.Http;
78
using System.Net.Http.Headers;
89
using System.Text;
910
using System.Threading;
1011
using System.Threading.Tasks;
1112
using Microsoft.AspNet.OData.Batch;
13+
using Microsoft.AspNet.OData.Test.Abstraction;
1214
using Microsoft.AspNet.OData.Test.Common;
1315
using Microsoft.OData;
1416
using Xunit;
@@ -76,6 +78,81 @@ public async Task ReadChangeSetOperationRequest_InvalidState_Throws()
7678
Guid.NewGuid(), Guid.NewGuid(), false, CancellationToken.None),
7779
"The current batch reader state 'Initial' is invalid. The expected state is 'Operation'.");
7880
}
81+
82+
private static ODataMessageQuotas _odataMessageQuotas = new ODataMessageQuotas { MaxReceivedMessageSize = Int64.MaxValue };
83+
84+
[Theory]
85+
// if no accept header, return multipart/mixed
86+
[InlineData(null, "multipart/mixed")]
87+
88+
// if accept is multipart/mixed, return multipart/mixed
89+
[InlineData(new[] { "multipart/mixed" }, "multipart/mixed")]
90+
91+
// if accept is application/json, return application/json
92+
[InlineData(new[] { "application/json" }, "application/json")]
93+
94+
// if accept is application/json with charset, return application/json
95+
[InlineData(new[] { "application/json; charset=utf-8" }, "application/json")]
96+
97+
// if multipart/mixed is high proprity, return multipart/mixed
98+
[InlineData(new[] { "multipart/mixed;q=0.9", "application/json;q=0.5" }, "multipart/mixed")]
99+
[InlineData(new[] { "application/json;q=0.5", "multipart/mixed;q=0.9" }, "multipart/mixed")]
100+
101+
// if application/json is high proprity, return application/json
102+
[InlineData(new[] { "application/json;q=0.9", "multipart/mixed;q=0.5" }, "application/json")]
103+
[InlineData(new[] { "multipart/mixed;q=0.5", "application/json;q=0.9" }, "application/json")]
104+
105+
// if priorities are same, return first
106+
[InlineData(new[] { "multipart/mixed;q=0.9", "application/json;q=0.9" }, "multipart/mixed")]
107+
[InlineData(new[] { "multipart/mixed", "application/json" }, "multipart/mixed")]
108+
109+
// if priorities are same, return first
110+
[InlineData(new[] { "application/json;q=0.9", "multipart/mixed;q=0.9" }, "application/json")]
111+
[InlineData(new[] { "application/json", "multipart/mixed" }, "application/json")]
112+
113+
// no priority has q=1.0
114+
[InlineData(new[] { "application/json", "multipart/mixed;q=0.9" }, "application/json")]
115+
[InlineData(new[] { "application/json;q=0.9", "multipart/mixed" }, "multipart/mixed")]
116+
117+
public async Task CreateODataBatchResponseAsync(string[] accept, string expected)
118+
{
119+
var request = RequestFactory.Create(HttpMethod.Get, "http://localhost/$batch");
120+
var responses = new[] { new ChangeSetResponseItem(Enumerable.Empty<HttpResponseMessage>()) };
121+
122+
if (accept != null)
123+
{
124+
request.Headers.Add("Accept", accept);
125+
}
126+
127+
var response = await request.CreateODataBatchResponseAsync(responses, _odataMessageQuotas);
128+
129+
Assert.StartsWith(expected, response.Content.Headers.ContentType.MediaType);
130+
}
131+
132+
[Theory]
133+
// if no contentType, return multipart/mixed
134+
[InlineData(null, "multipart/mixed")]
135+
// if contentType is application/json, return application/json
136+
[InlineData("application/json", "application/json")]
137+
[InlineData("application/json; charset=utf-8", "application/json")]
138+
// if contentType is multipart/mixed, return multipart/mixed
139+
[InlineData("multipart/mixed", "multipart/mixed")]
140+
public async Task CreateODataBatchResponseAsyncWhenNoAcceptHeader(string contentType, string expected)
141+
{
142+
var request = RequestFactory.Create(HttpMethod.Get, "http://localhost/$batch");
143+
var responses = new[] { new ChangeSetResponseItem(Enumerable.Empty<HttpResponseMessage>()) };
144+
145+
if (contentType != null)
146+
{
147+
request.Content = new ByteArrayContent(new byte[] { });
148+
request.Content.Headers.Add("Content-Type", contentType);
149+
}
150+
151+
var response = await request.CreateODataBatchResponseAsync(responses, _odataMessageQuotas);
152+
153+
Assert.False(request.Headers.Contains("Accept")); // check no accept header
154+
Assert.StartsWith(expected, response.Content.Headers.ContentType.MediaType);
155+
}
79156
}
80157
}
81158
#endif

‎test/UnitTest/Microsoft.AspNetCore.OData.Test/Batch/ODataBatchHttpRequestMessageExtensionsTest.cs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using System;
2+
using System.Linq;
3+
using System.Net;
4+
using System.Net.Http;
5+
using System.Threading.Tasks;
6+
using Microsoft.AspNet.OData.Batch;
7+
using Microsoft.AspNet.OData.Extensions;
8+
using Microsoft.AspNet.OData.Test.Abstraction;
9+
using Microsoft.AspNetCore.Http;
10+
using Microsoft.OData;
11+
using Xunit;
12+
13+
namespace Microsoft.AspNet.OData.Test.Batch
14+
{
15+
public class ODataBatchHttpRequestMessageExtensionsTest
16+
{
17+
private static ODataMessageQuotas _odataMessageQuotas = new ODataMessageQuotas { MaxReceivedMessageSize = Int64.MaxValue };
18+
19+
[Theory]
20+
// if no accept header, return multipart/mixed
21+
[InlineData(null, "multipart/mixed")]
22+
23+
// if accept is multipart/mixed, return multipart/mixed
24+
[InlineData(new[] { "multipart/mixed" }, "multipart/mixed")]
25+
26+
// if accept is application/json, return application/json
27+
[InlineData(new[] { "application/json" }, "application/json")]
28+
29+
// if accept is application/json with charset, return application/json
30+
[InlineData(new[] { "application/json; charset=utf-8" }, "application/json")]
31+
32+
// if multipart/mixed is high proprity, return multipart/mixed
33+
[InlineData(new[] { "multipart/mixed;q=0.9", "application/json;q=0.5" }, "multipart/mixed")]
34+
[InlineData(new[] { "application/json;q=0.5", "multipart/mixed;q=0.9" }, "multipart/mixed")]
35+
36+
// if application/json is high proprity, return application/json
37+
[InlineData(new[] { "application/json;q=0.9", "multipart/mixed;q=0.5" }, "application/json")]
38+
[InlineData(new[] { "multipart/mixed;q=0.5", "application/json;q=0.9" }, "application/json")]
39+
40+
// if priorities are same, return first
41+
[InlineData(new[] { "multipart/mixed;q=0.9", "application/json;q=0.9" }, "multipart/mixed")]
42+
[InlineData(new[] { "multipart/mixed", "application/json" }, "multipart/mixed")]
43+
44+
// if priorities are same, return first
45+
[InlineData(new[] { "application/json;q=0.9", "multipart/mixed;q=0.9" }, "application/json")]
46+
[InlineData(new[] { "application/json", "multipart/mixed" }, "application/json")]
47+
48+
// no priority has q=1.0
49+
[InlineData(new[] { "application/json", "multipart/mixed;q=0.9" }, "application/json")]
50+
[InlineData(new[] { "application/json;q=0.9", "multipart/mixed" }, "multipart/mixed")]
51+
public async Task CreateODataBatchResponseAsync(string[] accept, string expected)
52+
{
53+
var request = RequestFactory.Create(HttpMethod.Get, "http://localhost/$batch");
54+
var responses = new[] { new ChangeSetResponseItem(Enumerable.Empty<HttpContext>()) };
55+
56+
if (accept != null)
57+
{
58+
request.Headers.Add("Accept", accept);
59+
}
60+
61+
await request.CreateODataBatchResponseAsync(responses, _odataMessageQuotas);
62+
63+
Assert.StartsWith(expected, request.HttpContext.Response.ContentType);
64+
}
65+
66+
[Theory]
67+
// if no contentType, return multipart/mixed
68+
[InlineData(null, "multipart/mixed")]
69+
// if contentType is application/json, return application/json
70+
[InlineData("application/json", "application/json")]
71+
[InlineData("application/json; charset=utf-8", "application/json")]
72+
// if contentType is multipart/mixed, return multipart/mixed
73+
[InlineData("multipart/mixed", "multipart/mixed")]
74+
public async Task CreateODataBatchResponseAsyncWhenNoAcceptHeader(string contentType, string expected)
75+
{
76+
var request = RequestFactory.Create(HttpMethod.Get, "http://localhost/$batch");
77+
var responses = new[] { new ChangeSetResponseItem(Enumerable.Empty<HttpContext>()) };
78+
79+
if (contentType != null)
80+
{
81+
request.ContentType = contentType;
82+
}
83+
84+
await request.CreateODataBatchResponseAsync(responses, _odataMessageQuotas);
85+
86+
Assert.False(request.Headers.ContainsKey("Accept")); // check no accept header
87+
Assert.StartsWith(expected, request.HttpContext.Response.ContentType);
88+
}
89+
}
90+
}

0 commit comments

Comments
 (0)