From f4c21d08f0b40f3bbda0aaeb28d41897e38c909e Mon Sep 17 00:00:00 2001 From: Chris R Date: Tue, 15 Dec 2020 13:01:15 -0800 Subject: [PATCH] Merge conflicts, cleanup, naming --- samples/ReverseProxy.Config.Sample/Startup.cs | 2 +- .../ReverseProxyConventionBuilder.cs | 11 ++++++---- .../Service/Management/ProxyConfigManager.cs | 20 +++++++------------ .../ReverseProxyConventionBuilderTests.cs | 6 +++--- .../Middleware/LoadBalancerMiddlewareTests.cs | 2 +- .../Proxy/LoadBalancingPoliciesTests.cs | 2 +- 6 files changed, 20 insertions(+), 23 deletions(-) diff --git a/samples/ReverseProxy.Config.Sample/Startup.cs b/samples/ReverseProxy.Config.Sample/Startup.cs index f2d8d6de3..99776fc58 100644 --- a/samples/ReverseProxy.Config.Sample/Startup.cs +++ b/samples/ReverseProxy.Config.Sample/Startup.cs @@ -66,7 +66,7 @@ public void Configure(IApplicationBuilder app) proxyPipeline.UseRequestAffinitizer(); proxyPipeline.UsePassiveHealthChecks(); }) - .Configure((builder, route) => builder.WithDisplayName($"ReverseProxy {route.RouteId}-{route.ClusterId}")); + .ConfigureEndpoints((builder, route) => builder.WithDisplayName($"ReverseProxy {route.RouteId}-{route.ClusterId}")); }); } } diff --git a/src/ReverseProxy/Service/DynamicEndpoint/ReverseProxyConventionBuilder.cs b/src/ReverseProxy/Service/DynamicEndpoint/ReverseProxyConventionBuilder.cs index 30b6b33d0..a7d10f88b 100644 --- a/src/ReverseProxy/Service/DynamicEndpoint/ReverseProxyConventionBuilder.cs +++ b/src/ReverseProxy/Service/DynamicEndpoint/ReverseProxyConventionBuilder.cs @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + using System; using System.Collections.Generic; using System.Linq; @@ -32,7 +35,7 @@ public void Add(Action convention) /// /// The convention to add to the builder. /// - public ReverseProxyConventionBuilder Configure(Action convention) + public ReverseProxyConventionBuilder ConfigureEndpoints(Action convention) { _ = convention ?? throw new ArgumentNullException(nameof(convention)); @@ -52,7 +55,7 @@ void Action(EndpointBuilder endpointBuilder) /// /// The convention to add to the builder. /// - public ReverseProxyConventionBuilder Configure(Action convention) + public ReverseProxyConventionBuilder ConfigureEndpoints(Action convention) { _ = convention ?? throw new ArgumentNullException(nameof(convention)); @@ -73,7 +76,7 @@ void Action(EndpointBuilder endpointBuilder) /// /// The convention to add to the builder. /// - public ReverseProxyConventionBuilder Configure(Action convention) + public ReverseProxyConventionBuilder ConfigureEndpoints(Action convention) { _ = convention ?? throw new ArgumentNullException(nameof(convention)); @@ -92,7 +95,7 @@ void Action(EndpointBuilder endpointBuilder) return this; } - internal class EndpointBuilderConventionBuilder : IEndpointConventionBuilder + private class EndpointBuilderConventionBuilder : IEndpointConventionBuilder { private readonly EndpointBuilder _endpointBuilder; diff --git a/src/ReverseProxy/Service/Management/ProxyConfigManager.cs b/src/ReverseProxy/Service/Management/ProxyConfigManager.cs index 1f20e446a..f956cb5ea 100644 --- a/src/ReverseProxy/Service/Management/ProxyConfigManager.cs +++ b/src/ReverseProxy/Service/Management/ProxyConfigManager.cs @@ -109,15 +109,15 @@ private void Initialize() private void CreateEndpoints() { - var endpoints = new List(); - foreach (var existingRoute in _routeManager.GetItems()) + var routes = _routeManager.GetItems(); + var endpoints = new List(routes.Count); + foreach (var existingRoute in routes) { - var runtimeConfig = existingRoute.Config; // Only rebuild the endpoint for modified routes or clusters. var endpoint = existingRoute.CachedEndpoint; if (endpoint == null) { - endpoint = _proxyEndpointFactory.CreateEndpoint(runtimeConfig, _conventions); + endpoint = _proxyEndpointFactory.CreateEndpoint(existingRoute.Config, _conventions); existingRoute.CachedEndpoint = endpoint; } endpoints.Add(endpoint); @@ -214,9 +214,9 @@ private async Task ApplyConfigAsync(IProxyConfig config) } // Update clusters first because routes need to reference them. - var clustersChanged = UpdateRuntimeClusters(configuredClusters); + UpdateRuntimeClusters(configuredClusters); var routesChanged = UpdateRuntimeRoutes(configuredRoutes); - return routesChanged || clustersChanged; + return routesChanged; } private async Task<(IList, IList)> VerifyRoutesAsync(IReadOnlyList routes, CancellationToken cancellation) @@ -326,10 +326,9 @@ private async Task ApplyConfigAsync(IProxyConfig config) return (configuredClusters, errors); } - private bool UpdateRuntimeClusters(IList newClusters) + private void UpdateRuntimeClusters(IList newClusters) { var desiredClusters = new HashSet(StringComparer.OrdinalIgnoreCase); - var changed = false; foreach (var newCluster in newClusters) { @@ -408,15 +407,12 @@ private bool UpdateRuntimeClusters(IList newClusters) currentCluster.ForceUpdateDynamicState(); }); - - changed |= clusterChanged; } foreach (var existingCluster in _clusterManager.GetItems()) { if (!desiredClusters.Contains(existingCluster.ClusterId)) { - changed = true; // NOTE 1: This is safe to do within the `foreach` loop // because `IClusterManager.GetItems` returns a copy of the list of clusters. // @@ -427,8 +423,6 @@ private bool UpdateRuntimeClusters(IList newClusters) _clusterManager.TryRemoveItem(existingCluster.ClusterId); } } - - return changed; } private bool UpdateRuntimeDestinations(IDictionary newDestinations, IDestinationManager destinationManager) diff --git a/test/ReverseProxy.Tests/DynamicEndpoint/ReverseProxyConventionBuilderTests.cs b/test/ReverseProxy.Tests/DynamicEndpoint/ReverseProxyConventionBuilderTests.cs index 274b1e5f3..3fff6b4cb 100644 --- a/test/ReverseProxy.Tests/DynamicEndpoint/ReverseProxyConventionBuilderTests.cs +++ b/test/ReverseProxy.Tests/DynamicEndpoint/ReverseProxyConventionBuilderTests.cs @@ -24,7 +24,7 @@ public void ReverseProxyConventionBuilder_Configure_Works() var conventions = new List>(); var builder = new ReverseProxyConventionBuilder(conventions); - builder.Configure(builder => + builder.ConfigureEndpoints(builder => { configured = true; }); @@ -47,7 +47,7 @@ public void ReverseProxyConventionBuilder_ConfigureWithProxy_Works() var conventions = new List>(); var builder = new ReverseProxyConventionBuilder(conventions); - builder.Configure((builder, proxy) => + builder.ConfigureEndpoints((builder, proxy) => { configured = true; }); @@ -70,7 +70,7 @@ public void ReverseProxyConventionBuilder_ConfigureWithProxyAndCluster_Works() var conventions = new List>(); var builder = new ReverseProxyConventionBuilder(conventions); - builder.Configure((builder, proxy, cluster) => + builder.ConfigureEndpoints((builder, proxy, cluster) => { configured = true; }); diff --git a/test/ReverseProxy.Tests/Middleware/LoadBalancerMiddlewareTests.cs b/test/ReverseProxy.Tests/Middleware/LoadBalancerMiddlewareTests.cs index 0180af958..958669ddd 100644 --- a/test/ReverseProxy.Tests/Middleware/LoadBalancerMiddlewareTests.cs +++ b/test/ReverseProxy.Tests/Middleware/LoadBalancerMiddlewareTests.cs @@ -186,7 +186,7 @@ private static HttpContext CreateContext(string loadBalancingPolicy, IReadOnlyLi }); context.Features.Set(cluster); - var routeConfig = new RouteConfig(new RouteInfo("route-1"), new ProxyRoute(), cluster, Array.Empty(), transforms: null); + var routeConfig = new RouteConfig(new RouteInfo("route-1"), new ProxyRoute(), cluster, transforms: null); var endpoint = new Endpoint(default, new EndpointMetadataCollection(routeConfig), string.Empty); context.SetEndpoint(endpoint); diff --git a/test/ReverseProxy.Tests/Service/Proxy/LoadBalancingPoliciesTests.cs b/test/ReverseProxy.Tests/Service/Proxy/LoadBalancingPoliciesTests.cs index 19b224d24..a21a2f70b 100644 --- a/test/ReverseProxy.Tests/Service/Proxy/LoadBalancingPoliciesTests.cs +++ b/test/ReverseProxy.Tests/Service/Proxy/LoadBalancingPoliciesTests.cs @@ -136,7 +136,7 @@ public void PickDestination_RoundRobin_Works() var context = new DefaultHttpContext(); - var routeConfig = new RouteConfig(new RouteInfo("route-1"), new ProxyRoute(), new ClusterInfo("cluster1", new DestinationManager()), Array.Empty(), transforms: null); + var routeConfig = new RouteConfig(new RouteInfo("route-1"), new ProxyRoute(), new ClusterInfo("cluster1", new DestinationManager()), transforms: null); var endpoint = new Endpoint(default, new EndpointMetadataCollection(routeConfig), string.Empty); context.SetEndpoint(endpoint);