Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug in route groups #1881

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Framework/Framework/Compilation/DotHtmlFileInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public sealed class DotHtmlFileInfo
public ImmutableArray<CompilationDiagnosticViewModel> Warnings { get; internal set; } = ImmutableArray<CompilationDiagnosticViewModel>.Empty;

/// <summary>Gets or sets the virtual path to the view.</summary>
public string VirtualPath { get; }
public string? VirtualPath { get; }

public string? TagName { get; }
public string? Namespace { get; }
Expand All @@ -25,7 +25,7 @@ public sealed class DotHtmlFileInfo
public ImmutableArray<string>? DefaultValues { get; }
public bool? HasParameters { get; }

public DotHtmlFileInfo(string virtualPath, string? tagName = null, string? nameSpace = null, string? assembly = null, string? tagPrefix = null, string? url = null, string? routeName = null, ImmutableArray<string>? defaultValues = null, bool? hasParameters = null)
public DotHtmlFileInfo(string? virtualPath, string? tagName = null, string? nameSpace = null, string? assembly = null, string? tagPrefix = null, string? url = null, string? routeName = null, ImmutableArray<string>? defaultValues = null, bool? hasParameters = null)
{
VirtualPath = virtualPath;
Status = IsDothtmlFile(virtualPath) ? CompilationState.None : CompilationState.NonCompilable;
Expand All @@ -40,7 +40,7 @@ public DotHtmlFileInfo(string virtualPath, string? tagName = null, string? nameS
HasParameters = hasParameters;
}

private static bool IsDothtmlFile(string virtualPath)
private static bool IsDothtmlFile(string? virtualPath)
{
return !string.IsNullOrWhiteSpace(virtualPath) &&
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public async Task<bool> CompileAll(bool buildInParallel = true, bool forceRecomp
var compilationTaskFactory = (DotHtmlFileInfo t) => () => {
BuildView(t, forceRecompile, out var masterPage);
if (masterPage != null && masterPage.Status == CompilationState.None)
discoveredMasterPages.TryAdd(masterPage.VirtualPath, masterPage);
discoveredMasterPages.TryAdd(masterPage.VirtualPath!, masterPage);
};

var compileTasks = filesToCompile.Select(compilationTaskFactory).ToArray();
Expand Down Expand Up @@ -185,9 +185,9 @@ public bool BuildView(DotHtmlFileInfo file, bool forceRecompile, out DotHtmlFile
{
if (forceRecompile)
// TODO: next major version - add method to interface
(controlBuilderFactory as DefaultControlBuilderFactory)?.InvalidateCache(file.VirtualPath);
(controlBuilderFactory as DefaultControlBuilderFactory)?.InvalidateCache(file.VirtualPath!);

var pageBuilder = controlBuilderFactory.GetControlBuilder(file.VirtualPath);
var pageBuilder = controlBuilderFactory.GetControlBuilder(file.VirtualPath!);

using var scopedServices = dotvvmConfiguration.ServiceProvider.CreateScope(); // dependencies that are configured as scoped cannot be resolved from root service provider
scopedServices.ServiceProvider.GetRequiredService<DotvvmRequestContextStorage>().Context = new ViewCompilationFakeRequestContext(scopedServices.ServiceProvider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using DotVVM.Framework.Configuration;
using DotVVM.Framework.Hosting;
using DotVVM.Framework.Security;
using DotVVM.Framework.Utils;
using Microsoft.CodeAnalysis;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
Expand All @@ -33,7 +34,7 @@ public static ImmutableArray<DotvvmCompilationDiagnostic> CompileAll(
diagnostics.AddRange(CompileNoThrow(configuration, markupControl!));
}

var views = configuration.RouteTable.Select(r => r.VirtualPath).ToImmutableArray();
var views = configuration.RouteTable.Select(r => r.VirtualPath).WhereNotNull().ToImmutableArray();
foreach(var view in views)
{
diagnostics.AddRange(CompileNoThrow(configuration, view));
Expand Down
6 changes: 5 additions & 1 deletion src/Framework/Framework/Configuration/FreezableDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public static void Freeze<K, V>([AllowNull] ref IDictionary<K, V> dict)
}
}
}
sealed class FreezableDictionary<K, V> : IDictionary<K, V>, IReadOnlyCollection<KeyValuePair<K, V>>
sealed class FreezableDictionary<K, V> : IDictionary<K, V>, IReadOnlyCollection<KeyValuePair<K, V>>, IReadOnlyDictionary<K, V>
where K : notnull
{
private readonly Dictionary<K, V> dict;
Expand Down Expand Up @@ -102,5 +102,9 @@ public V this[K index]
public ICollection<K> Keys => ((IDictionary<K, V>)dict).Keys.ToArray();

public ICollection<V> Values => ((IDictionary<K, V>)dict).Values.ToArray();

IEnumerable<K> IReadOnlyDictionary<K, V>.Keys => Keys;

IEnumerable<V> IReadOnlyDictionary<K, V>.Values => Values;
}
}
2 changes: 1 addition & 1 deletion src/Framework/Framework/Controls/RouteLinkHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ private static void EnsureValidBindingType(IBinding binding)

private static Dictionary<string, object?> ComposeNewRouteParameters(RouteLink control, IDotvvmRequestContext context, RouteBase route)
{
var parameters = new Dictionary<string, object?>(route.DefaultValues, StringComparer.OrdinalIgnoreCase);
var parameters = route.CloneDefaultValues();
foreach (var param in context.Parameters!)
{
parameters[param.Key] = param.Value;
Expand Down
4 changes: 3 additions & 1 deletion src/Framework/Framework/Hosting/AggregateMarkupFileLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Linq;
using System.Text;
using DotVVM.Framework.Configuration;
using DotVVM.Framework.Utils;

namespace DotVVM.Framework.Hosting
{
Expand Down Expand Up @@ -39,7 +40,8 @@ public AggregateMarkupFileLoader()
/// </summary>
public string GetMarkupFileVirtualPath(IDotvvmRequestContext context)
{
return context.Route!.VirtualPath;
return context.Route!.VirtualPath
?? throw new Exception($"The route {context.Route.RouteName} must have a non-null virtual path.");
}
}
}
3 changes: 2 additions & 1 deletion src/Framework/Framework/Hosting/DefaultMarkupFileLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public class DefaultMarkupFileLoader : IMarkupFileLoader
/// </summary>
public string GetMarkupFileVirtualPath(IDotvvmRequestContext context)
{
return context.Route!.VirtualPath;
return context.Route!.VirtualPath
?? throw new Exception($"The route {context.Route.RouteName} must have a non-null virtual path.");
}
}
}
3 changes: 2 additions & 1 deletion src/Framework/Framework/Hosting/EmbeddedMarkupFileLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ public class EmbeddedMarkupFileLoader : IMarkupFileLoader
/// </summary>
public string GetMarkupFileVirtualPath(IDotvvmRequestContext context)
{
return context.Route!.VirtualPath;
return context.Route!.VirtualPath
?? throw new Exception($"The route {context.Route.RouteName} must have a non-null virtual path.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public LocalResourceUrlManager(DotvvmConfiguration configuration, IResourceHashS
this.resourceRoute = new DotvvmRoute(
url: $"{HostingConstants.ResourceRouteName}/{{{HashParameterName}}}/{{{NameParameterName}:regex(.*)}}",
virtualPath: "",
name: $"_dotvvm_{nameof(LocalResourceUrlManager)}",
defaultValues: null,
presenterFactory: _ => throw new NotSupportedException(),
configuration: configuration);
Expand Down
7 changes: 2 additions & 5 deletions src/Framework/Framework/Routing/DefaultRouteStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,7 @@ protected virtual RouteBase BuildRoute(RouteStrategyMarkupFileInfo file)
var defaultParameters = GetRouteDefaultParameters(file);
var presenterFactory = GetRoutePresenterFactory(file);

return new DotvvmRoute(url, file.AppRelativePath, defaultParameters, presenterFactory, configuration)
{
RouteName = routeName
};
return new DotvvmRoute(url, file.AppRelativePath, routeName, defaultParameters, presenterFactory, configuration);
}

protected virtual string GetRouteName(RouteStrategyMarkupFileInfo file)
Expand Down Expand Up @@ -162,7 +159,7 @@ private static IEnumerable<string> GetRoutesForFile(string fileName)
protected override IEnumerable<RouteBase> BuildRoutes(RouteStrategyMarkupFileInfo file)
{
return getRouteList(file.AppRelativePath)
.Select(url => new DotvvmRoute(url, file.AppRelativePath, GetRouteDefaultParameters(file), GetRoutePresenterFactory(file), this.configuration) { RouteName = url });
.Select(url => new DotvvmRoute(url, file.AppRelativePath, url, GetRouteDefaultParameters(file), GetRoutePresenterFactory(file), this.configuration));
}
}
}
39 changes: 6 additions & 33 deletions src/Framework/Framework/Routing/DotvvmRoute.cs
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using DotVVM.Framework.Hosting;
using System.Text.RegularExpressions;
using DotVVM.Framework.Configuration;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using DotVVM.Framework.Utils;

namespace DotVVM.Framework.Routing
{
public sealed class DotvvmRoute : RouteBase
{
private Func<IServiceProvider,IDotvvmPresenter> presenterFactory;

private Regex routeRegex;
private List<Func<Dictionary<string, string?>, string>> urlBuilders;
private List<KeyValuePair<string, Func<string, ParameterParseResult>?>> parameters;
Expand All @@ -35,42 +29,26 @@ public sealed class DotvvmRoute : RouteBase
/// <summary>
/// Initializes a new instance of the <see cref="DotvvmRoute"/> class.
/// </summary>
#pragma warning disable CS8618
public DotvvmRoute(string url, string virtualPath, object? defaultValues, Func<IServiceProvider, IDotvvmPresenter> presenterFactory, DotvvmConfiguration configuration)
: base(url, virtualPath, defaultValues)
{
this.presenterFactory = presenterFactory;

ParseRouteUrl(configuration);
}

/// <summary>
/// Initializes a new instance of the <see cref="DotvvmRoute"/> class.
/// </summary>
public DotvvmRoute(string url, string virtualPath, IDictionary<string, object?>? defaultValues, Func<IServiceProvider, IDotvvmPresenter> presenterFactory, DotvvmConfiguration configuration)
: base(url, virtualPath, defaultValues)
public DotvvmRoute(string url, string? virtualPath, string name, object? defaultValues, Func<IServiceProvider, IDotvvmPresenter> presenterFactory, DotvvmConfiguration configuration)
: base(url, virtualPath, name, defaultValues, presenterFactory)
{
this.presenterFactory = presenterFactory;

ParseRouteUrl(configuration);
}

/// <summary>
/// Initializes a new instance of the <see cref="DotvvmRoute"/> class.
/// </summary>
public DotvvmRoute(string url, string virtualPath, string name, IDictionary<string, object?>? defaultValues, Func<IServiceProvider, IDotvvmPresenter> presenterFactory, DotvvmConfiguration configuration)
: base(url, virtualPath, name, defaultValues)
public DotvvmRoute(string url, string? virtualPath, string name, IDictionary<string, object?>? defaultValues, Func<IServiceProvider, IDotvvmPresenter> presenterFactory, DotvvmConfiguration configuration)
: base(url, virtualPath, name, defaultValues, presenterFactory)
{
this.presenterFactory = presenterFactory;

ParseRouteUrl(configuration);
}
#pragma warning restore CS8618


/// <summary>
/// Parses the route URL and extracts the components.
/// </summary>
[MemberNotNull(nameof(routeRegex), nameof(urlBuilders), nameof(parameters), nameof(parameterMetadata), nameof(urlWithoutTypes))]
private void ParseRouteUrl(DotvvmConfiguration configuration)
{
var parser = new DotvvmRouteParser(configuration.RouteConstraints);
Expand Down Expand Up @@ -99,8 +77,7 @@ public override bool IsMatch(string url, [MaybeNullWhen(false)] out IDictionary<
return false;
}

values = new Dictionary<string, object?>(DefaultValues, StringComparer.OrdinalIgnoreCase);

values = CloneDefaultValues();
foreach (var parameter in parameters)
{
var g = match.Groups["param" + parameter.Key];
Expand Down Expand Up @@ -155,10 +132,6 @@ protected internal override string BuildUrlCore(Dictionary<string, object?> valu
}
}

/// <summary>
/// Processes the request.
/// </summary>
public override IDotvvmPresenter GetPresenter(IServiceProvider provider) => presenterFactory(provider);
protected override void Freeze2()
{
// there is no property that would have to be frozen
Expand Down
4 changes: 2 additions & 2 deletions src/Framework/Framework/Routing/DotvvmRouteParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public DotvvmRouteParser(IDictionary<string, IRouteParameterConstraint> routeCon
this.routeConstraints = routeConstrains;
}

public UrlParserResult ParseRouteUrl(string url, IDictionary<string, object?> defaultValues)
public UrlParserResult ParseRouteUrl(string url, IReadOnlyDictionary<string, object?> defaultValues)
{
if (url.StartsWith("/", StringComparison.Ordinal))
throw new ArgumentException("The route URL must not start with '/'!");
Expand Down Expand Up @@ -85,7 +85,7 @@ void AppendParameterParserResult(UrlParameterParserResult result)
};
}

private UrlParameterParserResult ParseParameter(string url, string prefix, ref int index, IDictionary<string, object?> defaultValues)
private UrlParameterParserResult ParseParameter(string url, string prefix, ref int index, IReadOnlyDictionary<string, object?> defaultValues)
{
// find the end of the route parameter name
var startIndex = index;
Expand Down
Loading
Loading