From cf16d6cba79974f716c1f6aa01a02141db5f0b65 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 26 Mar 2014 18:44:57 -0700 Subject: [PATCH] Small refactor of TemplateBinder We'll need to access the accepted values to do proper link generation, so separating this process out into 2 parts. Also moving defaults into the TemplateBinder because they are conceptually part of the route, not part of the request. I'll do the same for TemplateMatcher soon, but it's a big change and worth separating. --- .../Template/TemplateBinder.cs | 140 ++++++------------ .../Template/TemplateRoute.cs | 13 +- .../Template/TemplateBinderTests.cs | 38 ++++- .../project.json | 2 +- 4 files changed, 82 insertions(+), 111 deletions(-) diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs index b712933cf5..d347e91ac8 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs @@ -12,59 +12,38 @@ namespace Microsoft.AspNet.Routing.Template { public class TemplateBinder { - public TemplateBinder(Template template) + private readonly IDictionary _defaults; + private readonly Template _template; + + public TemplateBinder(Template template, IDictionary defaults) { if (template == null) { throw new ArgumentNullException("template"); } - Template = template; - } - - public Template Template { get; private set; } - - public string Bind(IDictionary defaults, IDictionary ambientValues, IDictionary values) - { - if (values == null) - { - throw new ArgumentNullException("values"); - } - - var context = GetAcceptedValues(defaults, ambientValues, values); - if (context == null) - { - // We couldn't get values for all the required parameters - return null; - } - - return BindValues(context); + _template = template; + _defaults = defaults; } // Step 1: Get the list of values we're going to try to use to match and generate this URI - private TemplateBindingContext GetAcceptedValues(IDictionary defaults, IDictionary ambientValues, IDictionary values) + public IDictionary GetAcceptedValues(IDictionary ambientValues, IDictionary values) { - Contract.Assert(values != null); - - var context = new TemplateBindingContext(defaults, values); + var context = new TemplateBindingContext(_defaults, values); // Find out which entries in the URI are valid for the URI we want to generate. // If the URI had ordered parameters a="1", b="2", c="3" and the new values // specified that b="9", then we need to invalidate everything after it. The new // values should then be a="1", b="9", c=. - for (var i = 0; i < Template.Parameters.Count; i++) + for (var i = 0; i < _template.Parameters.Count; i++) { - var parameter = Template.Parameters[i]; + var parameter = _template.Parameters[i]; // If it's a parameter subsegment, examine the current value to see if it matches the new value var parameterName = parameter.Name; object newParameterValue; var hasNewParameterValue = values.TryGetValue(parameterName, out newParameterValue); - if (hasNewParameterValue) - { - context.Use(parameterName); - } object currentParameterValue = null; var hasCurrentParameterValue = ambientValues != null && ambientValues.TryGetValue(parameterName, out currentParameterValue); @@ -104,23 +83,10 @@ namespace Microsoft.AspNet.Routing.Template } } - // Add all current values that aren't in the URI at all - if (ambientValues != null) - { - foreach (var kvp in ambientValues) - { - var parameter = GetParameter(kvp.Key); - if (parameter == null) - { - context.Accept(kvp.Key, kvp.Value); - } - } - } - // Accept all remaining default values if they match a required parameter - for (int i = 0; i < Template.Parameters.Count; i++) + for (int i = 0; i < _template.Parameters.Count; i++) { - var parameter = Template.Parameters[i]; + var parameter = _template.Parameters[i]; if (parameter.IsOptional || parameter.IsCatchAll) { continue; @@ -136,9 +102,9 @@ namespace Microsoft.AspNet.Routing.Template } // Validate that all required parameters have a value. - for (var i = 0; i < Template.Parameters.Count; i++) + for (var i = 0; i < _template.Parameters.Count; i++) { - var parameter = Template.Parameters[i]; + var parameter = _template.Parameters[i]; if (parameter.IsOptional || parameter.IsCatchAll) { continue; @@ -166,11 +132,7 @@ namespace Microsoft.AspNet.Routing.Template object value; if (values.TryGetValue(filter.Key, out value)) { - if (RoutePartsEqual(value, filter.Value)) - { - context.Use(filter.Key); - } - else + if (!RoutePartsEqual(value, filter.Value)) { // If there is a non-parameterized value in the route and there is a // new value for it and it doesn't match, this route won't match. @@ -180,20 +142,20 @@ namespace Microsoft.AspNet.Routing.Template } } - return context; + return context.AcceptedValues; } // Step 2: If the route is a match generate the appropriate URI - private string BindValues(TemplateBindingContext bindingContext) + public string BindValues(IDictionary acceptedValues) { var context = new UriBuildingContext(); - for (var i = 0; i < Template.Segments.Count; i++) + for (var i = 0; i < _template.Segments.Count; i++) { Contract.Assert(context.BufferState == SegmentState.Beginning); Contract.Assert(context.UriState == SegmentState.Beginning); - var segment = Template.Segments[i]; + var segment = _template.Segments[i]; for (var j = 0; j < segment.Parts.Count; j++) { @@ -210,14 +172,24 @@ namespace Microsoft.AspNet.Routing.Template { // If it's a parameter, get its value object value; - var hasValue = bindingContext.AcceptedValues.TryGetValue(part.Name, out value); + var hasValue = acceptedValues.TryGetValue(part.Name, out value); if (hasValue) { - bindingContext.Use(part.Name); + acceptedValues.Remove(part.Name); + } + + bool isSameAsDefault = false; + object defaultValue; + if (_defaults != null && _defaults.TryGetValue(part.Name, out defaultValue)) + { + if (RoutePartsEqual(value, defaultValue)) + { + isSameAsDefault = true; + } } var converted = Convert.ToString(value, CultureInfo.InvariantCulture); - if (bindingContext.AcceptedDefaultValues.Contains(part.Name)) + if (isSameAsDefault) { // If the accepted value is the same as the default value buffer it since // we won't necessarily add it to the URI we generate. @@ -243,10 +215,16 @@ namespace Microsoft.AspNet.Routing.Template var encoded = new StringBuilder(); encoded.Append(UriEncode(context.Build())); - // Generate the query string + // Generate the query string from the remaining values var firstParam = true; - foreach (var kvp in bindingContext.UnusedValues) + foreach (var kvp in acceptedValues) { + if (_defaults != null && _defaults.ContainsKey(kvp.Key)) + { + // This value is a 'filter' we don't need to put it in the query string. + continue; + } + var converted = Convert.ToString(kvp.Value, CultureInfo.InvariantCulture); if (String.IsNullOrEmpty(converted)) { @@ -277,9 +255,9 @@ namespace Microsoft.AspNet.Routing.Template private TemplatePart GetParameter(string name) { - for (int i = 0; i < Template.Parameters.Count; i++) + for (int i = 0; i < _template.Parameters.Count; i++) { - var parameter = Template.Parameters[i]; + var parameter = _template.Parameters[i]; if (string.Equals(parameter.Name, name, StringComparison.OrdinalIgnoreCase)) { return parameter; @@ -333,8 +311,6 @@ namespace Microsoft.AspNet.Routing.Template private readonly IDictionary _defaults; private readonly Dictionary _acceptedValues; - private readonly HashSet _acceptedDefaultValues; - private readonly Dictionary _unusedValues; private readonly Dictionary _filters; public TemplateBindingContext(IDictionary defaults, IDictionary values) @@ -347,8 +323,6 @@ namespace Microsoft.AspNet.Routing.Template _defaults = defaults; _acceptedValues = new Dictionary(StringComparer.OrdinalIgnoreCase); - _acceptedDefaultValues = new HashSet(StringComparer.OrdinalIgnoreCase); - _unusedValues = new Dictionary(values, StringComparer.OrdinalIgnoreCase); if (_defaults != null) { @@ -361,20 +335,6 @@ namespace Microsoft.AspNet.Routing.Template get { return _acceptedValues; } } - /// - /// These are values that are equivalent to the default. These aren't written to the url unless - /// necessary. - /// > - public HashSet AcceptedDefaultValues - { - get { return _acceptedDefaultValues; } - } - - public Dictionary UnusedValues - { - get { return _unusedValues; } - } - public Dictionary Filters { get { return _filters; } @@ -385,15 +345,6 @@ namespace Microsoft.AspNet.Routing.Template if (!_acceptedValues.ContainsKey(key)) { _acceptedValues.Add(key, value); - - object defaultValue; - if (_defaults != null && _defaults.TryGetValue(key, out defaultValue)) - { - if (RoutePartsEqual(value, defaultValue)) - { - _acceptedDefaultValues.Add(key); - } - } } } @@ -406,8 +357,6 @@ namespace Microsoft.AspNet.Routing.Template { _filters.Remove(key); _acceptedValues.Add(key, value); - - _acceptedDefaultValues.Add(key); } } @@ -416,11 +365,6 @@ namespace Microsoft.AspNet.Routing.Template return !_acceptedValues.ContainsKey(key); } - public void Use(string key) - { - _unusedValues.Remove(key); - } - private string DebuggerToString() { return string.Format( diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs index a331a5ba00..2e664a0d26 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs @@ -37,7 +37,7 @@ namespace Microsoft.AspNet.Routing.Template _parsedTemplate = TemplateParser.Parse(RouteTemplate); _matcher = new TemplateMatcher(_parsedTemplate); - _binder = new TemplateBinder(_parsedTemplate); + _binder = new TemplateBinder(_parsedTemplate, _defaults); } public IDictionary Defaults @@ -80,6 +80,13 @@ namespace Microsoft.AspNet.Routing.Template public string GetVirtualPath(VirtualPathContext context) { + var values = _binder.GetAcceptedValues(context.AmbientValues, context.Values); + if (values == null) + { + // We're missing one the required values for this route. + return null; + } + // Validate that the target can accept these values. var path = _target.GetVirtualPath(context); if (path != null) @@ -93,9 +100,7 @@ namespace Microsoft.AspNet.Routing.Template return null; } - // This could be optimized more heavily - right now we try to do the full url - // generation after validating, but we could do it in two phases if the perf is better. - path = _binder.Bind(_defaults, context.AmbientValues, context.Values); + path = _binder.BindValues(values); if (path == null) { context.IsBound = false; diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs index 47d86e37ca..1c929f3234 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs @@ -125,12 +125,23 @@ namespace Microsoft.AspNet.Routing.Template.Tests string expected) { // Arrange - var binder = new TemplateBinder(TemplateParser.Parse(template)); + var binder = new TemplateBinder(TemplateParser.Parse(template), defaults); - // Act - var boundTemplate = binder.Bind(defaults, null, values); + // Act & Assert + var acceptedValues = binder.GetAcceptedValues(null, values); + if (acceptedValues == null) + { + if (expected == null) + { + return; + } + else + { + Assert.NotNull(acceptedValues); + } + } - // Assert + var boundTemplate = binder.BindValues(acceptedValues); if (expected == null) { Assert.Null(boundTemplate); @@ -947,12 +958,23 @@ namespace Microsoft.AspNet.Routing.Template.Tests string expected) { // Arrange - var binder = new TemplateBinder(TemplateParser.Parse(template)); + var binder = new TemplateBinder(TemplateParser.Parse(template), defaults); - // Act - var boundTemplate = binder.Bind(defaults, ambientValues, values); + // Act & Assert + var acceptedValues = binder.GetAcceptedValues(ambientValues, values); + if (acceptedValues == null) + { + if (expected == null) + { + return; + } + else + { + Assert.NotNull(acceptedValues); + } + } - // Assert + var boundTemplate = binder.BindValues(acceptedValues); if (expected == null) { Assert.Null(boundTemplate); diff --git a/test/Microsoft.AspNet.Routing.Tests/project.json b/test/Microsoft.AspNet.Routing.Tests/project.json index e933186c8a..fc38997e9c 100644 --- a/test/Microsoft.AspNet.Routing.Tests/project.json +++ b/test/Microsoft.AspNet.Routing.Tests/project.json @@ -23,7 +23,7 @@ }, "net45": { "dependencies": { - "Moq": "4.2.1402.2112", + "Moq": "4.2.1312.1622", "System.Runtime": "" } }