Skip to content

Commit

Permalink
Bug Fix #65 - If one path was substring another one, then transfromat… (
Browse files Browse the repository at this point in the history
#66)

* Bug Fix #65 - If one path was substring another one, then transfromation removed same methods.

* Codefactor fix
  • Loading branch information
Burgyn authored Jan 16, 2020
1 parent 34dd25b commit dc31a7f
Show file tree
Hide file tree
Showing 10 changed files with 1,381 additions and 56 deletions.
49 changes: 46 additions & 3 deletions src/MMLib.SwaggerForOcelot/Configuration/ReRouteOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,33 @@ public ReRouteOptions()
{
_httpMethods = new Lazy<HashSet<string>>(()
=> new HashSet<string>(
UpstreamHttpMethod?.Count() > 0 ? UpstreamHttpMethod: DefaultMethodsTypes,
UpstreamHttpMethod?.Count() > 0 ? UpstreamHttpMethod : DefaultMethodsTypes,
StringComparer.OrdinalIgnoreCase));
}

/// <summary>
/// Initializes a new instance of the <see cref="ReRouteOptions"/> class.
/// </summary>
/// <param name="swaggerKey">The swagger key.</param>
/// <param name="upstreamPathTemplate">The upstream path template.</param>
/// <param name="downstreamPathTemplate">The downstream path template.</param>
/// <param name="virtualDirectory">The virtual directory.</param>
/// <param name="upstreamMethods">The upstream methods.</param>
public ReRouteOptions(
string swaggerKey,
string upstreamPathTemplate,
string downstreamPathTemplate,
string virtualDirectory,
IEnumerable<string> upstreamMethods)
: this()
{
SwaggerKey = swaggerKey;
UpstreamPathTemplate = upstreamPathTemplate;
DownstreamPathTemplate = downstreamPathTemplate;
VirtualDirectory = virtualDirectory;
UpstreamHttpMethod = upstreamMethods;
}

/// <summary>
/// Swagger key. This key is used for generating swagger documentation for downstream services.
/// The same key have to be in <see cref="SwaggerEndPointOptions"/> collection.
Expand Down Expand Up @@ -64,18 +87,29 @@ internal bool ContainsHttpMethod(string methodType)
/// <summary>
/// Gets the downstream path.
/// </summary>
public string DownstreamPath
public string DownstreamPath => DownstreamPathWithVirtualDirectory.RemoveSlashFromEnd();

internal string DownstreamPathWithShash => DownstreamPathWithVirtualDirectory.WithShashEnding();

private string _downstreamPathWithVirtualDirectory = null;

private string DownstreamPathWithVirtualDirectory
{
get
{
if (!_downstreamPathWithVirtualDirectory.IsNullOrEmpty())
{
return _downstreamPathWithVirtualDirectory;
}

var ret = Replace(DownstreamPathTemplate);
if (!VirtualDirectory.IsNullOrWhiteSpace()
&& ret.StartsWith(VirtualDirectory, StringComparison.OrdinalIgnoreCase))
{
ret = ret.Substring(VirtualDirectory.Length);
}

return ret.RemoveSlashFromEnd();
return ret;
}
}

Expand All @@ -91,5 +125,14 @@ public bool CanCatchAll
public string UpstreamPath => Replace(UpstreamPathTemplate).RemoveSlashFromEnd();

private string Replace(string value) => value.Replace(CatchAllPlaceHolder, "");

/// <summary>
/// Converts to string.
/// </summary>
/// <returns>
/// A <see cref="System.String" /> that represents this instance.
/// </returns>
public override string ToString()
=> $"{UpstreamPathTemplate} => {DownstreamPathTemplate} | ({UpstreamPath} => {DownstreamPath})";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,45 +65,16 @@ public async Task Invoke(HttpContext context)
AddHeaders(httpClient);
var content = await httpClient.GetStringAsync(endPoint.Url);
var hostName = endPoint.EndPoint.HostOverride ?? UriHelper.BuildAbsolute(context.Request.Scheme, context.Request.Host).RemoveSlashFromEnd();
var reRouteOptions = ExpandReRouteOptions(endPoint.EndPoint);
var reRouteOptions = _reRoutes.Value
.ExpandConfig(endPoint.EndPoint)
.GroupByPaths();

content = _transformer.Transform(content, reRouteOptions, hostName);
content = await ReconfigureUpstreamSwagger(context, content);

await context.Response.WriteAsync(content);
}

