diff --git a/src/Microsoft.AspNet.Routing/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Routing/Properties/Resources.Designer.cs index c6d51b59e9..e9cca0d601 100644 --- a/src/Microsoft.AspNet.Routing/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Routing/Properties/Resources.Designer.cs @@ -205,17 +205,17 @@ namespace Microsoft.AspNet.Routing /// /// A path segment that contains more than one section, such as a literal section or a parameter, cannot contain an optional parameter. /// - internal static string TemplateRoute_CannotHaveOptionalParameterInMultiSegment + internal static string TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator { - get { return GetString("TemplateRoute_CannotHaveOptionalParameterInMultiSegment"); } + get { return GetString("TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator"); } } /// /// A path segment that contains more than one section, such as a literal section or a parameter, cannot contain an optional parameter. /// - internal static string FormatTemplateRoute_CannotHaveOptionalParameterInMultiSegment() + internal static string FormatTemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator() { - return GetString("TemplateRoute_CannotHaveOptionalParameterInMultiSegment"); + return GetString("TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator"); } /// diff --git a/src/Microsoft.AspNet.Routing/Resources.resx b/src/Microsoft.AspNet.Routing/Resources.resx index b4ce8a15c3..4b34eea2d3 100644 --- a/src/Microsoft.AspNet.Routing/Resources.resx +++ b/src/Microsoft.AspNet.Routing/Resources.resx @@ -1,17 +1,17 @@  - @@ -153,8 +153,8 @@ The route template separator character '/' cannot appear consecutively. It must be separated by either a parameter or a literal value. - - A path segment that contains more than one section, such as a literal section or a parameter, cannot contain an optional parameter. + + In a path segment that contains more than one section, such as a literal section or a parameter, there can only be one optional parameter. The optional parameter must be the last parameter in the segment and must be preceded by one single period (.). A catch-all parameter cannot be marked optional. diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs index 155486d73f..1a0adb9409 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs @@ -220,14 +220,29 @@ namespace Microsoft.AspNet.Routing.Template // we won't necessarily add it to the URI we generate. if (!context.Buffer(converted)) { - return null; + return null; } } else { + // If the value is not accepted, it is null or empty value in the + // middle of the segment. We accept this if the parameter is an + // optional parameter and it is preceded by an optional seperator. + // I this case, we need to remove the optional seperator that we + // have added to the URI + // Example: template = {id}.{format?}. parameters: id=5 + // In this case after we have generated "5.", we wont find any value + // for format, so we remove '.' and generate 5. if (!context.Accept(converted)) { - return null; + if (j != 0 && part.IsOptional && segment.Parts[j - 1].IsOptionalSeperator) + { + context.Remove(segment.Parts[j - 1].Text); + } + else + { + return null; + } } } } @@ -472,6 +487,11 @@ namespace Microsoft.AspNet.Routing.Template return true; } + public void Remove(string literal) + { + _uri.Length -= literal.Length; + } + public bool Buffer(string value) { if (string.IsNullOrEmpty(value)) diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs b/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs index f5768fad5e..36ee764815 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs @@ -168,17 +168,63 @@ namespace Microsoft.AspNet.Routing.Template string requestSegment, IReadOnlyDictionary defaults, RouteValueDictionary values) + { + var indexOfLastSegment = routeSegment.Parts.Count - 1; + + // We match the request to the template starting at the rightmost parameter + // If the last segment of template is optional, then request can match the + // template with or without the last parameter. So we start with regular matching, + // but if it doesn't match, we start with next to last parameter. Example: + // Template: {p1}/{p2}.{p3?}. If the request is foo/bar.moo it will match right away + // giving p3 value of moo. But if the request is foo/bar, we start matching from the + // rightmost giving p3 the value of bar, then we end up not matching the segment. + // In this case we start again from p2 to match the request and we succeed giving + // the value bar to p2 + if (routeSegment.Parts[indexOfLastSegment].IsOptional && + routeSegment.Parts[indexOfLastSegment - 1].IsOptionalSeperator) + { + if (MatchComplexSegmentCore(routeSegment, requestSegment, Defaults, values, indexOfLastSegment)) + { + return true; + } + else + { + if (requestSegment.EndsWith(routeSegment.Parts[indexOfLastSegment - 1].Text)) + { + return false; + } + + return MatchComplexSegmentCore( + routeSegment, + requestSegment, + Defaults, + values, + indexOfLastSegment - 2); + } + } + else + { + return MatchComplexSegmentCore(routeSegment, requestSegment, Defaults, values, indexOfLastSegment); + } + } + + private bool MatchComplexSegmentCore(TemplateSegment routeSegment, + string requestSegment, + IReadOnlyDictionary defaults, + RouteValueDictionary values, + int indexOfLastSegmentUsed) { Debug.Assert(routeSegment != null); Debug.Assert(routeSegment.Parts.Count > 1); // Find last literal segment and get its last index in the string var lastIndex = requestSegment.Length; - var indexOfLastSegmentUsed = routeSegment.Parts.Count - 1; - + TemplatePart parameterNeedsValue = null; // Keeps track of a parameter segment that is pending a value TemplatePart lastLiteral = null; // Keeps track of the left-most literal we've encountered + var outValues = new RouteValueDictionary(); + while (indexOfLastSegmentUsed >= 0) { var newLastIndex = lastIndex; @@ -187,7 +233,7 @@ namespace Microsoft.AspNet.Routing.Template if (part.IsParameter) { // Hold on to the parameter so that we can fill it in when we locate the next literal - parameterNeedsValue = part; + parameterNeedsValue = part; } else { @@ -209,10 +255,10 @@ namespace Microsoft.AspNet.Routing.Template var indexOfLiteral = requestSegment.LastIndexOf(part.Text, startIndex, StringComparison.OrdinalIgnoreCase); - if (indexOfLiteral == -1) + if (indexOfLiteral == -1) { // If we couldn't find this literal index, this segment cannot match - return false; + return false; } // If the first subsegment is a literal, it must match at the right-most extent of the request URI. @@ -271,13 +317,14 @@ namespace Microsoft.AspNet.Routing.Template { // If we're here that means we have a segment that contains multiple sub-segments. // For these segments all parameters must have non-empty values. If the parameter - // has an empty value it's not a match. + // has an empty value it's not a match. return false; + } else { // If there's a value in the segment for this parameter, use the subsegment value - values.Add(parameterNeedsValue.Name, parameterValueString); + outValues.Add(parameterNeedsValue.Name, parameterValueString); } parameterNeedsValue = null; @@ -294,7 +341,17 @@ namespace Microsoft.AspNet.Routing.Template // the route "Foo" to the request URI "somethingFoo". Thus we have to check that we parsed the *entire* // request URI in order for it to be a match. // This check is related to the check we do earlier in this function for LiteralSubsegments. - return (lastIndex == 0) || routeSegment.Parts[0].IsParameter; + if (lastIndex == 0 || routeSegment.Parts[0].IsParameter) + { + foreach (var item in outValues) + { + values.Add(item.Key, item.Value); + } + + return true; + } + + return false; } } } diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs b/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs index a7fe485d31..03664f25f5 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNet.Routing.Template private const char EqualsSign = '='; private const char QuestionMark = '?'; private const char Asterisk = '*'; + private const string PeriodString = "."; public static RouteTemplate Parse(string routeTemplate) { @@ -318,18 +319,40 @@ namespace Microsoft.AspNet.Routing.Template } } - // if a segment has multiple parts, then the parameters can't be optional + // if a segment has multiple parts, then only the last one parameter can be optional + // if it is following a optional seperator. for (var i = 0; i < segment.Parts.Count; i++) { var part = segment.Parts[i]; + if (part.IsParameter && part.IsOptional && segment.Parts.Count > 1) { - context.Error = Resources.TemplateRoute_CannotHaveOptionalParameterInMultiSegment; - return false; + // This is the last part + if (i == segment.Parts.Count - 1) + { + Debug.Assert(segment.Parts[i - 1].IsLiteral); + + if (segment.Parts[i - 1].Text == PeriodString) + { + segment.Parts[i - 1].IsOptionalSeperator = true; + } + else + { + context.Error = + Resources.TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator; + return false; + } + } + else + { + context.Error = + Resources.TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator; + return false; + } } } - // A segment cannot containt two consecutive parameters + // A segment cannot contain two consecutive parameters var isLastSegmentParameter = false; for (var i = 0; i < segment.Parts.Count; i++) { diff --git a/src/Microsoft.AspNet.Routing/Template/TemplatePart.cs b/src/Microsoft.AspNet.Routing/Template/TemplatePart.cs index 65099bce1a..dc1195a82d 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplatePart.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplatePart.cs @@ -40,6 +40,7 @@ namespace Microsoft.AspNet.Routing.Template public bool IsLiteral { get; private set; } public bool IsParameter { get; private set; } public bool IsOptional { get; private set; } + public bool IsOptionalSeperator { get; set; } public string Name { get; private set; } public string Text { get; private set; } public object DefaultValue { get; private set; } diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs index 3750eaecc7..34ccb0376d 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs @@ -197,6 +197,131 @@ namespace Microsoft.AspNet.Routing.Template.Tests "language/axx-yy"); } + public static IEnumerable OptionalParamValues + { + get + { + return new object[][] + { + // defaults + // ambient values + // values + new object[] + { + "Test/{val1}/{val2}.{val3?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), + new RouteValueDictionary(new {val3 = "someval3"}), + new RouteValueDictionary(new {val3 = "someval3"}), + "Test/someval1/someval2.someval3", + }, + new object[] + { + "Test/{val1}/{val2}.{val3?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), + new RouteValueDictionary(new {val3 = "someval3a"}), + new RouteValueDictionary(new {val3 = "someval3v"}), + "Test/someval1/someval2.someval3v", + }, + new object[] + { + "Test/{val1}/{val2}.{val3?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), + new RouteValueDictionary(new {val3 = "someval3a"}), + new RouteValueDictionary(), + "Test/someval1/someval2.someval3a", + }, + new object[] + { + "Test/{val1}/{val2}.{val3?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), + new RouteValueDictionary(), + new RouteValueDictionary(new {val3 = "someval3v"}), + "Test/someval1/someval2.someval3v", + }, + new object[] + { + "Test/{val1}/{val2}.{val3?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), + new RouteValueDictionary(), + new RouteValueDictionary(), + "Test/someval1/someval2", + }, + new object[] + { + "Test/{val1}.{val2}.{val3}.{val4?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), + new RouteValueDictionary(), + new RouteValueDictionary(new {val4 = "someval4", val3 = "someval3" }), + "Test/someval1.someval2.someval3.someval4", + }, + new object[] + { + "Test/{val1}.{val2}.{val3}.{val4?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), + new RouteValueDictionary(), + new RouteValueDictionary(new {val3 = "someval3" }), + "Test/someval1.someval2.someval3", + }, + new object[] + { + "Test/.{val2?}", + new RouteValueDictionary(new { }), + new RouteValueDictionary(), + new RouteValueDictionary(new {val2 = "someval2" }), + "Test/.someval2", + }, + new object[] + { + "Test/{val1}.{val2}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), + new RouteValueDictionary(), + new RouteValueDictionary(new {val3 = "someval3" }), + "Test/someval1.someval2?val3=someval3", + }, + }; + } + } + + [Theory] + [MemberData("OptionalParamValues")] + public void GetVirtualPathWithMultiSegmentWithOptionalParam( + string template, + IReadOnlyDictionary defaults, + IDictionary ambientValues, + IDictionary values, + string expected) + { + // Arrange + var binder = new TemplateBinder( + TemplateParser.Parse(template), + defaults); + + // Act & Assert + var result = binder.GetValues(ambientValues: ambientValues, values: values); + if (result == null) + { + if (expected == null) + { + return; + } + else + { + Assert.NotNull(result); + } + } + + var boundTemplate = binder.BindValues(result.AcceptedValues); + if (expected == null) + { + Assert.Null(boundTemplate); + } + else + { + Assert.NotNull(boundTemplate); + Assert.Equal(expected, boundTemplate); + } + } + [Fact] public void GetVirtualPathWithMultiSegmentParamsOnNeitherEndMatches() { diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateMatcherTests.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateMatcherTests.cs index 30073881a9..5aa0bfc9ad 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateMatcherTests.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateMatcherTests.cs @@ -101,6 +101,103 @@ namespace Microsoft.AspNet.Routing.Template.Tests Assert.Equal("default p2", rd["p2"]); } + [Theory] + [InlineData("moo/{p1}.{p2?}", "moo/foo.bar", "foo", "bar")] + [InlineData("moo/{p1?}", "moo/foo", "foo", null)] + [InlineData("moo/{p1?}", "moo", null, null)] + [InlineData("moo/{p1}.{p2?}", "moo/foo", "foo", null)] + [InlineData("moo/{p1}.{p2?}", "moo/foo..bar", "foo.", "bar")] + [InlineData("moo/{p1}.{p2?}", "moo/foo.moo.bar", "foo.moo", "bar")] + [InlineData("moo/{p1}.{p2}", "moo/foo.bar", "foo", "bar")] + [InlineData("moo/foo.{p1}.{p2?}", "moo/foo.moo.bar", "moo", "bar")] + [InlineData("moo/foo.{p1}.{p2?}", "moo/foo.moo", "moo", null)] + [InlineData("moo/.{p2?}", "moo/.foo", null, "foo")] + [InlineData("moo/.{p2?}", "moo", null, null)] + [InlineData("moo/{p1}.{p2?}", "moo/....", "..", ".")] + [InlineData("moo/{p1}.{p2?}", "moo/.bar", ".bar", null)] + public void MatchRoute_OptionalParameter_FollowedByPeriod_Valid( + string template, + string path, + string p1, + string p2) + { + // Arrange + var matcher = CreateMatcher(template); + + // Act + var rd = matcher.Match(path); + + // Assert + if (p1 != null) + { + Assert.Equal(p1, rd["p1"]); + } + if (p2 != null) + { + Assert.Equal(p2, rd["p2"]); + } + } + + [Theory] + [InlineData("moo/{p1}.{p2}.{p3?}", "moo/foo.moo.bar", "foo", "moo", "bar")] + [InlineData("moo/{p1}.{p2}.{p3?}", "moo/foo.moo", "foo", "moo", null)] + [InlineData("moo/{p1}.{p2}.{p3}.{p4?}", "moo/foo.moo.bar", "foo", "moo", "bar")] + [InlineData("{p1}.{p2?}/{p3}", "foo.moo/bar", "foo", "moo", "bar")] + [InlineData("{p1}.{p2?}/{p3}", "foo/bar", "foo", null, "bar")] + [InlineData("{p1}.{p2?}/{p3}", ".foo/bar", ".foo", null, "bar")] + public void MatchRoute_OptionalParameter_FollowedByPeriod_3Parameters_Valid( + string template, + string path, + string p1, + string p2, + string p3) + { + // Arrange + var matcher = CreateMatcher(template); + + // Act + var rd = matcher.Match(path); + + // Assert + + Assert.Equal(p1, rd["p1"]); + + if (p2 != null) + { + Assert.Equal(p2, rd["p2"]); + } + + if (p3 != null) + { + Assert.Equal(p3, rd["p3"]); + } + } + + [Theory] + [InlineData("moo/{p1}.{p2?}", "moo/foo.")] + [InlineData("moo/{p1}.{p2?}", "moo/.")] + [InlineData("moo/{p1}.{p2}", "foo.")] + [InlineData("moo/{p1}.{p2}", "foo")] + [InlineData("moo/{p1}.{p2}.{p3?}", "moo/foo.moo.")] + [InlineData("moo/foo.{p2}.{p3?}", "moo/bar.foo.moo")] + [InlineData("moo/foo.{p2}.{p3?}", "moo/kungfoo.moo.bar")] + [InlineData("moo/foo.{p2}.{p3?}", "moo/kungfoo.moo")] + [InlineData("moo/{p1}.{p2}.{p3?}", "moo/foo")] + [InlineData("{p1}.{p2?}/{p3}", "foo./bar")] + [InlineData("moo/.{p2?}", "moo/.")] + [InlineData("{p1}.{p2}/{p3}", ".foo/bar")] + public void MatchRoute_OptionalParameter_FollowedByPeriod_Invalid(string template, string path) + { + // Arrange + var matcher = CreateMatcher(template); + + // Act + var rd = matcher.Match(path); + + // Assert + Assert.Null(rd); + } + [Fact] public void MatchRouteWithOnlyLiterals() { @@ -183,7 +280,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests // Assert Assert.Null(rd); } - + [Fact] public void NoMatchLongerUrl() { diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateParserTests.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateParserTests.cs index c10847bbf4..181a586b6f 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateParserTests.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateParserTests.cs @@ -40,7 +40,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests var expected = new RouteTemplate(new List()); expected.Segments.Add(new TemplateSegment()); - expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p", false, false, defaultValue: null, inlineConstraints: null)); + expected.Segments[0].Parts.Add( + TemplatePart.CreateParameter("p", false, false, defaultValue: null, inlineConstraints: null)); expected.Parameters.Add(expected.Segments[0].Parts[0]); // Act @@ -58,7 +59,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests var expected = new RouteTemplate(new List()); expected.Segments.Add(new TemplateSegment()); - expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p", false, true, defaultValue: null, inlineConstraints: null)); + expected.Segments[0].Parts.Add( + TemplatePart.CreateParameter("p", false, true, defaultValue: null, inlineConstraints: null)); expected.Parameters.Add(expected.Segments[0].Parts[0]); // Act @@ -227,12 +229,275 @@ namespace Microsoft.AspNet.Routing.Template.Tests Assert.Equal(expected, actual, new TemplateEqualityComparer()); } + [Fact] + public void Parse_ComplexSegment_OptionalParameterFollowingPeriod() + { + // Arrange + var template = "{p1}.{p2?}"; + + var expected = new RouteTemplate(new List()); + expected.Segments.Add(new TemplateSegment()); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p1", + false, + false, + defaultValue: null, + inlineConstraints: null)); + expected.Segments[0].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p2", + false, + true, + defaultValue: null, + inlineConstraints: null)); + + expected.Parameters.Add(expected.Segments[0].Parts[0]); + expected.Parameters.Add(expected.Segments[0].Parts[2]); + + // Act + var actual = TemplateParser.Parse(template); + + // Assert + Assert.Equal(expected, actual, new TemplateEqualityComparer()); + } + + [Fact] + public void Parse_ComplexSegment_ParametersFollowingPeriod() + { + // Arrange + var template = "{p1}.{p2}"; + + var expected = new RouteTemplate(new List()); + expected.Segments.Add(new TemplateSegment()); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p1", + false, + false, + defaultValue: null, + inlineConstraints: null)); + expected.Segments[0].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p2", + false, + false, + defaultValue: null, + inlineConstraints: null)); + + expected.Parameters.Add(expected.Segments[0].Parts[0]); + expected.Parameters.Add(expected.Segments[0].Parts[2]); + + // Act + var actual = TemplateParser.Parse(template); + + // Assert + Assert.Equal(expected, actual, new TemplateEqualityComparer()); + } + + [Fact] + public void Parse_ComplexSegment_OptionalParameterFollowingPeriod_ThreeParameters() + { + // Arrange + var template = "{p1}.{p2}.{p3?}"; + + var expected = new RouteTemplate(new List()); + expected.Segments.Add(new TemplateSegment()); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p1", + false, + false, + defaultValue: null, + inlineConstraints: null)); + expected.Segments[0].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p2", + false, + false, + defaultValue: null, + inlineConstraints: null)); + + expected.Segments[0].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p3", + false, + true, + defaultValue: null, + inlineConstraints: null)); + + + expected.Parameters.Add(expected.Segments[0].Parts[0]); + expected.Parameters.Add(expected.Segments[0].Parts[2]); + expected.Parameters.Add(expected.Segments[0].Parts[4]); + + // Act + var actual = TemplateParser.Parse(template); + + // Assert + Assert.Equal(expected, actual, new TemplateEqualityComparer()); + } + + [Fact] + public void Parse_ComplexSegment_ThreeParametersSeperatedByPeriod() + { + // Arrange + var template = "{p1}.{p2}.{p3}"; + + var expected = new RouteTemplate(new List()); + expected.Segments.Add(new TemplateSegment()); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p1", + false, + false, + defaultValue: null, + inlineConstraints: null)); + expected.Segments[0].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p2", + false, + false, + defaultValue: null, + inlineConstraints: null)); + + expected.Segments[0].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p3", + false, + false, + defaultValue: null, + inlineConstraints: null)); + + + expected.Parameters.Add(expected.Segments[0].Parts[0]); + expected.Parameters.Add(expected.Segments[0].Parts[2]); + expected.Parameters.Add(expected.Segments[0].Parts[4]); + + // Act + var actual = TemplateParser.Parse(template); + + // Assert + Assert.Equal(expected, actual, new TemplateEqualityComparer()); + } + + [Fact] + public void Parse_ComplexSegment_OptionalParameterFollowingPeriod_MiddleSegment() + { + // Arrange + var template = "{p1}.{p2?}/{p3}"; + + var expected = new RouteTemplate(new List()); + expected.Segments.Add(new TemplateSegment()); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p1", + false, + false, + defaultValue: null, + inlineConstraints: null)); + expected.Segments[0].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p2", + false, + true, + defaultValue: null, + inlineConstraints: null)); + + expected.Parameters.Add(expected.Segments[0].Parts[0]); + expected.Parameters.Add(expected.Segments[0].Parts[2]); + + expected.Segments.Add(new TemplateSegment()); + expected.Segments[1].Parts.Add(TemplatePart.CreateParameter("p3", + false, + false, + null, + null)); + expected.Parameters.Add(expected.Segments[1].Parts[0]); + // Act + var actual = TemplateParser.Parse(template); + + // Assert + Assert.Equal(expected, actual, new TemplateEqualityComparer()); + } + + public void Parse_ComplexSegment_OptionalParameterFollowingPeriod_LastSegment() + { + // Arrange + var template = "{p1}/{p2}.{p3?}"; + + var expected = new RouteTemplate(new List()); + expected.Segments.Add(new TemplateSegment()); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p1", + false, + false, + defaultValue: null, + inlineConstraints: null)); + + expected.Segments.Add(new TemplateSegment()); + expected.Segments[1].Parts.Add(TemplatePart.CreateParameter("p2", + false, + true, + defaultValue: null, + inlineConstraints: null)); + expected.Segments[1].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[1].Parts.Add(TemplatePart.CreateParameter("p3", + false, + true, + null, + null)); + expected.Parameters.Add(expected.Segments[0].Parts[0]); + expected.Parameters.Add(expected.Segments[1].Parts[0]); + expected.Parameters.Add(expected.Segments[1].Parts[2]); + + // Act + var actual = TemplateParser.Parse(template); + + // Assert + Assert.Equal(expected, actual, new TemplateEqualityComparer()); + } + + [Fact] + public void Parse_ComplexSegment_OptionalParameterFollowingPeriod_PeriodAfterSlash() + { + // Arrange + var template = "{p2}/.{p3?}"; + + var expected = new RouteTemplate(new List()); + expected.Segments.Add(new TemplateSegment()); + expected.Segments[0].Parts.Add(TemplatePart.CreateParameter("p2", + false, + false, + defaultValue: null, + inlineConstraints: null)); + + expected.Segments.Add(new TemplateSegment()); + expected.Segments[1].Parts.Add(TemplatePart.CreateLiteral(".")); + expected.Segments[1].Parts.Add(TemplatePart.CreateParameter("p3", + false, + true, + null, + null)); + expected.Parameters.Add(expected.Segments[0].Parts[0]); + expected.Parameters.Add(expected.Segments[1].Parts[1]); + + // Act + var actual = TemplateParser.Parse(template); + + // Assert + Assert.Equal(expected, actual, new TemplateEqualityComparer()); + } + + [Theory] + [InlineData("{p1?}.{p2?}")] + [InlineData("{p1}.{p2?}.{p3}")] + [InlineData("{p1?}.{p2}/{p3}")] + [InlineData("{p3}.{p1?}.{p2?}")] + [InlineData("{p1}-{p2?}")] + [InlineData("{p1}..{p2?}")] + [InlineData("..{p2?}")] + [InlineData("{p1}.abc.{p2?}")] + public void Parse_ComplexSegment_OptionalParametersSeperatedByPeriod_Invalid(string template) + { + // Act and Assert + ExceptionAssert.Throws( + () => TemplateParser.Parse(template), + "In a path segment that contains more than one section, such as a literal section or a parameter, " + + "there can only be one optional parameter. The optional parameter must be the last parameter in the " + + "segment and must be preceded by one single period (.)." + Environment.NewLine + + "Parameter name: routeTemplate"); + } + [Fact] public void InvalidTemplate_WithRepeatedParameter() { var ex = ExceptionAssert.Throws( () => TemplateParser.Parse("{Controller}.mvc/{id}/{controller}"), - "The route parameter name 'controller' appears more than one time in the route template." + Environment.NewLine + "Parameter name: routeTemplate"); + "The route parameter name 'controller' appears more than one time in the route template." + + Environment.NewLine + "Parameter name: routeTemplate"); } [Theory] @@ -246,7 +511,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse(template), - @"There is an incomplete parameter in the route template. Check that each '{' character has a matching '}' character." + Environment.NewLine + + @"There is an incomplete parameter in the route template. Check that each '{' character has a " + + "matching '}' character." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -255,7 +521,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("123{a}abc{*moo}"), - "A path segment that contains more than one section, such as a literal section or a parameter, cannot contain a catch-all parameter." + Environment.NewLine + + "A path segment that contains more than one section, such as a literal section or a parameter, " + + "cannot contain a catch-all parameter." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -264,7 +531,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("{*p1}/{*p2}"), - "A catch-all parameter can only appear as the last segment of the route template." + Environment.NewLine + + "A catch-all parameter can only appear as the last segment of the route template." + + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -273,7 +541,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("{*p1}abc{*p2}"), - "A path segment that contains more than one section, such as a literal section or a parameter, cannot contain a catch-all parameter." + Environment.NewLine + + "A path segment that contains more than one section, such as a literal section or a parameter, " + + "cannot contain a catch-all parameter." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -315,7 +584,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("foo/{{p1}"), - "There is an incomplete parameter in the route template. Check that each '{' character has a matching '}' character." + Environment.NewLine + + "There is an incomplete parameter in the route template. Check that each '{' character has a " + + "matching '}' character." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -324,7 +594,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("foo/{p1}}"), - "There is an incomplete parameter in the route template. Check that each '{' character has a matching '}' character." + Environment.NewLine + + "There is an incomplete parameter in the route template. Check that each '{' character has a " + + "matching '}' character." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -333,7 +604,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("{aaa}/{AAA}"), - "The route parameter name 'AAA' appears more than one time in the route template." + Environment.NewLine + + "The route parameter name 'AAA' appears more than one time in the route template." + + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -342,7 +614,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("{aaa}/{*AAA}"), - "The route parameter name 'AAA' appears more than one time in the route template." + Environment.NewLine + + "The route parameter name 'AAA' appears more than one time in the route template." + + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -351,7 +624,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("{a}/{aa}a}/{z}"), - "There is an incomplete parameter in the route template. Check that each '{' character has a matching '}' character." + Environment.NewLine + + "There is an incomplete parameter in the route template. Check that each '{' character has a " + + "matching '}' character." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -396,7 +670,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("{a}//{z}"), - "The route template separator character '/' cannot appear consecutively. It must be separated by either a parameter or a literal value." + Environment.NewLine + + "The route template separator character '/' cannot appear consecutively. It must be separated by " + + "either a parameter or a literal value." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -405,7 +680,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("foo/{p1}/{*p2}/{p3}"), - "A catch-all parameter can only appear as the last segment of the route template." + Environment.NewLine + + "A catch-all parameter can only appear as the last segment of the route template." + + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -414,7 +690,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("foo/aa{p1}{p2}"), - "A path segment cannot contain two consecutive parameters. They must be separated by a '/' or by a literal string." + Environment.NewLine + + "A path segment cannot contain two consecutive parameters. They must be separated by a '/' or by " + + "a literal string." + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -441,7 +718,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("foor?bar"), - "The literal section 'foor?bar' is invalid. Literal sections cannot contain the '?' character." + Environment.NewLine + + "The literal section 'foor?bar' is invalid. Literal sections cannot contain the '?' character." + + Environment.NewLine + "Parameter name: routeTemplate"); } @@ -462,7 +740,9 @@ namespace Microsoft.AspNet.Routing.Template.Tests { ExceptionAssert.Throws( () => TemplateParser.Parse("{foorb?}-bar-{z}"), - "A path segment that contains more than one section, such as a literal section or a parameter, cannot contain an optional parameter." + Environment.NewLine + + "In a path segment that contains more than one section, such as a literal section or a parameter, " + + "there can only be one optional parameter. The optional parameter must be the last parameter in " + + "the segment and must be preceded by one single period (.)." + Environment.NewLine + "Parameter name: routeTemplate"); } diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs index 5baaf3b9c8..67b3d80d85 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs @@ -223,83 +223,7 @@ namespace Microsoft.AspNet.Routing.Template Assert.NotSame(originalDataTokens, context.RouteData.DataTokens); Assert.NotSame(route.DataTokens, context.RouteData.DataTokens); } - - [Fact] - public async Task RouteAsync_InlineConstrait_OptionalParameter() - { - // Arrange - var template = "{controller}/{action}/{id:int?}"; - - var context = CreateRouteContext("/Home/Index/5"); - - IDictionary routeValues = null; - var mockTarget = new Mock(MockBehavior.Strict); - mockTarget - .Setup(s => s.RouteAsync(It.IsAny())) - .Callback(ctx => - { - routeValues = ctx.RouteData.Values; - ctx.IsHandled = true; - }) - .Returns(Task.FromResult(true)); - - var route = new TemplateRoute( - mockTarget.Object, - template, - defaults: null, - constraints: null, - dataTokens: null, - inlineConstraintResolver: _inlineConstraintResolver); - - // Act - await route.RouteAsync(context); - - // Assert - Assert.NotNull(routeValues); - Assert.True(routeValues.ContainsKey("id")); - Assert.Equal("5", routeValues["id"]); - - Assert.True(context.RouteData.Values.ContainsKey("id")); - Assert.Equal("5", context.RouteData.Values["id"]); - } - - [Fact] - public async Task RouteAsync_InlineConstrait_OptionalParameter_NotPresent() - { - // Arrange - var template = "{controller}/{action}/{id:int?}"; - - var context = CreateRouteContext("/Home/Index"); - - IDictionary routeValues = null; - var mockTarget = new Mock(MockBehavior.Strict); - mockTarget - .Setup(s => s.RouteAsync(It.IsAny())) - .Callback(ctx => - { - routeValues = ctx.RouteData.Values; - ctx.IsHandled = true; - }) - .Returns(Task.FromResult(true)); - - var route = new TemplateRoute( - mockTarget.Object, - template, - defaults: null, - constraints: null, - dataTokens: null, - inlineConstraintResolver: _inlineConstraintResolver); - - // Act - await route.RouteAsync(context); - - // Assert - Assert.NotNull(routeValues); - Assert.False(routeValues.ContainsKey("id")); - - Assert.False(context.RouteData.Values.ContainsKey("id")); - } - + [Fact] public async Task RouteAsync_MergesExistingRouteData_PassedToConstraint() { @@ -862,6 +786,73 @@ namespace Microsoft.AspNet.Routing.Template Assert.Null(context.RouteData.Values["1controller"]); } + [Fact] + public async Task Match_Success_OptionalParameter_ValueProvided() + { + // Arrange + var route = CreateRoute("{controller}/{action}.{format?}", new { action = "Index" }); + var context = CreateRouteContext("/Home/Create.xml"); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.True(context.IsHandled); + Assert.Equal(3, context.RouteData.Values.Count); + Assert.Equal("Home", context.RouteData.Values["controller"]); + Assert.Equal("Create", context.RouteData.Values["action"]); + Assert.Equal("xml", context.RouteData.Values["format"]); + } + + [Fact] + public async Task Match_Success_OptionalParameter_ValueNotProvided() + { + // Arrange + var route = CreateRoute("{controller}/{action}.{format?}", new { action = "Index" }); + var context = CreateRouteContext("/Home/Create"); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.True(context.IsHandled); + Assert.Equal(2, context.RouteData.Values.Count); + Assert.Equal("Home", context.RouteData.Values["controller"]); + Assert.Equal("Create", context.RouteData.Values["action"]); + } + + [Fact] + public async Task Match_Success_OptionalParameter_DefaultValue() + { + // Arrange + var route = CreateRoute("{controller}/{action}.{format?}", new { action = "Index", format = "xml" }); + var context = CreateRouteContext("/Home/Create"); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.True(context.IsHandled); + Assert.Equal(3, context.RouteData.Values.Count); + Assert.Equal("Home", context.RouteData.Values["controller"]); + Assert.Equal("Create", context.RouteData.Values["action"]); + Assert.Equal("xml", context.RouteData.Values["format"]); + } + + [Fact] + public async Task Match_Success_OptionalParameter_EndsWithDot() + { + // Arrange + var route = CreateRoute("{controller}/{action}.{format?}", new { action = "Index" }); + var context = CreateRouteContext("/Home/Create."); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.False(context.IsHandled); + } + private static RouteContext CreateRouteContext(string requestPath, ILoggerFactory factory = null) { if (factory == null) @@ -1070,7 +1061,9 @@ namespace Microsoft.AspNet.Routing.Template .Returns(null); var route = CreateRoute(target.Object, "{controller}/{action}"); - var context = CreateVirtualPathContext(new { action = "Store" }, new { Controller = "Home", action = "Blog" }); + var context = CreateVirtualPathContext( + new { action = "Store" }, + new { Controller = "Home", action = "Blog" }); var expectedValues = new RouteValueDictionary(new { controller = "Home", action = "Store" }); @@ -1094,9 +1087,11 @@ namespace Microsoft.AspNet.Routing.Template .Returns(null); var route = CreateRoute(target.Object, "Admin/{controller}/{action}", new { area = "Admin" }); - var context = CreateVirtualPathContext(new { action = "Store" }, new { Controller = "Home", action = "Blog" }); + var context = CreateVirtualPathContext( + new { action = "Store" }, new { Controller = "Home", action = "Blog" }); - var expectedValues = new RouteValueDictionary(new { controller = "Home", action = "Store", area = "Admin" }); + var expectedValues = new RouteValueDictionary( + new { controller = "Home", action = "Store", area = "Admin" }); // Act var path = route.GetVirtualPath(context); @@ -1118,7 +1113,8 @@ namespace Microsoft.AspNet.Routing.Template .Returns(null); var route = CreateRoute(target.Object, "{controller}/{action}"); - var context = CreateVirtualPathContext(new { action = "Store", id = 5 }, new { Controller = "Home", action = "Blog" }); + var context = CreateVirtualPathContext( + new { action = "Store", id = 5 }, new { Controller = "Home", action = "Blog" }); var expectedValues = new RouteValueDictionary(new { controller = "Home", action = "Store" }); @@ -1352,7 +1348,166 @@ namespace Microsoft.AspNet.Routing.Template Assert.Equal("Home/Index/products", path); } - + [Fact] + public void GetVirtualPath_OptionalParameter_ParameterPresentInValues() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/{name}.{format?}", + defaults: null, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home", name = "products", format = "xml"}); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index/products.xml", path); + } + + [Fact] + public void GetVirtualPath_OptionalParameter_ParameterNotPresentInValues() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/{name}.{format?}", + defaults: null, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home", name = "products"}); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index/products", path); + } + + [Fact] + public void GetVirtualPath_OptionalParameter_ParameterPresentInValuesAndDefaults() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/{name}.{format?}", + defaults: new { format = "json" }, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home", name = "products" , format = "xml"}); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index/products.xml", path); + } + + [Fact] + public void GetVirtualPath_OptionalParameter_ParameterNotPresentInValues_PresentInDefaults() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/{name}.{format?}", + defaults: new { format = "json" }, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home", name = "products"}); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index/products", path); + } + + [Fact] + public void GetVirtualPath_OptionalParameter_ParameterNotPresentInTemplate_PresentInValues() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/{name}", + defaults: null, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home", name = "products", format = "json" }); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index/products?format=json", path); + } + + [Fact] + public void GetVirtualPath_OptionalParameter_FollowedByDotAfterSlash_ParameterPresent() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/.{name?}", + defaults: null, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home", name = "products"}); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index/.products", path); + } + + [Fact] + public void GetVirtualPath_OptionalParameter_FollowedByDotAfterSlash_ParameterNotPresent() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/.{name?}", + defaults: null, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home"}); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index/", path); + } + + [Fact] + public void GetVirtualPath_OptionalParameter_InSimpleSegment() + { + // Arrange + var route = CreateRoute( + template: "{controller}/{action}/{name?}", + defaults: null, + accept: true, + constraints: null); + + var context = CreateVirtualPathContext( + values: new { action = "Index", controller = "Home"}); + + // Act + var path = route.GetVirtualPath(context); + + // Assert + Assert.Equal("Home/Index", path); + } + private static VirtualPathContext CreateVirtualPathContext(object values) { return CreateVirtualPathContext(new RouteValueDictionary(values), null); @@ -1363,7 +1518,9 @@ namespace Microsoft.AspNet.Routing.Template return CreateVirtualPathContext(new RouteValueDictionary(values), new RouteValueDictionary(ambientValues)); } - private static VirtualPathContext CreateVirtualPathContext(IDictionary values, IDictionary ambientValues) + private static VirtualPathContext CreateVirtualPathContext( + IDictionary values, + IDictionary ambientValues) { var context = new Mock(MockBehavior.Strict); context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory)))