Fixing the error message. The error message for malformed template was too complex listing all

the errors that can happen in one message. I have separated the message in 2 different messages.
 1. When there is a parameter or a literal following the optional parameter.
 2. when optional parameter is preceded by a literal other than period which is not allowed.
This commit is contained in:
Mugdha Kulkarni 2015-02-13 17:15:30 -08:00
parent 1d92960ccd
commit f9a9b80681
4 changed files with 93 additions and 49 deletions

View File

@ -202,22 +202,6 @@ namespace Microsoft.AspNet.Routing
return GetString("TemplateRoute_CannotHaveConsecutiveSeparators");
}
/// <summary>
/// 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 (.).
/// </summary>
internal static string TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator
{
get { return GetString("TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator"); }
}
/// <summary>
/// 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 (.).
/// </summary>
internal static string FormatTemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator()
{
return GetString("TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator");
}
/// <summary>
/// A catch-all parameter cannot be marked optional.
/// </summary>
@ -394,6 +378,38 @@ namespace Microsoft.AspNet.Routing
return GetString("TemplateRoute_UnescapedBrace");
}
/// <summary>
/// In the segment '{0}', the optional parameter '{1}' is preceded by an invalid segment '{2}'. Only a period (.) can precede an optional parameter.
/// </summary>
internal static string TemplateRoute_OptionalParameterCanbBePrecededByPeriod
{
get { return GetString("TemplateRoute_OptionalParameterCanbBePrecededByPeriod"); }
}
/// <summary>
/// In the segment '{0}', the optional parameter '{1}' is preceded by an invalid segment '{2}'. Only a period (.) can precede an optional parameter.
/// </summary>
internal static string FormatTemplateRoute_OptionalParameterCanbBePrecededByPeriod(object p0, object p1, object p2)
{
return string.Format(CultureInfo.CurrentCulture, GetString("TemplateRoute_OptionalParameterCanbBePrecededByPeriod"), p0, p1, p2);
}
/// <summary>
/// An optional parameter must be at the end of the segment. In the segment '{0}', optional parameter '{1}' is followed by '{2}'.
/// </summary>
internal static string TemplateRoute_OptionalParameterHasTobeTheLast
{
get { return GetString("TemplateRoute_OptionalParameterHasTobeTheLast"); }
}
/// <summary>
/// An optional parameter must be at the end of the segment. In the segment '{0}', optional parameter '{1}' is followed by '{2}'.
/// </summary>
internal static string FormatTemplateRoute_OptionalParameterHasTobeTheLast(object p0, object p1, object p2)
{
return string.Format(CultureInfo.CurrentCulture, GetString("TemplateRoute_OptionalParameterHasTobeTheLast"), p0, p1, p2);
}
private static string GetString(string name, params string[] formatterNames)
{
var value = _resourceManager.GetString(name);

View File

@ -153,9 +153,6 @@
<data name="TemplateRoute_CannotHaveConsecutiveSeparators" xml:space="preserve">
<value>The route template separator character '/' cannot appear consecutively. It must be separated by either a parameter or a literal value.</value>
</data>
<data name="TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator" xml:space="preserve">
<value>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 (.).</value>
</data>
<data name="TemplateRoute_CatchAllCannotBeOptional" xml:space="preserve">
<value>A catch-all parameter cannot be marked optional.</value>
</data>
@ -189,4 +186,10 @@
<data name="TemplateRoute_UnescapedBrace" xml:space="preserve">
<value>In a route parameter, '{' and '}' must be escaped with '{{' and '}}'</value>
</data>
<data name="TemplateRoute_OptionalParameterCanbBePrecededByPeriod" xml:space="preserve">
<value>In the segment '{0}', the optional parameter '{1}' is preceded by an invalid segment '{2}'. Only a period (.) can precede an optional parameter.</value>
</data>
<data name="TemplateRoute_OptionalParameterHasTobeTheLast" xml:space="preserve">
<value>An optional parameter must be at the end of the segment. In the segment '{0}', optional parameter '{1}' is followed by '{2}'.</value>
</data>
</root>

View File

@ -340,26 +340,48 @@ namespace Microsoft.AspNet.Routing.Template
if (part.IsParameter && part.IsOptional && segment.Parts.Count > 1)
{
// This is the last part
// This optional parameter is the last part in the segment
if (i == segment.Parts.Count - 1)
{
Debug.Assert(segment.Parts[i - 1].IsLiteral);
// the optional parameter is preceded by a period
if (segment.Parts[i - 1].Text == PeriodString)
{
segment.Parts[i - 1].IsOptionalSeperator = true;
}
else
{
context.Error =
Resources.TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator;
// The optional parameter is preceded by a literal other than period
// Example of error message:
// "In the complex segment {RouteValue}-{param?}, the optional parameter 'param'is preceded
// by an invalid segment "-". Only valid literal to precede an optional parameter is a
// period (.).
context.Error = string.Format(
Resources.TemplateRoute_OptionalParameterCanbBePrecededByPeriod,
segment.DebuggerToString(),
part.Name,
segment.Parts[i - 1].Text);
return false;
}
}
else
{
context.Error =
Resources.TemplateRoute_CanHaveOnlyLastParameterOptional_IfFollowingOptionalSeperator;
// This optional parameter is not the last one in the segment
// Example:
// An optional parameter must be at the end of the segment.In the segment '{RouteValue?})',
// optional parameter 'RouteValue' is followed by ')'
var nextPart = segment.Parts[i + 1];
var invalidPartText = nextPart.IsParameter ? nextPart.Name : nextPart.Text;
context.Error = string.Format(
Resources.TemplateRoute_OptionalParameterHasTobeTheLast,
segment.DebuggerToString(),
segment.Parts[i].Name,
invalidPartText
);
return false;
}
}

View File

@ -528,23 +528,37 @@ namespace Microsoft.AspNet.Routing.Template.Tests
}
[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)
[InlineData("{p1}.{p2?}.{p3}", "p2", ".")]
[InlineData("{p1?}{p2}", "p1", "p2")]
[InlineData("{p1?}{p2?}", "p1", "p2")]
[InlineData("{p1}.{p2?})", "p2", ")")]
[InlineData("{foorb?}-bar-{z}", "foorb", "-bar-")]
public void Parse_ComplexSegment_OptionalParameter_NotTheLastPart(
string template,
string parameter,
string invalid)
{
// Act and Assert
ExceptionAssert.Throws<ArgumentException>(
() => 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");
"An optional parameter must be at the end of the segment. In the segment '" + template +
"', optional parameter '" + parameter + "' is followed by '" + invalid + "'."
+ Environment.NewLine + "Parameter name: routeTemplate");
}
[InlineData("{p1}-{p2?}", "-")]
[InlineData("{p1}..{p2?}", "..")]
[InlineData("..{p2?}", "..")]
[InlineData("{p1}.abc.{p2?}", ".abc.")]
[InlineData("{p1}{p2?}", "p1")]
public void Parse_ComplexSegment_OptionalParametersSeperatedByPeriod_Invalid(string template, string parameter)
{
// Act and Assert
ExceptionAssert.Throws<ArgumentException>(
() => TemplateParser.Parse(template),
"In the complex segment '"+ template +"', the optional parameter 'p2' is preceded by an invalid " +
"segment '" + parameter +"'. Only valid literal to precede an optional parameter is a period (.)." +
Environment.NewLine + "Parameter name: routeTemplate");
}
[Fact]
@ -626,7 +640,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests
[InlineData("{p}}}", "p}")]
[InlineData("{p/}", "p/")]
public void ParseRouteParameter_ThrowsIf_ParameterContainsSpecialCharacters(
string template,
string template,
string parameterName)
{
// Arrange
@ -794,17 +808,6 @@ namespace Microsoft.AspNet.Routing.Template.Tests
"Parameter name: routeTemplate");
}
[Fact]
public void InvalidTemplate_MultiSegmentParameterCannotContainOptionalParameter()
{
ExceptionAssert.Throws<ArgumentException>(
() => TemplateParser.Parse("{foorb?}-bar-{z}"),
"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_CatchAllMarkedOptional()
{