From ef0ea537e270a508738b857d273d4948a7c9d9e5 Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Tue, 15 Dec 2015 15:02:29 -0800 Subject: [PATCH] Removed CatchAll and Fallback from DecisionTree --- .../DecisionCriterion.cs | 2 - .../DecisionCriterionValue.cs | 9 +- .../DecisionCriterionValueEqualityComparer.cs | 11 +-- .../DecisionTreeBuilder.cs | 13 +-- .../Internal/LinkGenerationDecisionTree.cs | 2 +- .../DecisionTreeBuilderTest.cs | 95 ++++--------------- 6 files changed, 23 insertions(+), 109 deletions(-) diff --git a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterion.cs b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterion.cs index 588f3f392b..5283e73d03 100644 --- a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterion.cs +++ b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterion.cs @@ -10,7 +10,5 @@ namespace Microsoft.AspNet.Routing.DecisionTree public string Key { get; set; } public Dictionary> Branches { get; set; } - - public DecisionTreeNode Fallback { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValue.cs b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValue.cs index 853628005e..4bf8131e73 100644 --- a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValue.cs +++ b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValue.cs @@ -5,18 +5,11 @@ namespace Microsoft.AspNet.Routing.DecisionTree { internal struct DecisionCriterionValue { - private readonly bool _isCatchAll; private readonly object _value; - public DecisionCriterionValue(object value, bool isCatchAll) + public DecisionCriterionValue(object value) { _value = value; - _isCatchAll = isCatchAll; - } - - public bool IsCatchAll - { - get { return _isCatchAll; } } public object Value diff --git a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValueEqualityComparer.cs b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValueEqualityComparer.cs index 7970937162..a678312b92 100644 --- a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValueEqualityComparer.cs +++ b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionCriterionValueEqualityComparer.cs @@ -16,19 +16,12 @@ namespace Microsoft.AspNet.Routing.DecisionTree public bool Equals(DecisionCriterionValue x, DecisionCriterionValue y) { - return x.IsCatchAll == y.IsCatchAll || InnerComparer.Equals(x.Value, y.Value); + return InnerComparer.Equals(x.Value, y.Value); } public int GetHashCode(DecisionCriterionValue obj) { - if (obj.IsCatchAll) - { - return 0; - } - else - { - return InnerComparer.GetHashCode(obj.Value); - } + return InnerComparer.GetHashCode(obj.Value); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionTreeBuilder.cs b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionTreeBuilder.cs index 446b591da9..56b28c446c 100644 --- a/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionTreeBuilder.cs +++ b/src/Microsoft.AspNet.Routing.DecisionTree.Sources/DecisionTreeBuilder.cs @@ -153,7 +153,6 @@ namespace Microsoft.AspNet.Routing.DecisionTree foreach (var criterion in criteria.OrderByDescending(c => c.Value.Count)) { var reducedBranches = new Dictionary>(comparer.InnerComparer); - DecisionTreeNode fallback = null; foreach (var branch in criterion.Value) { @@ -172,24 +171,16 @@ namespace Microsoft.AspNet.Routing.DecisionTree childContext.CurrentCriteria.Add(criterion.Key); var newBranch = GenerateNode(childContext, comparer, branch.Value); - if (branch.Key.IsCatchAll) - { - fallback = newBranch; - } - else - { - reducedBranches.Add(branch.Key.Value, newBranch); - } + reducedBranches.Add(branch.Key.Value, newBranch); } } - if (reducedBranches.Count > 0 || fallback != null) + if (reducedBranches.Count > 0) { var newCriterion = new DecisionCriterion() { Key = criterion.Key, Branches = reducedBranches, - Fallback = fallback, }; reducedCriteria.Add(newCriterion); diff --git a/src/Microsoft.AspNet.Routing/Internal/LinkGenerationDecisionTree.cs b/src/Microsoft.AspNet.Routing/Internal/LinkGenerationDecisionTree.cs index 4ae1da2ef0..5c82fcb0de 100644 --- a/src/Microsoft.AspNet.Routing/Internal/LinkGenerationDecisionTree.cs +++ b/src/Microsoft.AspNet.Routing/Internal/LinkGenerationDecisionTree.cs @@ -117,7 +117,7 @@ namespace Microsoft.AspNet.Routing.Internal var results = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var kvp in item.RequiredLinkValues) { - results.Add(kvp.Key, new DecisionCriterionValue(kvp.Value ?? string.Empty, isCatchAll: false)); + results.Add(kvp.Key, new DecisionCriterionValue(kvp.Value ?? string.Empty)); } return results; diff --git a/test/Microsoft.AspNet.Routing.DecisionTree.Sources.Tests/DecisionTreeBuilderTest.cs b/test/Microsoft.AspNet.Routing.DecisionTree.Sources.Tests/DecisionTreeBuilderTest.cs index 3da8b6d9a8..68904645c4 100644 --- a/test/Microsoft.AspNet.Routing.DecisionTree.Sources.Tests/DecisionTreeBuilderTest.cs +++ b/test/Microsoft.AspNet.Routing.DecisionTree.Sources.Tests/DecisionTreeBuilderTest.cs @@ -47,9 +47,9 @@ namespace Microsoft.AspNet.Routing.DecisionTree var items = new List(); var item = new Item(); - item.Criteria.Add("area", new DecisionCriterionValue(value: "Admin", isCatchAll: false)); - item.Criteria.Add("controller", new DecisionCriterionValue(value: "Users", isCatchAll: false)); - item.Criteria.Add("action", new DecisionCriterionValue(value: "AddUser", isCatchAll: false)); + item.Criteria.Add("area", new DecisionCriterionValue(value: "Admin")); + item.Criteria.Add("controller", new DecisionCriterionValue(value: "Users")); + item.Criteria.Add("action", new DecisionCriterionValue(value: "AddUser")); items.Add(item); // Act @@ -60,7 +60,6 @@ namespace Microsoft.AspNet.Routing.DecisionTree var area = Assert.Single(tree.Criteria); Assert.Equal("area", area.Key); - Assert.Null(area.Fallback); var admin = Assert.Single(area.Branches); Assert.Equal("Admin", admin.Key); @@ -68,7 +67,6 @@ namespace Microsoft.AspNet.Routing.DecisionTree var controller = Assert.Single(admin.Value.Criteria); Assert.Equal("controller", controller.Key); - Assert.Null(controller.Fallback); var users = Assert.Single(controller.Branches); Assert.Equal("Users", users.Key); @@ -76,7 +74,6 @@ namespace Microsoft.AspNet.Routing.DecisionTree var action = Assert.Single(users.Value.Criteria); Assert.Equal("action", action.Key); - Assert.Null(action.Fallback); var addUser = Assert.Single(action.Branches); Assert.Equal("AddUser", addUser.Key); @@ -91,13 +88,13 @@ namespace Microsoft.AspNet.Routing.DecisionTree var items = new List(); var item1 = new Item(); - item1.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item1.Criteria.Add("action", new DecisionCriterionValue(value: "Buy", isCatchAll: false)); + item1.Criteria.Add("controller", new DecisionCriterionValue(value: "Store")); + item1.Criteria.Add("action", new DecisionCriterionValue(value: "Buy")); items.Add(item1); var item2 = new Item(); - item2.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item2.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout", isCatchAll: false)); + item2.Criteria.Add("controller", new DecisionCriterionValue(value: "Store")); + item2.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout")); items.Add(item2); // Act @@ -108,14 +105,12 @@ namespace Microsoft.AspNet.Routing.DecisionTree var action = Assert.Single(tree.Criteria); Assert.Equal("action", action.Key); - Assert.Null(action.Fallback); var buy = action.Branches["Buy"]; Assert.Empty(buy.Matches); var controller = Assert.Single(buy.Criteria); Assert.Equal("controller", controller.Key); - Assert.Null(controller.Fallback); var store = Assert.Single(controller.Branches); Assert.Equal("Store", store.Key); @@ -127,7 +122,6 @@ namespace Microsoft.AspNet.Routing.DecisionTree controller = Assert.Single(checkout.Criteria); Assert.Equal("controller", controller.Key); - Assert.Null(controller.Fallback); store = Assert.Single(controller.Branches); Assert.Equal("Store", store.Key); @@ -142,17 +136,17 @@ namespace Microsoft.AspNet.Routing.DecisionTree var items = new List(); var item1 = new Item(); - item1.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item1.Criteria.Add("action", new DecisionCriterionValue(value: "Buy", isCatchAll: false)); + item1.Criteria.Add("controller", new DecisionCriterionValue(value: "Store")); + item1.Criteria.Add("action", new DecisionCriterionValue(value: "Buy")); items.Add(item1); var item2 = new Item(); - item2.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item2.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout", isCatchAll: false)); + item2.Criteria.Add("controller", new DecisionCriterionValue(value: "Store")); + item2.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout")); items.Add(item2); var item3 = new Item(); - item3.Criteria.Add("action", new DecisionCriterionValue(value: "Buy", isCatchAll: false)); + item3.Criteria.Add("action", new DecisionCriterionValue(value: "Buy")); items.Add(item3); // Act @@ -163,66 +157,11 @@ namespace Microsoft.AspNet.Routing.DecisionTree var action = Assert.Single(tree.Criteria); Assert.Equal("action", action.Key); - Assert.Null(action.Fallback); var buy = action.Branches["Buy"]; Assert.Same(item3, Assert.Single(buy.Matches)); } - [Fact] - public void BuildTree_WithCatchAll() - { - // Arrange - var items = new List(); - - var item1 = new Item(); - item1.Criteria.Add("country", new DecisionCriterionValue(value: "CA", isCatchAll: false)); - item1.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item1.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout", isCatchAll: false)); - items.Add(item1); - - var item2 = new Item(); - item2.Criteria.Add("country", new DecisionCriterionValue(value: "US", isCatchAll: false)); - item2.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item2.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout", isCatchAll: false)); - items.Add(item2); - - var item3 = new Item(); - item3.Criteria.Add("country", new DecisionCriterionValue(value: null, isCatchAll: true)); - item3.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item3.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout", isCatchAll: false)); - items.Add(item3); - - // Act - var tree = DecisionTreeBuilder.GenerateTree(items, new ItemClassifier()); - - // Assert - Assert.Empty(tree.Matches); - - var country = Assert.Single(tree.Criteria); - Assert.Equal("country", country.Key); - - var fallback = country.Fallback; - Assert.NotNull(fallback); - - var controller = Assert.Single(fallback.Criteria); - Assert.Equal("controller", controller.Key); - Assert.Null(controller.Fallback); - - var store = Assert.Single(controller.Branches); - Assert.Equal("Store", store.Key); - Assert.Empty(store.Value.Matches); - - var action = Assert.Single(store.Value.Criteria); - Assert.Equal("action", action.Key); - Assert.Null(action.Fallback); - - var checkout = Assert.Single(action.Branches); - Assert.Equal("Checkout", checkout.Key); - Assert.Empty(checkout.Value.Criteria); - Assert.Same(item3, Assert.Single(checkout.Value.Matches)); - } - [Fact] public void BuildTree_WithDivergentCriteria() { @@ -230,17 +169,17 @@ namespace Microsoft.AspNet.Routing.DecisionTree var items = new List(); var item1 = new Item(); - item1.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item1.Criteria.Add("action", new DecisionCriterionValue(value: "Buy", isCatchAll: false)); + item1.Criteria.Add("controller", new DecisionCriterionValue(value: "Store")); + item1.Criteria.Add("action", new DecisionCriterionValue(value: "Buy")); items.Add(item1); var item2 = new Item(); - item2.Criteria.Add("controller", new DecisionCriterionValue(value: "Store", isCatchAll: false)); - item2.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout", isCatchAll: false)); + item2.Criteria.Add("controller", new DecisionCriterionValue(value: "Store")); + item2.Criteria.Add("action", new DecisionCriterionValue(value: "Checkout")); items.Add(item2); var item3 = new Item(); - item3.Criteria.Add("stub", new DecisionCriterionValue(value: "Bleh", isCatchAll: false)); + item3.Criteria.Add("stub", new DecisionCriterionValue(value: "Bleh")); items.Add(item3); // Act