From 027bf6867a0a4c630a038832a9878d58c4d3d748 Mon Sep 17 00:00:00 2001 From: Dilsy99 <38723855+Dilsy99@users.noreply.github.com> Date: Wed, 25 Apr 2018 16:46:41 +0100 Subject: [PATCH] #330 Fix and issue with the httpclient being cached on the url but not taking the http verb into accont. This caused an issue because if the same url but different verbs had handlers, the incorrect handler would be called (#331) --- src/Ocelot/Requester/HttpClientBuilder.cs | 6 ++- .../Requester/HttpClientBuilderTests.cs | 39 ++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/Ocelot/Requester/HttpClientBuilder.cs b/src/Ocelot/Requester/HttpClientBuilder.cs index fdc90ebf..6d3ffcb0 100644 --- a/src/Ocelot/Requester/HttpClientBuilder.cs +++ b/src/Ocelot/Requester/HttpClientBuilder.cs @@ -97,7 +97,11 @@ namespace Ocelot.Requester private string GetCacheKey(DownstreamContext request) { - return request.DownstreamRequest.OriginalString; + var cacheKey = $"{request.DownstreamRequest.Method}:{request.DownstreamRequest.OriginalString}"; + + this._logger.LogDebug($"Cache key for request is {cacheKey}"); + + return cacheKey; } } } diff --git a/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs b/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs index d4bc994c..e4227580 100644 --- a/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs +++ b/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs @@ -130,6 +130,38 @@ namespace Ocelot.UnitTests.Requester .BDDfy(); } + [Theory] + [InlineData("GET")] + [InlineData("POST")] + [InlineData("PUT")] + [InlineData("DELETE")] + [InlineData("PATCH")] + public void should_add_verb_to_cache_key(string verb) + { + var client = "http://localhost:5012"; + + HttpMethod method = new HttpMethod(verb); + + var reRoute = new DownstreamReRouteBuilder() + .WithIsQos(false) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false)) + .WithReRouteKey("") + .WithQosOptions(new QoSOptionsBuilder().Build()) + .Build(); + + this.Given(_ => GivenADownstreamService()) + .And(_ => GivenARequestWithAUrlAndMethod(reRoute, client, method)) + .And(_ => GivenTheFactoryReturnsNothing()) + .And(_ => WhenIBuild()) + .And(_ => GivenCacheIsCalledWithExpectedKey($"{method.ToString()}:{client}")) + .BDDfy(); + } + + private void GivenCacheIsCalledWithExpectedKey(string expectedKey) + { + this._cacheHandlers.Verify(x => x.Get(It.Is(p => p.Equals(expectedKey, StringComparison.OrdinalIgnoreCase))), Times.Once); + } + private void ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged() { _logger.Verify(x => x.LogWarning($"You have ignored all SSL warnings by using DangerousAcceptAnyServerCertificateValidator for this DownstreamReRoute, UpstreamPathTemplate: {_context.DownstreamReRoute.UpstreamPathTemplate}, DownstreamPathTemplate: {_context.DownstreamReRoute.DownstreamPathTemplate}"), Times.Once); @@ -197,11 +229,16 @@ namespace Ocelot.UnitTests.Requester } private void GivenARequest(DownstreamReRoute downstream) + { + GivenARequestWithAUrlAndMethod(downstream, "http://localhost:5003", HttpMethod.Get); + } + + private void GivenARequestWithAUrlAndMethod(DownstreamReRoute downstream, string url, HttpMethod method) { var context = new DownstreamContext(new DefaultHttpContext()) { DownstreamReRoute = downstream, - DownstreamRequest = new DownstreamRequest(new HttpRequestMessage() { RequestUri = new Uri("http://localhost:5003") }), + DownstreamRequest = new DownstreamRequest(new HttpRequestMessage() { RequestUri = new Uri(url), Method = method }), }; _context = context;