Skip to content

Commit

Permalink
Review comments resolved
Browse files Browse the repository at this point in the history
  • Loading branch information
tomasherceg committed Jul 12, 2024
1 parent 93107dd commit e1951ab
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 31 deletions.
5 changes: 5 additions & 0 deletions src/Framework/Framework/Hosting/HostingConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ public class HostingConstants

public const string HostAppModeKey = "host.AppMode";

/// <summary>
/// When this key is set to true in the OWIN environment, the request culture will not be set by DotVVM to config.DefaultCulture.
/// Use this key when the request culture is set by the host or the middleware preceding DotVVM.
/// See https://github.com/riganti/dotvvm/blob/93107dd07127ff2bd29c2934f3aa2a26ec2ca79c/src/Samples/Owin/Startup.cs#L34
/// </summary>
public const string OwinDoNotSetRequestCulture = "OwinDoNotSetRequestCulture";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,9 @@ private static bool TryParseGooglebotHashbangEscapedFragment(string queryString,
return false;
}

public static RouteBase? FindMatchingRoute(IEnumerable<RouteBase> routes, IDotvvmRequestContext context, out IDictionary<string, object?>? parameters, out bool isPartialMatch)
public static string GetRouteMatchUrl(IDotvvmRequestContext context)
{
string? url;
if (!TryParseGooglebotHashbangEscapedFragment(context.HttpContext.Request.Url.Query, out url))
if (!TryParseGooglebotHashbangEscapedFragment(context.HttpContext.Request.Url.Query, out var url))
{
url = context.HttpContext.Request.Path.Value;
}
Expand All @@ -52,41 +51,47 @@ private static bool TryParseGooglebotHashbangEscapedFragment(string queryString,
{
url = url.Substring(HostingConstants.SpaUrlIdentifier.Length).Trim('/');
}
return url;
}

// find the route
RouteBase? partialMatch = null;
IDictionary<string, object?>? partialMatchParameters = null;

internal static RouteBase? FindExactMatchRoute(IEnumerable<RouteBase> routes, string matchUrl, out IDictionary<string, object?>? parameters)
{
foreach (var r in routes)
{
if (r.IsMatch(url, out parameters))
if (r.IsMatch(matchUrl, out parameters))
{
isPartialMatch = false;
return r;
}
}
parameters = null;
return null;
}

if (partialMatch == null
&& r is IPartialMatchRoute partialMatchRoute
&& partialMatchRoute.IsPartialMatch(url, out var partialMatchResult, out var partialMatchParametersResult))
{
partialMatch = partialMatchResult;
partialMatchParameters = partialMatchParametersResult;
}
public static RouteBase? FindMatchingRoute(DotvvmRouteTable routes, IDotvvmRequestContext context, out IDictionary<string, object?>? parameters, out bool isPartialMatch)
{
var url = GetRouteMatchUrl(context);

var route = FindExactMatchRoute(routes, url, out parameters);
if (route is { })
{
isPartialMatch = false;
return route;
}

if (partialMatch != null)
foreach (var r in routes.PartialMatchRoutes)
{
isPartialMatch = true;
parameters = partialMatchParameters;
return partialMatch;
if (r.IsPartialMatch(url, out var matchedRoute, out parameters))
{
isPartialMatch = true;
return matchedRoute;
}
}

isPartialMatch = false;
parameters = null;
return null;
}


public async Task<bool> Handle(IDotvvmRequestContext context)
{
var requestTracer = context.Services.GetRequiredService<AggregateRequestTracer>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ protected virtual string GetVersionHash(ILocalResourceLocation location, IDotvvm
public ILocalResourceLocation? FindResource(string url, IDotvvmRequestContext context, out string? mimeType)
{
mimeType = null;
if (DotvvmRoutingMiddleware.FindMatchingRoute(new[] { resourceRoute }, context, out var parameters, out _) == null)

var routeMatchUrl = DotvvmRoutingMiddleware.GetRouteMatchUrl(context);
if (DotvvmRoutingMiddleware.FindExactMatchRoute(new[] { resourceRoute }, routeMatchUrl, out var parameters) == null)
{
return null;
}
Expand Down
4 changes: 4 additions & 0 deletions src/Framework/Framework/Routing/DotvvmRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ public sealed class DotvvmRoute : RouteBase
private List<Func<Dictionary<string, string?>, string>> urlBuilders;
private List<KeyValuePair<string, Func<string, ParameterParseResult>?>> parameters;
private string urlWithoutTypes;
private List<KeyValuePair<string, DotvvmRouteParameterMetadata>> parameterMetadata;

/// <summary>
/// Gets the names of the route parameters in the order in which they appear in the URL.
/// </summary>
public override IEnumerable<string> ParameterNames => parameters.Select(p => p.Key);

public override IEnumerable<KeyValuePair<string, DotvvmRouteParameterMetadata>> ParameterMetadata => parameterMetadata;

public override string UrlWithoutTypes => urlWithoutTypes;


Expand Down Expand Up @@ -77,6 +80,7 @@ private void ParseRouteUrl(DotvvmConfiguration configuration)
routeRegex = result.RouteRegex;
urlBuilders = result.UrlBuilders;
parameters = result.Parameters;
parameterMetadata = result.ParameterMetadata;
urlWithoutTypes = result.UrlWithoutTypes;
}

Expand Down
13 changes: 11 additions & 2 deletions src/Framework/Framework/Routing/DotvvmRouteParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ public UrlParserResult ParseRouteUrl(string url, IDictionary<string, object?> de

var regex = new StringBuilder("^");
var parameters = new List<KeyValuePair<string, Func<string, ParameterParseResult>?>>();
var parameterMetadata = new List<KeyValuePair<string, DotvvmRouteParameterMetadata>>();
var urlBuilders = new List<Func<Dictionary<string, string?>, string>>();
urlBuilders.Add(_ => "~");

void AppendParameterParserResult(UrlParameterParserResult result)
{
regex.Append(result.ParameterRegexPart);
parameters.Add(result.Parameter);
parameterMetadata.Add(new KeyValuePair<string, DotvvmRouteParameterMetadata>(result.Parameter.Key, result.Metadata));
urlBuilders.Add(result.UrlBuilder);
}

Expand Down Expand Up @@ -78,6 +80,7 @@ void AppendParameterParserResult(UrlParameterParserResult result)
RouteRegex = new Regex(regex.ToString(), RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.CultureInvariant),
UrlBuilders = urlBuilders,
Parameters = parameters,
ParameterMetadata = parameterMetadata,
UrlWithoutTypes = string.Concat(urlBuilders.Skip(1).Select(b => b(fakeParameters))).TrimStart('/')
};
}
Expand Down Expand Up @@ -109,6 +112,7 @@ private UrlParameterParserResult ParseParameter(string url, string prefix, ref i
// determine route parameter constraint
IRouteParameterConstraint? type = null;
string? parameter = null;
string? typeName = null;
if (url[index] == ':')
{
startIndex = index + 1;
Expand All @@ -118,7 +122,7 @@ private UrlParameterParserResult ParseParameter(string url, string prefix, ref i
throw new ArgumentException($"The route URL '{url}' is not valid! It contains an unclosed parameter.");
}

var typeName = url.Substring(startIndex, index - startIndex);
typeName = url.Substring(startIndex, index - startIndex);
if (!routeConstraints.ContainsKey(typeName))
{
throw new ArgumentException($"The route parameter constraint '{typeName}' is not valid!");
Expand Down Expand Up @@ -181,7 +185,8 @@ private UrlParameterParserResult ParseParameter(string url, string prefix, ref i
{
ParameterRegexPart = result,
UrlBuilder = urlBuilder,
Parameter = parameterParser
Parameter = parameterParser,
Metadata = new DotvvmRouteParameterMetadata(isOptional, parameter != null ? $"{typeName}({parameter})" : typeName)
};
}

Expand All @@ -190,14 +195,18 @@ private struct UrlParameterParserResult
public string ParameterRegexPart { get; set; }
public Func<Dictionary<string, string?>, string> UrlBuilder { get; set; }
public KeyValuePair<string, Func<string, ParameterParseResult>?> Parameter { get; set; }
public DotvvmRouteParameterMetadata Metadata { get; set; }
}
}

public record DotvvmRouteParameterMetadata(bool IsOptional, string? ConstraintName);

public struct UrlParserResult
{
public Regex RouteRegex { get; set; }
public List<Func<Dictionary<string, string?>, string>> UrlBuilders { get; set; }
public List<KeyValuePair<string, Func<string, ParameterParseResult>?>> Parameters { get; set; }
public string UrlWithoutTypes { get; set; }
public List<KeyValuePair<string, DotvvmRouteParameterMetadata>> ParameterMetadata { get; set; }
}
}
19 changes: 14 additions & 5 deletions src/Framework/Framework/Routing/DotvvmRouteTable.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using DotVVM.Framework.Configuration;
using DotVVM.Framework.Hosting;
Expand All @@ -14,19 +15,22 @@ namespace DotVVM.Framework.Routing
public sealed class DotvvmRouteTable : IEnumerable<RouteBase>
{
private readonly DotvvmConfiguration configuration;
private readonly List<KeyValuePair<string, RouteBase>> list
= new List<KeyValuePair<string, RouteBase>>();
private List<IPartialMatchRouteHandler> partialMatchHandlers = new List<IPartialMatchRouteHandler>();
private readonly List<KeyValuePair<string, RouteBase>> list = new();
private readonly List<IPartialMatchRouteHandler> partialMatchHandlers = new();
private readonly List<IPartialMatchRoute> partialMatchRoutes = new();

private readonly Dictionary<string, RouteBase> dictionary
= new Dictionary<string, RouteBase>(StringComparer.OrdinalIgnoreCase);

private readonly Dictionary<string, DotvvmRouteTable> routeTableGroups
= new Dictionary<string, DotvvmRouteTable>();
private RouteTableGroup? group = null;

private RouteTableGroup? group = null;

public IReadOnlyList<IPartialMatchRouteHandler> PartialMatchHandlers => partialMatchHandlers;

internal IEnumerable<IPartialMatchRoute> PartialMatchRoutes => partialMatchRoutes;

/// <summary>
/// Initializes a new instance of the <see cref="DotvvmRouteTable"/> class.
/// </summary>
Expand Down Expand Up @@ -254,6 +258,11 @@ public void Add(string routeName, RouteBase route)
// The list is used for finding the routes because it keeps the ordering, the dictionary is for checking duplicates
list.Add(new KeyValuePair<string, RouteBase>(routeName, route));
dictionary.Add(routeName, route);

if (route is IPartialMatchRoute partialMatchRoute)
{
partialMatchRoutes.Add(partialMatchRoute);
}
}

public void AddPartialMatchHandler(IPartialMatchRouteHandler handler)
Expand All @@ -267,7 +276,7 @@ public bool Contains(string routeName)
return dictionary.ContainsKey(routeName);
}

public bool TryGetValue(string routeName, out RouteBase? route)
public bool TryGetValue(string routeName, [MaybeNullWhen(false)] out RouteBase route)
{
return dictionary.TryGetValue(routeName, out route);
}
Expand Down
15 changes: 14 additions & 1 deletion src/Framework/Framework/Routing/LocalizedDotvvmRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public sealed class LocalizedDotvvmRoute : RouteBase, IPartialMatchRoute
/// </summary>
public override IEnumerable<string> ParameterNames => GetRouteForCulture(CultureInfo.CurrentUICulture).ParameterNames;

public override IEnumerable<KeyValuePair<string, DotvvmRouteParameterMetadata>> ParameterMetadata => GetRouteForCulture(CultureInfo.CurrentUICulture).ParameterMetadata;

public override string RouteName
{
get
Expand Down Expand Up @@ -61,13 +63,24 @@ public LocalizedDotvvmRoute(string defaultLanguageUrl, LocalizedRouteUrl[] local
throw new ArgumentException("There must be at least one localized route URL!", nameof(localizedUrls));
}

var defaultRoute = new DotvvmRoute(defaultLanguageUrl, virtualPath, defaultValues, presenterFactory, configuration);

var sortedParameters = defaultRoute.ParameterMetadata
.OrderBy(n => n.Key)
.ToArray();

foreach (var localizedUrl in localizedUrls)
{
var localizedRoute = new DotvvmRoute(localizedUrl.RouteUrl, virtualPath, defaultValues, presenterFactory, configuration);
if (!localizedRoute.ParameterMetadata.OrderBy(n => n.Key)
.SequenceEqual(sortedParameters))
{
throw new ArgumentException($"Localized route URL '{localizedUrl.RouteUrl}' must contain the same parameters with equal constraints as the default route URL!", nameof(localizedUrls));
}

localizedRoutes.Add(localizedUrl.CultureIdentifier, localizedRoute);
}

var defaultRoute = new DotvvmRoute(defaultLanguageUrl, virtualPath, defaultValues, presenterFactory, configuration);
localizedRoutes.Add("", defaultRoute);
}

Expand Down
5 changes: 5 additions & 0 deletions src/Framework/Framework/Routing/RouteBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ public RouteBase(string url, string virtualPath, IDictionary<string, object?>? d
/// </summary>
public abstract IEnumerable<string> ParameterNames { get; }

/// <summary>
/// Gets the metadata of the route parameters.
/// </summary>
public abstract IEnumerable<KeyValuePair<string, DotvvmRouteParameterMetadata>> ParameterMetadata { get; }

/// <summary>
/// Determines whether the route matches to the specified URL and extracts the parameter values.
/// </summary>
Expand Down
4 changes: 3 additions & 1 deletion src/Framework/Framework/Routing/RouteTableJsonConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ public ErrorRoute(string? url, string? virtualPath, string? name, IDictionary<st
this.error = error;
}

public override IEnumerable<string> ParameterNames => new string[0];
public override IEnumerable<string> ParameterNames { get; } = new string[0];

public override IEnumerable<KeyValuePair<string, DotvvmRouteParameterMetadata>> ParameterMetadata { get; } = new KeyValuePair<string, DotvvmRouteParameterMetadata>[0];

public override string UrlWithoutTypes => base.Url;

Expand Down
15 changes: 15 additions & 0 deletions src/Tests/Routing/DotvvmRouteTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,21 @@ public void LocalizedDotvvmRoute_IsPartialMatch()
});
}

[DataTestMethod]
[DataRow("product/{id?}/{name:maxLength(5)}", "en/products/{id?}/{name:maxLength(10)}")]
[DataRow("product/{id?}/{name:maxLength(5)}", "en/products/{id?}/{name}")]
[DataRow("product/{id?}/{name:maxLength(5)}", "en/products/{Id:int?}/{name}")]
[DataRow("product/{id?}/{name:maxLength(5)}", "en/products/{abc}")]
[DataRow("product/{id?}/{name:maxLength(5)}", "en/products/{Id?}/{name:maxLength(5)}")]
public void LocalizedDotvvmRoute_RouteConstraintChecks(string defaultRoute, string localizedRoute)
{
Assert.ThrowsException<ArgumentException>(() => {
var route = new LocalizedDotvvmRoute(defaultRoute, new[] {
new LocalizedRouteUrl("en", localizedRoute)
}, "", null, _ => null, configuration);
});
}

[TestMethod]
public void DotvvmRoute_BuildUrl_UrlTwoParameters()
{
Expand Down

0 comments on commit e1951ab

Please sign in to comment.