private IEnumerable<ReRouteOptions> ExpandReRouteOptions(SwaggerEndPointOptions endPoint)
{
var reRouteOptions = _reRoutes.Value.Where(p => p.SwaggerKey == endPoint.Key).ToList();

if (string.IsNullOrWhiteSpace(endPoint.VersionPlaceholder))
return reRouteOptions;

var versionReRouteOptions = reRouteOptions.Where(x =>
x.DownstreamPathTemplate.Contains(endPoint.VersionPlaceholder)
|| x.UpstreamPathTemplate.Contains(endPoint.VersionPlaceholder)).ToList();
versionReRouteOptions.ForEach(o => reRouteOptions.Remove(o));
foreach (var reRouteOption in versionReRouteOptions)
{
var versionMappedReRouteOptions = endPoint.Config.Select(c => new ReRouteOptions()
{
SwaggerKey = reRouteOption.SwaggerKey,
DownstreamPathTemplate =
reRouteOption.DownstreamPathTemplate.Replace(endPoint.VersionPlaceholder,
c.Version),
UpstreamHttpMethod = reRouteOption.UpstreamHttpMethod,
UpstreamPathTemplate =
reRouteOption.UpstreamPathTemplate.Replace(endPoint.VersionPlaceholder,
c.Version),
VirtualDirectory = reRouteOption.VirtualDirectory
});
reRouteOptions.AddRange(versionMappedReRouteOptions);
}

return reRouteOptions;
}

private async Task<string> ReconfigureUpstreamSwagger(HttpContext context, string swaggerJson)
{
if (_options.ReConfigureUpstreamSwaggerJson != null && _options.ReConfigureUpstreamSwaggerJsonAsync != null)
Expand Down
3 changes: 3 additions & 0 deletions src/MMLib.SwaggerForOcelot/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("MMLib.SwaggerForOcelot.Tests")]
67 changes: 67 additions & 0 deletions src/MMLib.SwaggerForOcelot/ReRouteOptionsExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
using MMLib.SwaggerForOcelot.Configuration;
using System.Collections.Generic;
using System.Linq;

