#2031 Don't validate placeholders in templates (#2032)

* Remove faulty rules which validate explicit placeholders but not implicit ones * Validation rule for Service Fabric placeholders * Revert "Validation rule for Service Fabric placeholders" This reverts commit c30fe45.
ThreeMammals · Apr 5, 2024 · 14c6d82 · 14c6d82
1 parent d1855cb
commit 14c6d82
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 60 deletions.
diff --git a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs
@@ -41,13 +41,6 @@ public FileConfigurationFluentValidator(IServiceProvider provider, RouteFluentVa
             RuleForEach(configuration => configuration.Routes)
                 .Must((_, route) => IsPlaceholderNotDuplicatedIn(route.DownstreamPathTemplate))
                 .WithMessage((_, route) => $"{nameof(route.DownstreamPathTemplate)} '{route.DownstreamPathTemplate}' has duplicated placeholder");
-
-            RuleForEach(configuration => configuration.Routes)
-                .Must(IsUpstreamPlaceholderDefinedInDownstream)
-                .WithMessage((_, route) => $"{nameof(route.UpstreamPathTemplate)} '{route.UpstreamPathTemplate}' doesn't contain the same placeholders in {nameof(route.DownstreamPathTemplate)} '{route.DownstreamPathTemplate}'");
-            RuleForEach(configuration => configuration.Routes)
-                .Must(IsDownstreamPlaceholderDefinedInUpstream)
-                .WithMessage((_, route) => $"{nameof(route.DownstreamPathTemplate)} '{route.DownstreamPathTemplate}' doesn't contain the same placeholders in {nameof(route.UpstreamPathTemplate)} '{route.UpstreamPathTemplate}'");
 
             RuleFor(configuration => configuration.GlobalConfiguration.ServiceDiscoveryProvider)
                 .Must(HaveServiceDiscoveryProviderRegistered)
@@ -122,37 +115,6 @@ private static bool IsPlaceholderNotDuplicatedIn(string pathTemplate)
             return placeholders.Count == placeholders.Distinct().Count();
         }
 
