Skip to content

Commit f0df15b

Browse files
author
Bart Koelman
committed
More assertions on non-success status codes and exceptions. Removed empty string check from CurrentRequestMiddleware because I found now way to get there. The unit-test was misleading: throwing JsonApiException from CurrentRequestMiddleware does not hit any handler, so it always results in HTTP 500 without a body. Better not throw from there.
1 parent d92b3b9 commit f0df15b

File tree

13 files changed

+211
-95
lines changed

13 files changed

+211
-95
lines changed

‎src/Examples/JsonApiDotNetCoreExample/Resources/LockableResource.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ protected void DisallowLocked(IEnumerable<T> entities)
2121
{
2222
throw new JsonApiException(new Error(HttpStatusCode.Forbidden)
2323
{
24-
Title = "You are not allowed to update fields or relations of locked todo items."
24+
Title = "You are not allowed to update fields or relationships of locked todo items."
2525
});
2626
}
2727
}

‎src/Examples/JsonApiDotNetCoreExample/Resources/PassportResource.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ private void DoesNotTouchLockedPassports(IEnumerable<Passport> entities)
4242
{
4343
throw new JsonApiException(new Error(HttpStatusCode.Forbidden)
4444
{
45-
Title = "You are not allowed to update fields or relations of locked persons."
45+
Title = "You are not allowed to update fields or relationships of locked persons."
4646
});
4747
}
4848
}

‎src/JsonApiDotNetCore/Middleware/CurrentRequestMiddleware.cs‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.IO;
23
using System.Linq;
34
using System.Net;
45
using System.Threading.Tasks;
@@ -7,9 +8,11 @@
78
using JsonApiDotNetCore.Internal;
89
using JsonApiDotNetCore.Internal.Contracts;
910
using JsonApiDotNetCore.Managers.Contracts;
11+
using JsonApiDotNetCore.Models.JsonApiDocuments;
1012
using Microsoft.AspNetCore.Http;
1113
using Microsoft.AspNetCore.Routing;
1214
using Microsoft.Extensions.Primitives;
15+
using Newtonsoft.Json;
1316

1417
namespace JsonApiDotNetCore.Middleware
1518
{
@@ -53,7 +56,7 @@ public async Task Invoke(HttpContext httpContext,
5356
_currentRequest.RelationshipId = GetRelationshipId();
5457
}
5558

56-
if (IsValid())
59+
if (await IsValidAsync())
5760
{
5861
await _next(httpContext);
5962
}
@@ -63,16 +66,10 @@ private string GetBaseId()
6366
{
6467
if (_routeValues.TryGetValue("id", out object stringId))
6568
{
66-
if ((string)stringId == string.Empty)
67-
{
68-
throw new JsonApiException(HttpStatusCode.BadRequest, "No empty string as id please.");
69-
}
7069
return (string)stringId;
7170
}
72-
else
73-
{
74-
return null;
75-
}
71+
72+
return null;
7673
}
7774
private string GetRelationshipId()
7875
{
@@ -140,23 +137,28 @@ private bool PathIsRelationship()
140137
return actionName.ToLower().Contains("relationships");
141138
}
142139

143-
private bool IsValid()
140+
private async Task<bool> IsValidAsync()
144141
{
145-
return IsValidContentTypeHeader(_httpContext) && IsValidAcceptHeader(_httpContext);
142+
return await IsValidContentTypeHeaderAsync(_httpContext) && await IsValidAcceptHeaderAsync(_httpContext);
146143
}
147144

148-
private bool IsValidContentTypeHeader(HttpContext context)
145+
private static async Task<bool> IsValidContentTypeHeaderAsync(HttpContext context)
149146
{
150147
var contentType = context.Request.ContentType;
151148
if (contentType != null && ContainsMediaTypeParameters(contentType))
152149
{
153-
FlushResponse(context, HttpStatusCode.UnsupportedMediaType);
150+
await FlushResponseAsync(context, new Error(HttpStatusCode.UnsupportedMediaType)
151+
{
152+
Title = "The specified Content-Type header value is not supported.",
153+
Detail = $"Please specify '{Constants.ContentType}' for the Content-Type header value."
154+
});
155+
154156
return false;
155157
}
156158
return true;
157159
}
158160

159-
private bool IsValidAcceptHeader(HttpContext context)
161+
private static async Task<bool> IsValidAcceptHeaderAsync(HttpContext context)
160162
{
161163
if (context.Request.Headers.TryGetValue(Constants.AcceptHeader, out StringValues acceptHeaders) == false)
162164
return true;
@@ -168,7 +170,11 @@ private bool IsValidAcceptHeader(HttpContext context)
168170
continue;
169171
}
170172

171-
FlushResponse(context, HttpStatusCode.NotAcceptable);
173+
await FlushResponseAsync(context, new Error(HttpStatusCode.NotAcceptable)
174+
{
175+
Title = "The specified Accept header value is not supported.",
176+
Detail = $"Please specify '{Constants.ContentType}' for the Accept header value."
177+
});
172178
return false;
173179
}
174180
return true;
@@ -195,9 +201,16 @@ private static bool ContainsMediaTypeParameters(string mediaType)
195201
);
196202
}
197203

