Add ValidationRuleSet.Remove (#1535)

LucGenetier · baywet · andrueastman · web-flow · commit 51d7b3041f14 · 2024-01-24T13:29:58.000Z
* Add ValidationRuleSet.Remove

* Updates following Vincent's comments

* Updates following Vincent's comments
Add ValidationRuleSet.Remove

* Update

* Apply suggestions from code review

---------

Co-authored-by: Vincent Biret &lt;vibiret@microsoft.com&gt;
Co-authored-by: Eastman &lt;andrueastman@users.noreply.github.com&gt;
Co-authored-by: Darrel &lt;darrmi@microsoft.com&gt;
Co-authored-by: Maggie Kimani &lt;maggiekim42@gmail.com&gt;
Co-authored-by: Irvine Sunday &lt;40403681+irvinesunday@users.noreply.github.com&gt;
Co-authored-by: Vincent Biret &lt;vincentbiret@hotmail.com&gt;
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiComponentsRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiComponentsRules.cs
@@ -24,7 +24,7 @@ public static class OpenApiComponentsRules
         /// that MUST use keys that match the regular expression: ^[a-zA-Z0-9\.\-_]+$.
         /// </summary>
         public static ValidationRule<OpenApiComponents> KeyMustBeRegularExpression =>
-            new(
+            new(nameof(KeyMustBeRegularExpression),
                 (context, components) =>
                 {
                     ValidateKeys(context, components.Schemas?.Keys, "schemas");
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiContactRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiContactRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiContactRules
         /// Email field MUST be email address.
         /// </summary>
         public static ValidationRule<OpenApiContact> EmailMustBeEmailFormat =>
-            new(
+            new(nameof(EmailMustBeEmailFormat),
                 (context, item) =>
                 {
                     context.Enter("email");
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiDocumentRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiDocumentRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiDocumentRules
         /// The Info field is required.
         /// </summary>
         public static ValidationRule<OpenApiDocument> OpenApiDocumentFieldIsMissing =>
-            new(
+            new(nameof(OpenApiDocumentFieldIsMissing),
                 (context, item) =>
                 {
                     // info
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiExtensionRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiExtensionRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiExtensibleRules
         /// Extension name MUST start with "x-".
         /// </summary>
         public static ValidationRule<IOpenApiExtensible> ExtensionNameMustStartWithXDash =>
-            new(
+            new(nameof(ExtensionNameMustStartWithXDash),
                 (context, item) =>
                 {
                     context.Enter("extensions");
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiExternalDocsRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiExternalDocsRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiExternalDocsRules
         /// Validate the field is required.
         /// </summary>
         public static ValidationRule<OpenApiExternalDocs> UrlIsRequired =>
-            new(
+            new(nameof(UrlIsRequired),
                 (context, item) =>
                 {
                     // url
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiHeaderRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiHeaderRules.cs
@@ -16,7 +16,7 @@ public static class OpenApiHeaderRules
         /// Validate the data matches with the given data type.
         /// </summary>
         public static ValidationRule<OpenApiHeader> HeaderMismatchedDataType =>
-            new(
+            new(nameof(HeaderMismatchedDataType),
                 (context, header) =>
                 {
                     // example
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiInfoRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiInfoRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiInfoRules
         /// Validate the field is required.
         /// </summary>
         public static ValidationRule<OpenApiInfo> InfoRequiredFields =>
-            new(
+            new(nameof(InfoRequiredFields),
                 (context, item) =>
                 {
                     // title
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiLicenseRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiLicenseRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiLicenseRules
         /// REQUIRED.
         /// </summary>
         public static ValidationRule<OpenApiLicense> LicenseRequiredFields =>
-            new(
+            new(nameof(LicenseRequiredFields),
                 (context, license) =>
                 {
                     context.Enter("name");
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiMediaTypeRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiMediaTypeRules.cs
@@ -24,7 +24,7 @@ public static class OpenApiMediaTypeRules
         /// Validate the data matches with the given data type.
         /// </summary>
         public static ValidationRule<OpenApiMediaType> MediaTypeMismatchedDataType =>
-            new(
+            new(nameof(MediaTypeMismatchedDataType),
                 (context, mediaType) =>
                 {
                     // example
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiOAuthFlowRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiOAuthFlowRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiOAuthFlowRules
         /// Validate the field is required.
         /// </summary>
         public static ValidationRule<OpenApiOAuthFlow> OAuthFlowRequiredFields =>
-            new(
+            new(nameof(OAuthFlowRequiredFields),
                 (context, flow) =>
                 {
                     // authorizationUrl
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiParameterRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiParameterRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiParameterRules
         /// Validate the field is required.
         /// </summary>
         public static ValidationRule<OpenApiParameter> ParameterRequiredFields =>
-            new(
+            new(nameof(ParameterRequiredFields),
                 (context, item) =>
                 {
                     // name
@@ -43,7 +43,7 @@ public static class OpenApiParameterRules
         /// Validate the "required" field is true when "in" is path.
         /// </summary>
         public static ValidationRule<OpenApiParameter> RequiredMustBeTrueWhenInIsPath =>
-            new(
+            new(nameof(RequiredMustBeTrueWhenInIsPath),
                 (context, item) =>
                 {
                     // required
@@ -62,7 +62,7 @@ public static class OpenApiParameterRules
         /// Validate the data matches with the given data type.
         /// </summary>
         public static ValidationRule<OpenApiParameter> ParameterMismatchedDataType =>
-            new(
+            new(nameof(ParameterMismatchedDataType),
                 (context, parameter) =>
                 {
                     // example
@@ -100,7 +100,7 @@ public static class OpenApiParameterRules
         /// Validate that a path parameter should always appear in the path
         /// </summary>
         public static ValidationRule<OpenApiParameter> PathParameterShouldBeInThePath =>
-            new(
+            new(nameof(PathParameterShouldBeInThePath),
                 (context, parameter) =>
                 {
                     if (parameter.In == ParameterLocation.Path &&
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiPathsRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiPathsRules.cs
@@ -18,7 +18,7 @@ public static class OpenApiPathsRules
         /// A relative path to an individual endpoint. The field name MUST begin with a slash.
         /// </summary>
         public static ValidationRule<OpenApiPaths> PathNameMustBeginWithSlash =>
-            new(
+            new(nameof(PathNameMustBeginWithSlash),
                 (context, item) =>
                 {
                     foreach (var pathName in item.Keys)
@@ -39,7 +39,7 @@ public static class OpenApiPathsRules
         /// A relative path to an individual endpoint. The field name MUST begin with a slash.
         /// </summary>
         public static ValidationRule<OpenApiPaths> PathMustBeUnique =>
-            new ValidationRule<OpenApiPaths>(
+            new ValidationRule<OpenApiPaths>(nameof(PathMustBeUnique),
                 (context, item) =>
                 {
                     var hashSet = new HashSet<string>();
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponseRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponseRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiResponseRules
         /// Validate the field is required.
         /// </summary>
         public static ValidationRule<OpenApiResponse> ResponseRequiredFields =>
-            new(
+            new(nameof(ResponseRequiredFields),
                 (context, response) =>
                 {
                     // description
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+﻿// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license.
 
 using System.Linq;
@@ -17,7 +17,7 @@ public static class OpenApiResponsesRules
         /// An OpenAPI operation must contain at least one response
         /// </summary>
         public static ValidationRule<OpenApiResponses> ResponsesMustContainAtLeastOneResponse =>
-            new(
+            new(nameof(ResponsesMustContainAtLeastOneResponse),
                 (context, responses) =>
                 {
                     if (!responses.Keys.Any())
@@ -31,7 +31,7 @@ public static class OpenApiResponsesRules
         /// The response key must either be "default" or an HTTP status code (1xx, 2xx, 3xx, 4xx, 5xx).
         /// </summary>
         public static ValidationRule<OpenApiResponses> ResponsesMustBeIdentifiedByDefaultOrStatusCode =>
-            new(
+            new(nameof(ResponsesMustBeIdentifiedByDefaultOrStatusCode),
                 (context, responses) =>
                 {
                     foreach (var key in responses.Keys)
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiSchemaRules
         /// Validate the data matches with the given data type.
         /// </summary>
         public static ValidationRule<OpenApiSchema> SchemaMismatchedDataType =>
-            new(
+            new(nameof(SchemaMismatchedDataType),
                 (context, schema) =>
                 {
                     // default
@@ -60,7 +60,7 @@ public static class OpenApiSchemaRules
         /// Validates Schema Discriminator
         /// </summary>
         public static ValidationRule<OpenApiSchema> ValidateSchemaDiscriminator =>
-            new(
+            new(nameof(ValidateSchemaDiscriminator),
                 (context, schema) =>
                 {
                     // discriminator
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiServerRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiServerRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiServerRules
         /// Validate the field is required.
         /// </summary>
         public static ValidationRule<OpenApiServer> ServerRequiredFields =>
-            new(
+            new(nameof(ServerRequiredFields),
                 (context, server) =>
                 {
                     context.Enter("url");
diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiTagRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiTagRules.cs
@@ -17,7 +17,7 @@ public static class OpenApiTagRules
         /// Validate the field is required.
         /// </summary>
         public static ValidationRule<OpenApiTag> TagRequiredFields =>
-            new(
+            new(nameof(TagRequiredFields),
                 (context, tag) =>
                 {
                     context.Enter("name");
diff --git a/src/Microsoft.OpenApi/Validations/ValidationRule.cs b/src/Microsoft.OpenApi/Validations/ValidationRule.cs
@@ -2,7 +2,7 @@
 // Licensed under the MIT license.
 
 using System;
-using System.Globalization;
+using Microsoft.OpenApi.Exceptions;
 using Microsoft.OpenApi.Interfaces;
 using Microsoft.OpenApi.Properties;
 
@@ -18,12 +18,22 @@ public abstract class ValidationRule
         /// </summary>
         internal abstract Type ElementType { get; }
 
+        /// <summary>
+        /// Validation rule Name.
+        /// </summary>
+        public string Name { get; }
+
         /// <summary>
         /// Validate the object.
         /// </summary>
         /// <param name="context">The context.</param>
         /// <param name="item">The object item.</param>
         internal abstract void Evaluate(IValidationContext context, object item);
+
+        internal ValidationRule(string name)
+        {
+            Name = !string.IsNullOrEmpty(name) ? name : throw new ArgumentNullException(nameof(name));
+        }
     }
 
     /// <summary>
@@ -33,14 +43,26 @@ public abstract class ValidationRule
     public class ValidationRule<T> : ValidationRule where T : IOpenApiElement
     {
         private readonly Action<IValidationContext, T> _validate;
+        
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ValidationRule"/> class.
+        /// </summary>        
+        /// <param name="validate">Action to perform the validation.</param>
+        [Obsolete("Please use the other constructor and specify a name")]
+        public ValidationRule(Action<IValidationContext, T> validate)
+            : this (Guid.NewGuid().ToString("D"), validate)
+        {
+        }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ValidationRule"/> class.
         /// </summary>
+        /// <param name="name">Validation rule name.</param>
         /// <param name="validate">Action to perform the validation.</param>
-        public ValidationRule(Action<IValidationContext, T> validate)
+        public ValidationRule(string name, Action<IValidationContext, T> validate)
+            : base(name) 
         {
-            _validate = Utils.CheckArgumentNull(validate);
+            _validate = Utils.CheckArgumentNull(validate);            
         }
 
         internal override Type ElementType
diff --git a/src/Microsoft.OpenApi/Validations/ValidationRuleSet.cs b/src/Microsoft.OpenApi/Validations/ValidationRuleSet.cs
@@ -134,6 +134,30 @@ public void Add(ValidationRule rule)
             item.Add(rule);
         }
 
+        /// <summary>
+        /// Remove a rule by its name from all types it is used by.
+        /// </summary>        
+        /// <param name="ruleName">Name of the rule.</param>
+        public void Remove(string ruleName)
+        {
+            foreach (KeyValuePair<Type, IList<ValidationRule>> rule in _rules)
+            {
+                _rules[rule.Key] = rule.Value.Where(vr => !vr.Name.Equals(ruleName, StringComparison.Ordinal)).ToList();                
+            }
+
+            // Remove types with no rule
+            _rules = _rules.Where(r => r.Value.Any()).ToDictionary(r => r.Key, r => r.Value);
+        }
+
+        /// <summary>
+        /// Remove a rule by element type.
+        /// </summary>        
+        /// <param name="type">Type of the rule.</param>
+        public void Remove(Type type)
+        {
+            _rules.Remove(type);
+        }
+
         /// <summary>
         /// Get the enumerator.
         /// </summary>
diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
@@ -1347,7 +1347,7 @@ namespace Microsoft.OpenApi.Validations
     }
     public abstract class ValidationRule
     {
-        protected ValidationRule() { }
+        public string Name { get; }
     }
     public sealed class ValidationRuleSet : System.Collections.Generic.IEnumerable<Microsoft.OpenApi.Validations.ValidationRule>, System.Collections.IEnumerable
     {
@@ -1358,13 +1358,17 @@ namespace Microsoft.OpenApi.Validations
         public void Add(Microsoft.OpenApi.Validations.ValidationRule rule) { }
         public System.Collections.Generic.IList<Microsoft.OpenApi.Validations.ValidationRule> FindRules(System.Type type) { }
         public System.Collections.Generic.IEnumerator<Microsoft.OpenApi.Validations.ValidationRule> GetEnumerator() { }
+        public void Remove(string ruleName) { }
+        public void Remove(System.Type type) { }
         public static Microsoft.OpenApi.Validations.ValidationRuleSet GetDefaultRuleSet() { }
         public static Microsoft.OpenApi.Validations.ValidationRuleSet GetEmptyRuleSet() { }
     }
     public class ValidationRule<T> : Microsoft.OpenApi.Validations.ValidationRule
         where T : Microsoft.OpenApi.Interfaces.IOpenApiElement
     {
+        [System.Obsolete("Please use the other constructor and specify a name")]
         public ValidationRule(System.Action<Microsoft.OpenApi.Validations.IValidationContext, T> validate) { }
+        public ValidationRule(string name, System.Action<Microsoft.OpenApi.Validations.IValidationContext, T> validate) { }
     }
 }
 namespace Microsoft.OpenApi.Validations.Rules
diff --git a/test/Microsoft.OpenApi.Tests/Services/OpenApiValidatorTests.cs b/test/Microsoft.OpenApi.Tests/Services/OpenApiValidatorTests.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using FluentAssertions;
+using Microsoft.OpenApi.Any;
 using Microsoft.OpenApi.Interfaces;
 using Microsoft.OpenApi.Models;
 using Microsoft.OpenApi.Properties;
@@ -96,7 +97,7 @@ public void ValidateCustomExtension()
             var ruleset = ValidationRuleSet.GetDefaultRuleSet();
 
             ruleset.Add(
-             new ValidationRule<FooExtension>(
+             new ValidationRule<FooExtension>("FooExtensionRule",
                  (context, item) =>
                  {
                      if (item.Bar == "hey")
@@ -133,6 +134,44 @@ public void ValidateCustomExtension()
                        new OpenApiValidatorError("FooExtensionRule", "#/info/x-foo", "Don't say hey")
                    });
         }
+
+        [Fact]
+        public void RemoveRuleByName_Invalid()
+        {
+            Assert.Throws<ArgumentNullException>(() => new ValidationRule<IOpenApiAny>(null, (vc, oaa) => { }));
+            Assert.Throws<ArgumentNullException>(() => new ValidationRule<IOpenApiAny>(string.Empty, (vc, oaa) => { }));
+        }
+
+        [Fact]
+        public void RemoveRuleByName()
+        {
+            var ruleset = ValidationRuleSet.GetDefaultRuleSet();
+            int expected = ruleset.Rules.Count - 1;
+            ruleset.Remove("KeyMustBeRegularExpression");
+
+            Assert.Equal(expected, ruleset.Rules.Count);
+            
+            ruleset.Remove("KeyMustBeRegularExpression");
+            ruleset.Remove("UnknownName");
+
+            Assert.Equal(expected, ruleset.Rules.Count);
+        }
+
+        [Fact]
+        public void RemoveRuleByType()
+        {
+            var ruleset = ValidationRuleSet.GetDefaultRuleSet();
+            int expected = ruleset.Rules.Count - 1;
+            
+            ruleset.Remove(typeof(OpenApiComponents));
+
+            Assert.Equal(expected, ruleset.Rules.Count);
+
+            ruleset.Remove(typeof(OpenApiComponents));
+            ruleset.Remove(typeof(int));
+
+            Assert.Equal(expected, ruleset.Rules.Count);
+        }
     }
 
     internal class FooExtension : IOpenApiExtension, IOpenApiElement
diff --git a/test/Microsoft.OpenApi.Tests/Validations/OpenApiReferenceValidationTests.cs b/test/Microsoft.OpenApi.Tests/Validations/OpenApiReferenceValidationTests.cs

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ public static class OpenApiComponentsRules`
`24`	`24`	`/// that MUST use keys that match the regular expression: ^[a-zA-Z0-9\.\-_]+$.`
`25`	`25`	`/// </summary>`
`26`	`26`	`public static ValidationRule<OpenApiComponents> KeyMustBeRegularExpression =>`
`27`		`- new(`
	`27`	`+ new(nameof(KeyMustBeRegularExpression),`
`28`	`28`	`(context, components) =>`
`29`	`29`	`{`
`30`	`30`	`ValidateKeys(context, components.Schemas?.Keys, "schemas");`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ public static class OpenApiContactRules`
`17`	`17`	`/// Email field MUST be email address.`
`18`	`18`	`/// </summary>`
`19`	`19`	`public static ValidationRule<OpenApiContact> EmailMustBeEmailFormat =>`
`20`		`- new(`
	`20`	`+ new(nameof(EmailMustBeEmailFormat),`
`21`	`21`	`(context, item) =>`
`22`	`22`	`{`
`23`	`23`	`context.Enter("email");`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ public static class OpenApiHeaderRules`
`16`	`16`	`/// Validate the data matches with the given data type.`
`17`	`17`	`/// </summary>`
`18`	`18`	`public static ValidationRule<OpenApiHeader> HeaderMismatchedDataType =>`
`19`		`- new(`
	`19`	`+ new(nameof(HeaderMismatchedDataType),`
`20`	`20`	`(context, header) =>`
`21`	`21`	`{`
`22`	`22`	`// example`