Skip to content

Commit

Permalink
Prefer non-wildcard routes over wildcard ones if multiple routes match (
Browse files Browse the repository at this point in the history
  • Loading branch information
Kaliumhexacyanoferrat authored May 27, 2024
1 parent 206e033 commit 081797a
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 8 deletions.
8 changes: 5 additions & 3 deletions Modules/Controllers/Provider/ControllerHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace GenHTTP.Modules.Controllers.Provider

public sealed class ControllerHandler : IHandler, IHandlerResolver
{
private static readonly MethodRouting EMPTY = new("/", "^(/|)$", null, true);
private static readonly MethodRouting EMPTY = new("/", "^(/|)$", null, true, false);

#region Get-/Setters

Expand Down Expand Up @@ -68,17 +68,19 @@ private static MethodRouting DeterminePath(MethodInfo method, List<string> argum
var pathArgs = string.Join('/', arguments.Select(a => a.ToParameter()));
var rawArgs = string.Join('/', arguments.Select(a => $":{a}"));

var isWildcard = PathArguments.CheckWildcardRoute(method.ReturnType);

if (method.Name == "Index")
{
return pathArgs.Length > 0 ? new MethodRouting($"/{rawArgs}/", $"^/{pathArgs}/", null, false) : EMPTY;
return pathArgs.Length > 0 ? new MethodRouting($"/{rawArgs}/", $"^/{pathArgs}/", null, false, isWildcard) : EMPTY;
}
else
{
var name = HypenCase(method.Name);

var path = $"^/{name}";

return pathArgs.Length > 0 ? new MethodRouting($"/{name}/{rawArgs}/", $"{path}/{pathArgs}/", name, false) : new MethodRouting($"/{name}", $"{path}/", name, false);
return pathArgs.Length > 0 ? new MethodRouting($"/{name}/{rawArgs}/", $"{path}/{pathArgs}/", name, false, isWildcard) : new MethodRouting($"/{name}", $"{path}/", name, false, isWildcard);
}
}

Expand Down
8 changes: 8 additions & 0 deletions Modules/Reflection/MethodCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ public MethodCollection(IHandler parent, IEnumerable<Func<IHandler, MethodHandle
}
else if (methods.Count > 1)
{
// if there is only one non-wildcard, use this one
var nonWildcards = methods.Where(m => m.Routing.IsWildcard == false).ToList();

if (nonWildcards.Count == 1)
{
return nonWildcards[0].HandleAsync(request);
}

throw new ProviderException(ResponseStatus.BadRequest, $"There are multiple methods matching '{request.Target.Path}'");
}
else
Expand Down
16 changes: 15 additions & 1 deletion Modules/Reflection/MethodRouting.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,32 @@ public sealed class MethodRouting
/// </summary>
public bool IsIndex { get; }

/// <summary>
/// True, if this is a wildcard route that is created
/// when returning a handler or handler builder from
/// a method.
/// </summary>
/// <remarks>
/// Wildcard routes have a lower priority compared to
/// non-wildcard routes and will not be considered
/// ambiguous.
/// </remarks>
public bool IsWildcard { get; }

#endregion

#region Initialization

public MethodRouting(string path, string pathExpression, string? segment, bool isIndex)
public MethodRouting(string path, string pathExpression, string? segment, bool isIndex, bool isWildcard)
{
Path = new PathBuilder(path).Build();

_PathExpression = pathExpression;

Segment = segment;

IsIndex = isIndex;
IsWildcard = isWildcard;
}

#endregion
Expand Down
8 changes: 4 additions & 4 deletions Modules/Reflection/PathArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ namespace GenHTTP.Modules.Reflection

public static class PathArguments
{
private static readonly MethodRouting EMPTY = new("/", "^(/|)$", null, true);
private static readonly MethodRouting EMPTY = new("/", "^(/|)$", null, true, false);

private static readonly MethodRouting EMPTY_WILDCARD = new("/", "^.*", null, true);
private static readonly MethodRouting EMPTY_WILDCARD = new("/", "^.*", null, true, true);

private static readonly Regex VAR_PATTERN = new(@"\:([a-z]+)", RegexOptions.IgnoreCase | RegexOptions.Compiled);

Expand Down Expand Up @@ -47,9 +47,9 @@ public static MethodRouting Route(string? path, bool wildcard = false)

var splitted = path.Split("/".ToCharArray(), StringSplitOptions.RemoveEmptyEntries);

var end = (wildcard) ? "/" : "(/|)$";
var end = (wildcard) ? "(/|)" : "(/|)$";

return new MethodRouting(path, $"^/{builder}{end}", (splitted.Length > 0) ? splitted[0] : null, false);
return new MethodRouting(path, $"^/{builder}{end}", (splitted.Length > 0) ? splitted[0] : null, false, wildcard);
}

return (wildcard) ? EMPTY_WILDCARD : EMPTY;
Expand Down
58 changes: 58 additions & 0 deletions Testing/Acceptance/Modules/Webservices/AmbiguityTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
using System.Net;
using System.Threading.Tasks;

using GenHTTP.Api.Content;

using GenHTTP.Modules.IO;
using GenHTTP.Modules.Layouting;
using GenHTTP.Modules.Webservices;

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace GenHTTP.Testing.Acceptance.Modules.Webservices
{

[TestClass]
public sealed class AmbiguityTests
{

#region Supporting data structures

public sealed class TestService
{

[ResourceMethod]
public IHandlerBuilder Wildcard()
{
return Content.From(Resource.FromString("Wildcard"));
}

[ResourceMethod(path: "/my.txt")]
public string Specific() => "Specific";

}

#endregion

#region Tests

[TestMethod]
public async Task TestSpecificPreferred()
{
var app = Layout.Create()
.AddService<TestService>("c");

using var host = TestHost.Run(app);

using var response = await host.GetResponseAsync("/c/my.txt");

await response.AssertStatusAsync(HttpStatusCode.OK);

Assert.AreEqual("Specific", await response.GetContentAsync());
}

#endregion

}

}

0 comments on commit 081797a

Please sign in to comment.