198-
private void FlushResponse(HttpContext context, HttpStatusCode statusCode)
204+
private static async Task FlushResponseAsync(HttpContext context, Error error)
199205
{
200-
context.Response.StatusCode = (int)statusCode;
206+
context.Response.StatusCode = (int) error.StatusCode;
207+
208+
string responseBody = JsonConvert.SerializeObject(new ErrorDocument(error));
209+
await using (var writer = new StreamWriter(context.Response.Body))
210+
{
211+
await writer.WriteAsync(responseBody);
212+
}
213+
201214
context.Response.Body.Flush();
202215
}
203216

‎test/JsonApiDotNetCoreExampleTests/Acceptance/Extensibility/OmitAttributeIfValueIsNullTests.cs‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Threading.Tasks;
55
using JsonApiDotNetCore.Configuration;
66
using JsonApiDotNetCore.Models;
7+
using JsonApiDotNetCore.Models.JsonApiDocuments;
78
using JsonApiDotNetCoreExample;
89
using JsonApiDotNetCoreExample.Data;
910
using JsonApiDotNetCoreExample.Models;
@@ -90,21 +91,50 @@ public async Task CheckNullBehaviorCombination(bool? omitAttributeIfValueIsNull,
9091
// Act
9192
var response = await _fixture.Client.SendAsync(request);
9293
var body = await response.Content.ReadAsStringAsync();
93-
var deserializeBody = JsonConvert.DeserializeObject<Document>(body);
9494

95-
if (queryString.Length > 0 && !bool.TryParse(queryStringOverride, out _))
95+
var isQueryStringMissing = queryString.Length > 0 && queryStringOverride == null;
96+
var isQueryStringInvalid = queryString.Length > 0 && queryStringOverride != null && !bool.TryParse(queryStringOverride, out _);
97+
var isDisallowedOverride = allowQueryStringOverride == false && queryStringOverride != null;
98+
99+
if (isDisallowedOverride)
96100
{
97101
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
102+
103+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
104+
Assert.Single(errorDocument.Errors);
105+
Assert.Equal(HttpStatusCode.BadRequest, errorDocument.Errors[0].StatusCode);
106+
Assert.Equal("Usage of one or more query string parameters is not allowed at the requested endpoint.", errorDocument.Errors[0].Title);
107+
Assert.Equal("The parameter 'omitNull' cannot be used at this endpoint.", errorDocument.Errors[0].Detail);
108+
Assert.Equal("omitNull", errorDocument.Errors[0].Source.Parameter);
98109
}
99-
else if (allowQueryStringOverride == false && queryStringOverride != null)
110+
else if (isQueryStringMissing)
100111
{
101112
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
113+
114+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
115+
Assert.Single(errorDocument.Errors);
116+
Assert.Equal(HttpStatusCode.BadRequest, errorDocument.Errors[0].StatusCode);
117+
Assert.Equal("Missing query string parameter value.", errorDocument.Errors[0].Title);
118+
Assert.Equal("Missing value for 'omitNull' query string parameter.", errorDocument.Errors[0].Detail);
119+
Assert.Equal("omitNull", errorDocument.Errors[0].Source.Parameter);
120+
}
121+
else if (isQueryStringInvalid)
122+
{
123+
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
124+
125+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
126+
Assert.Single(errorDocument.Errors);
127+
Assert.Equal(HttpStatusCode.BadRequest, errorDocument.Errors[0].StatusCode);
128+
Assert.Equal("The specified query string value must be 'true' or 'false'.", errorDocument.Errors[0].Title);
129+
Assert.Equal("The value 'this-is-not-a-boolean-value' for parameter 'omitNull' is not a valid boolean value.", errorDocument.Errors[0].Detail);
130+
Assert.Equal("omitNull", errorDocument.Errors[0].Source.Parameter);
102131
}
103132
else
104133
{
105134
// Assert: does response contain a null valued attribute?
106135
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
107136

137+
var deserializeBody = JsonConvert.DeserializeObject<Document>(body);
108138
Assert.Equal(expectNullsMissing, !deserializeBody.SingleData.Attributes.ContainsKey("description"));
109139
Assert.Equal(expectNullsMissing, !deserializeBody.Included[0].Attributes.ContainsKey("lastName"));
110140
}

‎test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs‎

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Threading.Tasks;
77
using Bogus;
88
using JsonApiDotNetCore.Models;
9+
using JsonApiDotNetCore.Models.JsonApiDocuments;
910
using JsonApiDotNetCoreExample;
1011
using JsonApiDotNetCoreExample.Data;
1112
using JsonApiDotNetCoreExample.Models;
@@ -149,7 +150,13 @@ public async Task Unauthorized_TodoItem()
149150

150151
// Assert
151152
var body = await response.Content.ReadAsStringAsync();
152-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
153+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
154+
155+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
156+
Assert.Single(errorDocument.Errors);
157+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
158+
Assert.Equal("You are not allowed to update the author of todo items.", errorDocument.Errors[0].Title);
159+
Assert.Null(errorDocument.Errors[0].Detail);
153160
}
154161