-        private static bool IsServiceFabricWithServiceName(FileConfiguration configuration, FileRoute route)
-            => Servicefabric.Equals(configuration?.GlobalConfiguration?.ServiceDiscoveryProvider?.Type, StringComparison.InvariantCultureIgnoreCase)
-                && !string.IsNullOrEmpty(route?.ServiceName) && PlaceholderRegex().IsMatch(route.ServiceName);
-
-        private bool IsUpstreamPlaceholderDefinedInDownstream(FileConfiguration configuration, FileRoute route)
-            => IsServiceFabricWithServiceName(configuration, route)
-                ? IsPlaceholderDefinedInBothTemplates(route.UpstreamPathTemplate, route.ServiceName + route.DownstreamPathTemplate)
-                : IsPlaceholderDefinedInBothTemplates(route.UpstreamPathTemplate, route.DownstreamPathTemplate);
-
-        private bool IsDownstreamPlaceholderDefinedInUpstream(FileConfiguration configuration, FileRoute route)
-            => IsServiceFabricWithServiceName(configuration, route)
-                ? IsPlaceholderDefinedInBothTemplates(route.ServiceName + route.DownstreamPathTemplate, route.UpstreamPathTemplate)
-                : IsPlaceholderDefinedInBothTemplates(route.DownstreamPathTemplate, route.UpstreamPathTemplate);
-
-        private static bool IsPlaceholderDefinedInBothTemplates(string firstPathTemplate, string secondPathTemplate)
-        {
-            var firstPlaceholders = PlaceholderRegex().Matches(firstPathTemplate)
-                .Select(m => m.Value).ToList();
-            var secondPlaceholders = PlaceholderRegex().Matches(secondPathTemplate)
-                .Select(m => m.Value).ToList();
-            foreach (var placeholder in firstPlaceholders)
-            {
-                if (!secondPlaceholders.Contains(placeholder))
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
-
         private static bool DoesNotContainRoutesWithSpecificRequestIdKeys(FileAggregateRoute fileAggregateRoute,
             IEnumerable<FileRoute> routes)
         {

diff --git a/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs b/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs
@@ -753,11 +753,11 @@ public void Configuration_is_not_valid_when_host_and_port_is_empty()
         [InlineData("/foo/{bar}/foo", "/yahoo/foo/{bar}")] // valid
         [InlineData("/foo/{bar}/{foo}", "/yahoo/{foo}/{bar}")] // valid
         [InlineData("/foo/{bar}/{bar}", "/yahoo/foo/{bar}", "UpstreamPathTemplate '/foo/{bar}/{bar}' has duplicated placeholder")] // invalid
-        [InlineData("/foo/{bar}/{bar}", "/yahoo/{foo}/{bar}", "UpstreamPathTemplate '/foo/{bar}/{bar}' has duplicated placeholder", "DownstreamPathTemplate '/yahoo/{foo}/{bar}' doesn't contain the same placeholders in UpstreamPathTemplate '/foo/{bar}/{bar}'")] // invalid
+        [InlineData("/foo/{bar}/{bar}", "/yahoo/{foo}/{bar}", "UpstreamPathTemplate '/foo/{bar}/{bar}' has duplicated placeholder")] // invalid
         [InlineData("/yahoo/foo/{bar}", "/foo/{bar}/foo")] // valid
         [InlineData("/yahoo/{foo}/{bar}", "/foo/{bar}/{foo}")] // valid
         [InlineData("/yahoo/foo/{bar}", "/foo/{bar}/{bar}", "DownstreamPathTemplate '/foo/{bar}/{bar}' has duplicated placeholder")] // invalid
-        [InlineData("/yahoo/{foo}/{bar}", "/foo/{bar}/{bar}", "DownstreamPathTemplate '/foo/{bar}/{bar}' has duplicated placeholder", "UpstreamPathTemplate '/yahoo/{foo}/{bar}' doesn't contain the same placeholders in DownstreamPathTemplate '/foo/{bar}/{bar}'")] // invalid
+        [InlineData("/yahoo/{foo}/{bar}", "/foo/{bar}/{bar}", "DownstreamPathTemplate '/foo/{bar}/{bar}' has duplicated placeholder")] // invalid
         public void IsPlaceholderNotDuplicatedIn_RuleForFileRoute_PathTemplatePlaceholdersAreValidated(string upstream, string downstream, params string[] expected)
         {
             // Arrange
@@ -772,26 +772,6 @@ public void IsPlaceholderNotDuplicatedIn_RuleForFileRoute_PathTemplatePlaceholde
             ThenTheErrorMessagesAre(expected);
         }
 
-        [Theory]
-        [Trait("PR", "1927")]
-        [InlineData("/foo/{bar}/{foo}", "/yahoo/{foo}/{bar}")] // valid
-        [InlineData("/foo/{bar}/{yahoo}", "/yahoo/{foo}/{bar}", "UpstreamPathTemplate '/foo/{bar}/{yahoo}' doesn't contain the same placeholders in DownstreamPathTemplate '/yahoo/{foo}/{bar}'", "DownstreamPathTemplate '/yahoo/{foo}/{bar}' doesn't contain the same placeholders in UpstreamPathTemplate '/foo/{bar}/{yahoo}'")] // invalid
-        [InlineData("/yahoo/{foo}/{bar}", "/foo/{bar}/{foo}")] // valid
-        [InlineData("/yahoo/{foo}/{bar}", "/foo/{bar}/{yahoo}", "UpstreamPathTemplate '/yahoo/{foo}/{bar}' doesn't contain the same placeholders in DownstreamPathTemplate '/foo/{bar}/{yahoo}'", "DownstreamPathTemplate '/foo/{bar}/{yahoo}' doesn't contain the same placeholders in UpstreamPathTemplate '/yahoo/{foo}/{bar}'")] // invalid
-        public void IsPlaceholderDefinedInBothTemplates_RuleForFileRoute_PathTemplatePlaceholdersAreValidated(string upstream, string downstream, params string[] expected)
-        {
-            // Arrange
-            var route = GivenDefaultRoute(upstream, downstream);
-            GivenAConfiguration(route);
-
-            // Act
-            WhenIValidateTheConfiguration();
-
-            // Assert
-            ThenThereAreErrors(expected.Length > 0);
-            ThenTheErrorMessagesAre(expected);
-        }
-
         [Theory]
         [Trait("PR", "1927")]
         [Trait("Bug", "683")]