namespace MMLib.SwaggerForOcelot
{
/// <summary>
/// Extensions for <see cref="ReRouteOptions"/>.
/// </summary>
internal static class ReRouteOptionsExtensions
{
/// <summary>
/// Goups the re routes by paths.
/// </summary>
/// <param name="reRouteOptions">The re route options.</param>
public static IEnumerable<ReRouteOptions> GroupByPaths(this IEnumerable<ReRouteOptions> reRouteOptions)
=> reRouteOptions
.GroupBy(p => new { p.SwaggerKey, p.UpstreamPathTemplate, p.DownstreamPathTemplate, p.VirtualDirectory})
.Select(p => {
var route = p.First();
return new ReRouteOptions(
p.Key.SwaggerKey,
route.UpstreamPathTemplate,
route.DownstreamPathTemplate,
p.Key.VirtualDirectory,
p.Where(r => r.UpstreamHttpMethod != null).SelectMany(r => r.UpstreamHttpMethod));
});

/// <summary>
/// Expands versions from one endpoint to more ReRoute options.
/// </summary>
/// <param name="reRoutes">The re routes.</param>
/// <param name="endPoint">The end point.</param>
public static IEnumerable<ReRouteOptions> ExpandConfig(
this IEnumerable<ReRouteOptions> reRoutes,
SwaggerEndPointOptions endPoint)
{
var reRouteOptions = reRoutes.Where(p => p.SwaggerKey == endPoint.Key).ToList();

if (string.IsNullOrWhiteSpace(endPoint.VersionPlaceholder))
return reRouteOptions;

var versionReRouteOptions = reRouteOptions.Where(x =>
x.DownstreamPathTemplate.Contains(endPoint.VersionPlaceholder)
|| x.UpstreamPathTemplate.Contains(endPoint.VersionPlaceholder)).ToList();
versionReRouteOptions.ForEach(o => reRouteOptions.Remove(o));
foreach (var reRouteOption in versionReRouteOptions)
{
var versionMappedReRouteOptions = endPoint.Config.Select(c => new ReRouteOptions()
{
SwaggerKey = reRouteOption.SwaggerKey,
DownstreamPathTemplate =
reRouteOption.DownstreamPathTemplate.Replace(endPoint.VersionPlaceholder,
c.Version),
UpstreamHttpMethod = reRouteOption.UpstreamHttpMethod,
UpstreamPathTemplate =
reRouteOption.UpstreamPathTemplate.Replace(endPoint.VersionPlaceholder,
c.Version),
VirtualDirectory = reRouteOption.VirtualDirectory
});
reRouteOptions.AddRange(versionMappedReRouteOptions);
}

return reRouteOptions;
}
}
}
12 changes: 10 additions & 2 deletions src/MMLib.SwaggerForOcelot/StringExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
namespace System
{
/// <summary>
///String extensions
/// String extensions
/// </summary>
public static class StringExtensions
internal static class StringExtensions
{
/// <summary>
/// Removes the slash from end.
Expand All @@ -13,5 +13,13 @@ public static string RemoveSlashFromEnd(this string value)
=> value.TrimEnd().EndsWith("/")
? value.TrimEnd().TrimEnd('/')
: value;

/// <summary>
/// Add slash to end.
/// </summary>
public static string WithShashEnding(this string value)
=> !value.TrimEnd().EndsWith("/")
? value + "/"
: value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private void RenameAndRemovePaths(IEnumerable<ReRouteOptions> reRoutes, JToken p
{
var path = paths.ElementAt(i) as JProperty;
string downstreamPath = path.Name.RemoveSlashFromEnd();
var reRoute = FindReRoute(reRoutes, downstreamPath, basePath);
var reRoute = FindReRoute(reRoutes, path.Name.WithShashEnding(), basePath);

if (reRoute != null && RemoveMethods(path, reRoute))
{
Expand Down Expand Up @@ -213,8 +213,8 @@ private static ReRouteOptions FindReRoute(IEnumerable<ReRouteOptions> reRoutes,
var downstreamPathWithBasePath = PathHelper.BuildPath(basePath, downstreamPath);
return reRoutes.FirstOrDefault(p
=> p.CanCatchAll
? downstreamPathWithBasePath.StartsWith(p.DownstreamPath, StringComparison.CurrentCultureIgnoreCase)
: p.DownstreamPath.Equals(downstreamPathWithBasePath, StringComparison.CurrentCultureIgnoreCase));
? downstreamPathWithBasePath.StartsWith(p.DownstreamPathWithShash, StringComparison.CurrentCultureIgnoreCase)
: p.DownstreamPathWithShash.Equals(downstreamPathWithBasePath, StringComparison.CurrentCultureIgnoreCase));
}

private static void AddHost(JObject swagger, string swaggerHost)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
<TargetFramework>netcoreapp3</TargetFramework>
<IsPackable>false</IsPackable>
</PropertyGroup>
<ItemGroup>
<None Remove="Tests\Issue_65.json" />
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="Resources\OpenApiWithVersionPlaceholderBase.json" />
<EmbeddedResource Include="Resources\OpenApiWithVersionPlaceholderBaseTransformed.json" />
<EmbeddedResource Include="Resources\OpenApiBase.json" />
<EmbeddedResource Include="Resources\OpenApiBaseTransformed.json" />

<EmbeddedResource Include="Tests\BasicConfiguration.json" />

<EmbeddedResource Include="Tests\Issue_65.json" />
<EmbeddedResource Include="Tests\SwaggerWithBasePath.json" />
<EmbeddedResource Include="Tests\OcelotRouteOnlyOneController.json" />
<EmbeddedResource Include="Tests\BasicConfigurationWithVirtualDirectory.json" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using FluentAssertions;
using MMLib.SwaggerForOcelot.Configuration;
using System.Collections.Generic;
using System.Linq;
using Xunit;

namespace MMLib.SwaggerForOcelot.Tests
{
public class ReRouteOptionsExtensionsShould
{
[Fact]
public void GroupReRoutesByPaths()
{
IEnumerable<ReRouteOptions> reRouteOptions = new List<ReRouteOptions>()
{
new ReRouteOptions(){
DownstreamPathTemplate= "/api/masterdatatype",
UpstreamPathTemplate = "/masterdatatype",
UpstreamHttpMethod = new HashSet<string>(){ "Get"}
},
new ReRouteOptions(){
DownstreamPathTemplate= "/api/masterdatatype",
UpstreamPathTemplate = "/masterdatatype",
UpstreamHttpMethod = new HashSet<string>(){ "Post"}
},
new ReRouteOptions(){
DownstreamPathTemplate= "/api/masterdatatype/{everything}",
UpstreamPathTemplate = "/masterdatatype/{everything}",
UpstreamHttpMethod = new HashSet<string>(){ "Delete"}
},
new ReRouteOptions(){
DownstreamPathTemplate= "/api/masterdatatype/{everything}",
UpstreamPathTemplate = "/masterdatatype/{everything}",
UpstreamHttpMethod = new HashSet<string>(){ "Delete"},
VirtualDirectory = "something"
},
new ReRouteOptions(){
DownstreamPathTemplate= "/api/masterdata",
UpstreamPathTemplate = "/masterdata",
UpstreamHttpMethod = new HashSet<string>(){ "Delete"}
},
};

IEnumerable<ReRouteOptions> actual = reRouteOptions.GroupByPaths();

actual
.Should()
.HaveCount(4);

actual.First()
.UpstreamHttpMethod
.Should()
.BeEquivalentTo("Get", "Post");
}
}
}
Loading

2 comments on commit dc31a7f

@nilukshan
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting this issue in this version.
https://stackoverflow.com/questions/59772540/request-url-error-in-swagger-for-ocelot-like-http-http
Can you please investigate it?

@Burgyn
Copy link
Owner Author

@Burgyn Burgyn commented on dc31a7f Jan 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, it looks like issue #53

Unfortunately it is still not fixed.

As a workaround you can downgrade to version 1.8

Thank you and apologize.

Please sign in to comment.