From 0c33323352b7bdea5f611586aa79f1608e5b73b5 Mon Sep 17 00:00:00 2001 From: Marc Denman Date: Tue, 14 Mar 2017 09:15:19 +0000 Subject: [PATCH] Change HttpStatusCodeMapper not to wrap responses As part of #66 we realised that the implementation of IErrorToHttpStatusCodeMapper would always return a wrapped StatusCode within an OK response, in turn meaning that ResponderMiddleware would never fall into the else branch for returning a 500. This commit removes the wrapping of the status code and removes the unused logic for generating the 500 status code, giving the mapper full responsbility for generating the correct status code. --- .../Responder/ErrorsToHttpStatusCodeMapper.cs | 11 +++++------ .../IErrorsToHttpStatusCodeMapper.cs | 8 +++++--- .../Middleware/ResponderMiddleware.cs | 19 ++++++------------- .../ErrorsToHttpStatusCodeMapperTests.cs | 5 ++--- 4 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs index 45aeafd1..a654b406 100644 --- a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs @@ -1,17 +1,16 @@ using System.Collections.Generic; using System.Linq; using Ocelot.Errors; -using Ocelot.Responses; namespace Ocelot.Responder { public class ErrorsToHttpStatusCodeMapper : IErrorsToHttpStatusCodeMapper { - public Response Map(List errors) + public int Map(List errors) { if (errors.Any(e => e.Code == OcelotErrorCode.UnauthenticatedError)) { - return new OkResponse(401); + return 401; } if (errors.Any(e => e.Code == OcelotErrorCode.UnauthorizedError @@ -19,15 +18,15 @@ namespace Ocelot.Responder || e.Code == OcelotErrorCode.UserDoesNotHaveClaimError || e.Code == OcelotErrorCode.CannotFindClaimError)) { - return new OkResponse(403); + return 403; } if (errors.Any(e => e.Code == OcelotErrorCode.RequestTimedOutError)) { - return new OkResponse(503); + return 503; } - return new OkResponse(404); + return 404; } } } \ No newline at end of file diff --git a/src/Ocelot/Responder/IErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/IErrorsToHttpStatusCodeMapper.cs index b4b610d9..61330db9 100644 --- a/src/Ocelot/Responder/IErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/IErrorsToHttpStatusCodeMapper.cs @@ -1,11 +1,13 @@ using System.Collections.Generic; using Ocelot.Errors; -using Ocelot.Responses; namespace Ocelot.Responder -{ +{ + /// + /// Map a list OceoltErrors to a single appropriate HTTP status code + /// public interface IErrorsToHttpStatusCodeMapper { - Response Map(List errors); + int Map(List errors); } } diff --git a/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs b/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs index 6bce4ac6..7de06d96 100644 --- a/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs +++ b/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs @@ -16,12 +16,12 @@ namespace Ocelot.Responder.Middleware private readonly IErrorsToHttpStatusCodeMapper _codeMapper; private readonly IOcelotLogger _logger; - public ResponderMiddleware(RequestDelegate next, + public ResponderMiddleware(RequestDelegate next, IHttpResponder responder, IOcelotLoggerFactory loggerFactory, - IRequestScopedDataRepository requestScopedDataRepository, + IRequestScopedDataRepository requestScopedDataRepository, IErrorsToHttpStatusCodeMapper codeMapper) - :base(requestScopedDataRepository) + : base(requestScopedDataRepository) { _next = next; _responder = responder; @@ -58,16 +58,9 @@ namespace Ocelot.Responder.Middleware private void SetErrorResponse(HttpContext context, List errors) { - var statusCode = _codeMapper.Map(errors); - - if (!statusCode.IsError) - { - _responder.SetErrorResponseOnContext(context, statusCode.Data); - } - else - { - _responder.SetErrorResponseOnContext(context, 500); - } + var statusCode = _codeMapper.Map(errors); + + _responder.SetErrorResponseOnContext(context, statusCode); } } } \ No newline at end of file diff --git a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs index d2ac91e0..3b79715e 100644 --- a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs +++ b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs @@ -4,7 +4,6 @@ using Ocelot.Errors; using Ocelot.Middleware; using Ocelot.Requester; using Ocelot.Responder; -using Ocelot.Responses; using Shouldly; using TestStack.BDDfy; using Xunit; @@ -14,7 +13,7 @@ namespace Ocelot.UnitTests.Responder public class ErrorsToHttpStatusCodeMapperTests { private readonly IErrorsToHttpStatusCodeMapper _codeMapper; - private Response _result; + private int _result; private List _errors; public ErrorsToHttpStatusCodeMapperTests() @@ -77,7 +76,7 @@ namespace Ocelot.UnitTests.Responder private void ThenTheResponseIsStatusCodeIs(int expectedCode) { - _result.Data.ShouldBe(expectedCode); + _result.ShouldBe(expectedCode); } } }