155162
[Fact]
@@ -163,7 +170,13 @@ public async Task Unauthorized_Passport()
163170

164171
// Assert
165172
var body = await response.Content.ReadAsStringAsync();
166-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
173+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
174+
175+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
176+
Assert.Single(errorDocument.Errors);
177+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
178+
Assert.Equal("You are not allowed to include passports on individual persons.", errorDocument.Errors[0].Title);
179+
Assert.Null(errorDocument.Errors[0].Detail);
167180
}
168181

169182
[Fact]
@@ -185,8 +198,13 @@ public async Task Unauthorized_Article()
185198

186199
// Assert
187200
var body = await response.Content.ReadAsStringAsync();
188-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
201+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
189202

203+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
204+
Assert.Single(errorDocument.Errors);
205+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
206+
Assert.Equal("You are not allowed to see this article.", errorDocument.Errors[0].Title);
207+
Assert.Null(errorDocument.Errors[0].Detail);
190208
}
191209

192210
[Fact]
@@ -300,10 +318,14 @@ public async Task Cascade_Permission_Error_Create_ToOne_Relationship()
300318

301319
// Assert
302320
var body = await response.Content.ReadAsStringAsync();
303-
// should throw 403 in PersonResource implicit hook
304-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
305-
}
321+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
306322

323+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
324+
Assert.Single(errorDocument.Errors);
325+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
326+
Assert.Equal("You are not allowed to update fields or relationships of locked todo items.", errorDocument.Errors[0].Title);
327+
Assert.Null(errorDocument.Errors[0].Detail);
328+
}
307329

