From c1abe11520b675357f091b9371bdac22d55403c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stano=20Pe=C5=A5ko?= Date: Mon, 4 Dec 2023 06:26:32 +0100 Subject: [PATCH] Match multiple ocelot routes for given downstream endpoint (#284) * Match multiple Ocelot routes for downstream path * Remove redundant routes * Add test * Version 8.1.0 --- .../Aggregates/RoutesDocumentationProvider.cs | 2 +- .../Configuration/RouteOptions.cs | 4 +- .../MMLib.SwaggerForOcelot.csproj | 2 +- .../StringExtensions.cs | 17 +- .../Transformation/SwaggerJsonTransformer.cs | 148 +++++++++--------- .../MMLib.SwaggerForOcelot.Tests.csproj | 4 + ...DifferentOcelotRoutesForOneDownstream.json | 48 ++++++ ...elotRoutesForOneDownstreamTransformed.json | 68 ++++++++ .../SwaggerForOcelotMiddlewareShould.cs | 81 ++++++++++ 9 files changed, 289 insertions(+), 85 deletions(-) create mode 100644 tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstream.json create mode 100644 tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstreamTransformed.json diff --git a/src/MMLib.SwaggerForOcelot/Aggregates/RoutesDocumentationProvider.cs b/src/MMLib.SwaggerForOcelot/Aggregates/RoutesDocumentationProvider.cs index 50da1fe..6d2c2ea 100644 --- a/src/MMLib.SwaggerForOcelot/Aggregates/RoutesDocumentationProvider.cs +++ b/src/MMLib.SwaggerForOcelot/Aggregates/RoutesDocumentationProvider.cs @@ -46,7 +46,7 @@ private RouteDocs GetRouteDocs(RouteOptions route) JToken paths = docs[OpenApiProperties.Paths]; string downstreamPath = GetDownstreamPath(route); JProperty path = paths.OfType().FirstOrDefault(p => - downstreamPath.Equals(p.Name.WithShashEnding(), StringComparison.CurrentCultureIgnoreCase)); + downstreamPath.Equals(p.Name.WithSlashEnding(), StringComparison.OrdinalIgnoreCase)); return new RouteDocs() { diff --git a/src/MMLib.SwaggerForOcelot/Configuration/RouteOptions.cs b/src/MMLib.SwaggerForOcelot/Configuration/RouteOptions.cs index 63c2c04..31e179f 100644 --- a/src/MMLib.SwaggerForOcelot/Configuration/RouteOptions.cs +++ b/src/MMLib.SwaggerForOcelot/Configuration/RouteOptions.cs @@ -134,7 +134,7 @@ public RouteOptions( /// public string DownstreamPath => DownstreamPathWithVirtualDirectory.RemoveSlashFromEnd(); - internal string DownstreamPathWithSlash => DownstreamPathWithVirtualDirectory.WithShashEnding(); + internal string DownstreamPathWithSlash => DownstreamPathWithVirtualDirectory.WithSlashEnding(); private readonly string _downstreamPathWithVirtualDirectory = null; @@ -162,7 +162,7 @@ private string DownstreamPathWithVirtualDirectory /// Gets a value indicating whether this instance can catch all. /// public bool CanCatchAll - => DownstreamPathTemplate.EndsWith(CatchAllPlaceHolder, StringComparison.CurrentCultureIgnoreCase); + => DownstreamPathTemplate.EndsWith(CatchAllPlaceHolder, StringComparison.OrdinalIgnoreCase); /// /// Gets the upstream path. diff --git a/src/MMLib.SwaggerForOcelot/MMLib.SwaggerForOcelot.csproj b/src/MMLib.SwaggerForOcelot/MMLib.SwaggerForOcelot.csproj index d473cf8..6744c72 100644 --- a/src/MMLib.SwaggerForOcelot/MMLib.SwaggerForOcelot.csproj +++ b/src/MMLib.SwaggerForOcelot/MMLib.SwaggerForOcelot.csproj @@ -1,7 +1,7 @@  net6.0;net7.0;net8.0 - 8.0.0 + 8.1.0 Milan Martiniak MMLib Swagger generator for Ocelot downstream services. diff --git a/src/MMLib.SwaggerForOcelot/StringExtensions.cs b/src/MMLib.SwaggerForOcelot/StringExtensions.cs index bdc1a82..f52fbdf 100644 --- a/src/MMLib.SwaggerForOcelot/StringExtensions.cs +++ b/src/MMLib.SwaggerForOcelot/StringExtensions.cs @@ -10,16 +10,19 @@ internal static class StringExtensions /// /// The value. public static string RemoveSlashFromEnd(this string value) - => value.TrimEnd().EndsWith("/") - ? value.TrimEnd().TrimEnd('/') - : value; + => value.TrimEnd().TrimEnd('/'); /// /// Add slash to end. /// - public static string WithShashEnding(this string value) - => !value.TrimEnd().EndsWith("/") - ? value + "/" - : value; + public static string WithSlashEnding(this string value) + { + value = value.TrimEnd(); + if (!value.EndsWith('/')) + { + value += "/"; + } + return value; + } } } diff --git a/src/MMLib.SwaggerForOcelot/Transformation/SwaggerJsonTransformer.cs b/src/MMLib.SwaggerForOcelot/Transformation/SwaggerJsonTransformer.cs index 8a1c5b5..f6056c1 100644 --- a/src/MMLib.SwaggerForOcelot/Transformation/SwaggerJsonTransformer.cs +++ b/src/MMLib.SwaggerForOcelot/Transformation/SwaggerJsonTransformer.cs @@ -1,11 +1,10 @@ -using Kros.IO; +using Kros.IO; using Microsoft.Extensions.Caching.Memory; using MMLib.SwaggerForOcelot.Configuration; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Security.Cryptography; using System.Text; @@ -171,81 +170,54 @@ private string TransformOpenApi( private void RenameAndRemovePaths(IEnumerable routes, JToken paths, string basePath) { - var forRemove = new List(); - + var oldPaths = new List(); + var newPaths = new Dictionary(StringComparer.OrdinalIgnoreCase); for (int i = 0; i < paths.Count(); i++) { - var path = paths.ElementAt(i) as JProperty; - string downstreamPath = path.Name.RemoveSlashFromEnd(); - RouteOptions route = FindRoute(routes, path.Name.WithShashEnding(), basePath); - - if (route != null && RemoveMethods(path, route)) - { - AddSecurityDefinitions(path, route); - - RenameToken(path, ConvertDownstreamPathToUpstreamPath(downstreamPath, route.DownstreamPath, route.UpstreamPath, basePath)); - } - else + var oldPath = paths.ElementAt(i) as JProperty; + oldPaths.Add(oldPath); + string downstreamPath = oldPath.Name.RemoveSlashFromEnd(); + foreach (var tmpMethod in oldPath.First) { - forRemove.Add(path); + var method = tmpMethod as JProperty; + List matchedRoutes = FindRoutes(routes, oldPath.Name.WithSlashEnding(), method.Name, basePath); + foreach (var route in matchedRoutes) + { + string newPath = ConvertDownstreamPathToUpstreamPath( + downstreamPath, route.DownstreamPath, route.UpstreamPath, basePath); + if (!newPaths.TryGetValue(newPath, out JProperty newPathJson)) + { + newPathJson = new JProperty(newPath, new JObject()); + newPaths.Add(newPath, newPathJson); + } + var newMethod = (method.DeepClone() as JProperty); + AddSecurityDefinition(newMethod, route); + ((JObject)newPathJson.Value).Add(newMethod); + } } } - foreach (JProperty p in forRemove) - { - p.Remove(); - } + oldPaths.ForEach(oldPath => oldPath.Remove()); + newPaths.Select(item => item.Value) + .OrderBy(newPath => newPath.Name) + .ForEach(newPath => ((JObject)paths).Add(newPath)); } - private bool RemoveMethods(JProperty path, RouteOptions route) - { - var forRemove = new List(); - var method = path.First.First as JProperty; - - while (method != null) - { - if (!route.ContainsHttpMethod(method.Name)) - { - forRemove.Add(method); - } - method = method.Next as JProperty; - } - - foreach (JProperty m in forRemove) - { - m.Remove(); - } - - return path.First.Any(); - } - - private void AddSecurityDefinitions(JProperty path, RouteOptions route) + private void AddSecurityDefinition(JProperty method, RouteOptions route) { var authProviderKey = route.AuthenticationOptions?.AuthenticationProviderKey; - if (string.IsNullOrEmpty(authProviderKey)) { return; } - - if (_ocelotSwaggerGenOptions.AuthenticationProviderKeyMap.TryGetValue( - authProviderKey, - out var securityScheme)) + if (_ocelotSwaggerGenOptions.AuthenticationProviderKeyMap.TryGetValue(authProviderKey, out var securityScheme)) { - var method = path.First.First as JProperty; - - while (method != null) - { - var securityProperty = new JProperty(OpenApiProperties.Security, - new JArray( - new JObject( - new JProperty(securityScheme, - new JArray(route.AuthenticationOptions?.AllowedScopes?.ToArray() ?? Array.Empty()))))); - - ((JObject)method.Value).Add(securityProperty); - - method = method.Next as JProperty; - } + var securityProperty = new JProperty(OpenApiProperties.Security, + new JArray( + new JObject( + new JProperty(securityScheme, + new JArray(route.AuthenticationOptions?.AllowedScopes?.ToArray() ?? []))))); + ((JObject)method.Value).Add(securityProperty); } } @@ -297,13 +269,47 @@ private static void CheckSubreferences(IEnumerable token, Func routes, string downstreamPath, string basePath) + private static List FindRoutes( + IEnumerable routes, + string downstreamPath, + string method, + string basePath) { + static bool MatchPaths(RouteOptions route, string downstreamPath) + => route.CanCatchAll + ? downstreamPath.StartsWith(route.DownstreamPathWithSlash, StringComparison.OrdinalIgnoreCase) + : route.DownstreamPathWithSlash.Equals(downstreamPath, StringComparison.OrdinalIgnoreCase); + string downstreamPathWithBasePath = PathHelper.BuildPath(basePath, downstreamPath); - return routes.FirstOrDefault(p - => p.CanCatchAll - ? downstreamPathWithBasePath.StartsWith(p.DownstreamPathWithSlash, StringComparison.CurrentCultureIgnoreCase) - : p.DownstreamPathWithSlash.Equals(downstreamPathWithBasePath, StringComparison.CurrentCultureIgnoreCase)); + var matchedRoutes = routes + .Where(route => route.ContainsHttpMethod(method) && MatchPaths(route, downstreamPathWithBasePath)) + .ToList(); + + RemoveRedundantRoutes(matchedRoutes); + return matchedRoutes; + } + + // Redundant routes are routes with the ALMOST same upstream path templates. For example these path templates + // are redundant: + // - /api/projects/Projects + // - /api/projects/Projects/ + // - /api/projects/Projects/{everything} + // + // `route.UpstreamPath` contains route without trailing slash and without catch-all placeholder, so all previous + // routes have the same upstream path `/api/projects/Projects`. The logic is to keep just the shortestof the path + // templates. If we would keep all routes, it will throw an exception during the generation of the swagger document + // later because of the same paths. + private static void RemoveRedundantRoutes(List routes) + { + IEnumerable> groups = routes + .GroupBy(route => route.UpstreamPath, StringComparer.OrdinalIgnoreCase) + .Where(group => group.Count() > 1); + foreach (var group in groups) + { + group.OrderBy(r => r.DownstreamPathTemplate.Length) + .Skip(1) + .ForEach(r => routes.Remove(r)); + } } private static void AddHost(JObject swagger, string swaggerHost) @@ -325,7 +331,7 @@ private static string ConvertDownstreamPathToUpstreamPath(string downstreamPath, downstreamPath = PathHelper.BuildPath(downstreamBasePath, downstreamPath); } - int pos = downstreamPath.IndexOf(downstreamPattern, StringComparison.CurrentCultureIgnoreCase); + int pos = downstreamPath.IndexOf(downstreamPattern, StringComparison.OrdinalIgnoreCase); if (pos < 0) { return downstreamPath; @@ -333,12 +339,6 @@ private static string ConvertDownstreamPathToUpstreamPath(string downstreamPath, return $"{downstreamPath.Substring(0, pos)}{upstreamPattern}{downstreamPath.Substring(pos + downstreamPattern.Length)}"; } - private static void RenameToken(JProperty property, string newName) - { - var newProperty = new JProperty(newName, property.Value); - property.Replace(newProperty); - } - private static void TransformServerPaths(JObject openApi, string serverOverride, bool takeServersFromDownstreamService) { if (!openApi.ContainsKey(OpenApiProperties.Servers) || takeServersFromDownstreamService) diff --git a/tests/MMLib.SwaggerForOcelot.Tests/MMLib.SwaggerForOcelot.Tests.csproj b/tests/MMLib.SwaggerForOcelot.Tests/MMLib.SwaggerForOcelot.Tests.csproj index 200316a..5cbb65d 100644 --- a/tests/MMLib.SwaggerForOcelot.Tests/MMLib.SwaggerForOcelot.Tests.csproj +++ b/tests/MMLib.SwaggerForOcelot.Tests/MMLib.SwaggerForOcelot.Tests.csproj @@ -10,6 +10,8 @@ + + @@ -20,6 +22,8 @@ + + diff --git a/tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstream.json b/tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstream.json new file mode 100644 index 0000000..61e3bb6 --- /dev/null +++ b/tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstream.json @@ -0,0 +1,48 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "WebApplication1", + "version": "1.0" + }, + "paths": { + "/api/test": { + "get": { + "tags": [ + "WebApplication1" + ], + "operationId": "testGet", + "responses": { + "200": { + "description": "OK", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } + } + } + }, + "post": { + "tags": [ + "WebApplication1" + ], + "operationId": "testPost", + "responses": { + "200": { + "description": "OK", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } + } + } + } + } + }, + "components": {} +} \ No newline at end of file diff --git a/tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstreamTransformed.json b/tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstreamTransformed.json new file mode 100644 index 0000000..8eda668 --- /dev/null +++ b/tests/MMLib.SwaggerForOcelot.Tests/Resources/DifferentOcelotRoutesForOneDownstreamTransformed.json @@ -0,0 +1,68 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "WebApplication1", + "version": "1.0" + }, + "paths": { + "/all/ocelot": { + "get": { + "tags": [ + "WebApplication1" + ], + "operationId": "testGet", + "responses": { + "200": { + "description": "OK", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } + } + } + }, + "post": { + "tags": [ + "WebApplication1" + ], + "operationId": "testPost", + "responses": { + "200": { + "description": "OK", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } + } + } + } + }, + "/post/ocelot": { + "post": { + "tags": [ + "WebApplication1" + ], + "operationId": "testPost", + "responses": { + "200": { + "description": "OK", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } + } + } + } + } + }, + "components": {} +} \ No newline at end of file diff --git a/tests/MMLib.SwaggerForOcelot.Tests/SwaggerForOcelotMiddlewareShould.cs b/tests/MMLib.SwaggerForOcelot.Tests/SwaggerForOcelotMiddlewareShould.cs index bd112a6..a8b4d33 100644 --- a/tests/MMLib.SwaggerForOcelot.Tests/SwaggerForOcelotMiddlewareShould.cs +++ b/tests/MMLib.SwaggerForOcelot.Tests/SwaggerForOcelotMiddlewareShould.cs @@ -30,6 +30,87 @@ namespace MMLib.SwaggerForOcelot.Tests { public class SwaggerForOcelotMiddlewareShould { + [Fact] + public async Task TransformDifferentOcelotRoutesForOneDownstreamPath() + { + // Arrange + const string version = "v1"; + const string key = "test"; + HttpContext httpContext = GetHttpContext(requestPath: $"/{version}/{key}"); + IMemoryCache memoryCache = new MemoryCache(Options.Create(new MemoryCacheOptions())); + + var next = new TestRequestDelegate(); + + // What is being tested + var swaggerForOcelotOptions = new SwaggerForOcelotUIOptions(); + TestSwaggerEndpointOptions swaggerEndpointOptions = CreateSwaggerEndpointOptions(key, version); + var routeOptions = new TestRouteOptions(new List + { + new () + { + SwaggerKey = "test", + UpstreamPathTemplate = "/post/ocelot", + UpstreamHttpMethod = [ "Post" ], + DownstreamPathTemplate = "/api/test", + }, + new() + { + SwaggerKey = "test", + UpstreamPathTemplate = "/all/ocelot", + DownstreamPathTemplate = "/api/test", + } + }); + + // downstreamSwagger is returned when client.GetStringAsync is called by the middleware. + string downstreamSwagger = await GetBaseOpenApi("DifferentOcelotRoutesForOneDownstream"); + HttpClient httClientMock = GetHttpClient(downstreamSwagger); + var httpClientFactory = new TestHttpClientFactory(httClientMock); + + // upstreamSwagger is returned after swaggerJsonTransformer transforms the downstreamSwagger + string expectedSwagger = await GetBaseOpenApi("DifferentOcelotRoutesForOneDownstreamTransformed"); + + var swaggerJsonTransformerMock = new Mock(); + swaggerJsonTransformerMock + .Setup(x => x.Transform( + It.IsAny(), + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .Returns(( + string swaggerJson, + IEnumerable routeOptions, + string serverOverride, + SwaggerEndPointOptions options) => new SwaggerJsonTransformer(OcelotSwaggerGenOptions.Default, memoryCache) + .Transform(swaggerJson, routeOptions, serverOverride, options)); + var swaggerForOcelotMiddleware = new SwaggerForOcelotMiddleware( + next.Invoke, + swaggerForOcelotOptions, + routeOptions, + swaggerJsonTransformerMock.Object, + Substitute.For()); + + // Act + await swaggerForOcelotMiddleware.Invoke( + httpContext, + new SwaggerEndPointProvider(swaggerEndpointOptions, OcelotSwaggerGenOptions.Default), + new DownstreamSwaggerDocsRepository(Options.Create(swaggerForOcelotOptions), + httpClientFactory, DummySwaggerServiceDiscoveryProvider.Default)); + httpContext.Response.Body.Seek(0, SeekOrigin.Begin); + + // Assert + using (var streamReader = new StreamReader(httpContext.Response.Body)) + { + string transformedUpstreamSwagger = await streamReader.ReadToEndAsync(); + AreEqual(transformedUpstreamSwagger, expectedSwagger); + } + + swaggerJsonTransformerMock.Verify(x => x.Transform( + It.IsAny(), + It.IsAny>(), + It.IsAny(), + It.IsAny()), Times.Once); + } + [Fact] public async Task AllowVersionPlaceholder() {