308330
[Fact]
309331
public async Task Cascade_Permission_Error_Updating_ToOne_Relationship()
@@ -348,8 +370,13 @@ public async Task Cascade_Permission_Error_Updating_ToOne_Relationship()
348370

349371
// Assert
350372
var body = await response.Content.ReadAsStringAsync();
351-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
373+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
352374

375+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
376+
Assert.Single(errorDocument.Errors);
377+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
378+
Assert.Equal("You are not allowed to update fields or relationships of locked persons.", errorDocument.Errors[0].Title);
379+
Assert.Null(errorDocument.Errors[0].Detail);
353380
}
354381

355382
[Fact]
@@ -395,12 +422,15 @@ public async Task Cascade_Permission_Error_Updating_ToOne_Relationship_Deletion(
395422

396423
// Assert
397424
var body = await response.Content.ReadAsStringAsync();
398-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
425+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
399426

427+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
428+
Assert.Single(errorDocument.Errors);
429+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
430+
Assert.Equal("You are not allowed to update fields or relationships of locked persons.", errorDocument.Errors[0].Title);
431+
Assert.Null(errorDocument.Errors[0].Detail);
400432
}
401433

402-
403-
404434
[Fact]
405435
public async Task Cascade_Permission_Error_Delete_ToOne_Relationship()
406436
{
@@ -422,10 +452,14 @@ public async Task Cascade_Permission_Error_Delete_ToOne_Relationship()
422452

423453
// Assert
424454
var body = await response.Content.ReadAsStringAsync();
425-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
426-
}
427-
455+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
428456

457+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
458+
Assert.Single(errorDocument.Errors);
459+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
460+
Assert.Equal("You are not allowed to update fields or relationships of locked todo items.", errorDocument.Errors[0].Title);
461+
Assert.Null(errorDocument.Errors[0].Detail);
462+
}
429463

430464
[Fact]
431465
public async Task Cascade_Permission_Error_Create_ToMany_Relationship()
@@ -473,7 +507,13 @@ public async Task Cascade_Permission_Error_Create_ToMany_Relationship()
473507

474508
// Assert
475509
var body = await response.Content.ReadAsStringAsync();
476-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
510+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
511+
512+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
513+
Assert.Single(errorDocument.Errors);
514+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
515+
Assert.Equal("You are not allowed to update fields or relationships of locked todo items.", errorDocument.Errors[0].Title);
516+
Assert.Null(errorDocument.Errors[0].Detail);
477517
}
478518

479519
[Fact]
@@ -525,10 +565,13 @@ public async Task Cascade_Permission_Error_Updating_ToMany_Relationship()
525565

526566
// Assert
527567
var body = await response.Content.ReadAsStringAsync();
568+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
528569

529-
// were unrelating a persons from a locked todo, so this should be unauthorized
530-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
531-
570+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
571+
Assert.Single(errorDocument.Errors);
572+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
573+
Assert.Equal("You are not allowed to update fields or relationships of locked todo items.", errorDocument.Errors[0].Title);
574+
Assert.Null(errorDocument.Errors[0].Detail);
532575
}
533576

534577
[Fact]
@@ -552,7 +595,13 @@ public async Task Cascade_Permission_Error_Delete_ToMany_Relationship()
552595

553596
// Assert
554597
var body = await response.Content.ReadAsStringAsync();
555-
Assert.True(HttpStatusCode.Forbidden == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
598+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
599+
600+
var errorDocument = JsonConvert.DeserializeObject<ErrorDocument>(body);
601+
Assert.Single(errorDocument.Errors);
602+
Assert.Equal(HttpStatusCode.Forbidden, errorDocument.Errors[0].StatusCode);
603+
Assert.Equal("You are not allowed to update fields or relationships of locked todo items.", errorDocument.Errors[0].Title);
604+
Assert.Null(errorDocument.Errors[0].Detail);
556605
}
557606
}
558607
}

0 commit comments

Comments
 (0)