From 747420e5aa7cc2c7834cfb9731510286ded6fc03 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 28 Dec 2017 09:43:24 -0800 Subject: [PATCH] Compatibility switches (#7142) * [Design] Compatibility switches This introduces a pattern for versioning breaking behaviour changes in minor releases of MVC. The general plan is that application developers choose a release version (2.0, 2.1, Latest) as their baseline which determines the effective 'defaults' for some options. Anything the developer sets explicitly is an override and always wins. Then we add a version setting to the template to point to the current release. This allows us to be progressive with fixing issues and improving areas that don't work well, but offers the developer some choice about when to adopt new behaviours. In effect, we separate new behaviours from the libraries that develiver them. Apps can update the version, and then opt in to new behaviours as a separate change. * Be more american * improve docs, add example * Fix visibility * Fix broken test * Add test * Docs! * The rest of the tests * fix example * Adding docs * PR feedback --- .../CompatibilityVersion.cs | 59 ++++++++ .../MvcCoreMvcBuilderExtensions.cs | 23 ++++ .../MvcCoreMvcCoreBuilderExtensions.cs | 18 +++ .../MvcCoreServiceCollectionExtensions.cs | 2 + .../Infrastructure/CompatibilitySwitch.cs | 129 ++++++++++++++++++ .../ConfigureCompatibilityOptions.cs | 105 ++++++++++++++ .../Infrastructure/ICompatibilitySwitch.cs | 36 +++++ .../Infrastructure/MvcCompatibilityOptions.cs | 23 ++++ ...MvcOptionsConfigureCompatibilityOptions.cs | 35 +++++ .../MvcOptions.cs | 45 +++++- .../MvcCoreServiceCollectionExtensionsTest.cs | 8 ++ .../Infrastructure/CompatibilitySwitchTest.cs | 60 ++++++++ .../ConfigureCompatibilityOptionsTest.cs | 117 ++++++++++++++++ .../CompatibilitySwitchIntegrationTest.cs | 83 +++++++++++ 14 files changed, 739 insertions(+), 4 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/CompatibilityVersion.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ConfigureCompatibilityOptions.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ICompatibilitySwitch.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcCompatibilityOptions.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/CompatibilitySwitchTest.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ConfigureCompatibilityOptionsTest.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/CompatibilityVersion.cs b/src/Microsoft.AspNetCore.Mvc.Core/CompatibilityVersion.cs new file mode 100644 index 0000000000..c49c27f454 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/CompatibilityVersion.cs @@ -0,0 +1,59 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Mvc +{ + /// + /// Specifies the version compatibility of runtime behaviors configured by . + /// + /// + /// + /// The best way to set a compatibility version is by using + /// or + /// in your application's + /// ConfigureServices method. + /// + /// Setting the compatibility version using : + /// + /// public class Startup + /// { + /// ... + /// + /// public void ConfigureServices(IServiceCollection services) + /// { + /// services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_1); + /// } + /// + /// ... + /// } + /// + /// + /// + /// + /// Setting compatiblity version to a specific version will change the default values of various + /// settings to match a particular minor release of ASP.NET Core MVC. + /// + /// + public enum CompatibilityVersion + { + /// + /// Sets the default value of settings on to match the behavior of + /// ASP.NET Core MVC 2.0. + /// + Version_2_0, + + /// + /// Sets the default value of settings on to match the behavior of + /// ASP.NET Core MVC 2.1. + /// + Version_2_1, + + /// + /// Sets the default value of settings on to match the latest release. Use this + /// value with care, upgrading minor versions will cause breaking changes when using . + /// + Latest = int.MaxValue, + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcBuilderExtensions.cs index 7481cdcea1..6e1f9f9a1b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcBuilderExtensions.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.Extensions.DependencyInjection.Extensions; namespace Microsoft.Extensions.DependencyInjection @@ -116,6 +117,11 @@ namespace Microsoft.Extensions.DependencyInjection /// The . public static IMvcBuilder AddControllersAsServices(this IMvcBuilder builder) { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + var feature = new ControllerFeature(); builder.PartManager.PopulateFeature(feature); @@ -128,5 +134,22 @@ namespace Microsoft.Extensions.DependencyInjection return builder; } + + /// + /// Sets the for ASP.NET Core MVC for the application. + /// + /// The . + /// The value to configure. + /// The . + public static IMvcBuilder SetCompatibilityVersion(this IMvcBuilder builder, CompatibilityVersion version) + { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + builder.Services.Configure(o => o.CompatibilityVersion = version); + return builder; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs index aabadff1ad..76ebe6c261 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -167,5 +168,22 @@ namespace Microsoft.Extensions.DependencyInjection return builder; } + + /// + /// Sets the for ASP.NET Core MVC for the application. + /// + /// The . + /// The value to configure. + /// The . + public static IMvcCoreBuilder SetCompatibilityVersion(this IMvcCoreBuilder builder, CompatibilityVersion version) + { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + builder.Services.Configure(o => o.CompatibilityVersion = version); + return builder; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 94c470e3a3..03ffe21330 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -147,6 +147,8 @@ namespace Microsoft.Extensions.DependencyInjection // services.TryAddEnumerable( ServiceDescriptor.Transient, MvcCoreMvcOptionsSetup>()); + services.TryAddEnumerable( + ServiceDescriptor.Transient, MvcOptionsConfigureCompatibilityOptions>()); services.TryAddEnumerable( ServiceDescriptor.Transient, ApiBehaviorOptionsSetup>()); services.TryAddEnumerable( diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs new file mode 100644 index 0000000000..59f650038a --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs @@ -0,0 +1,129 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + // Guide to making breaking behavior changes in MVC: + // + // Hello, if you're reading this, you're probably trying to make a change in behavior in MVC in a minor + // version. Every change in behavior is a breaking change to someone, even if a feature was buggy or + // broken in some scenarios. + // + // To help make things easier for current users, we don't automatically opt users into breaking changes when + // they upgrade applications to a new minor version of ASP.NET Core. It's a separate choice to opt in to new + // behaviors in a minor release. + // + // To make things better for future users, we also want to provide an easy way for applications to get + // access to the new behaviors. We make changes when they are improvments, and if we're changing something + // we've already shipped, it must add value for all of our users (eventually). To this end, new applications + // created using the template are always opted in to the 'current' version. + // + // This means that all changes in behavior should be opt-in. + // + // ----- + // + // Moving on from general philosophy, here's how to implement a behavior change and corresponding + // compatibility switch. + // + // Add a new property on options that uses a CompatibilitySwitch as a backing field. Make sure the + // new switch is exposed by implementing IEnumerable on the options class. Pass the + // property name to the CompatibilitySwitch constructor using nameof. + // + // Choose a boolean value or a new enum type as the 'value' of the property. + // + // If the new property has a boolean value, it should be named something like `SuppressFoo` + // (if the new value deactivates some behavior) or like `AllowFoo` (if the new value enables some behavior). + // Choose a name so that the old behavior equates to 'false'. + // + // If it's an enum, make sure you initialize the compatibility switch using the + // CompatibilitySwitch(string, value) constructor to make it obvious the correct value is passed in. It's + // a good idea to equate the original behavior with the default enum value as well. + // + // Then create (or modify) a subclass of ConfigureCompatibilityOptions appropriate for your options type. + // Override the DefaultValues property and provide appropriate values based on the value of the Version + // property. If you just added this class, register it as an IPostConfigureOptions in DI. + // + /// + /// Infrastructure supporting the implementation of . This is an + /// implementation of suitible for use with the + /// pattern. This is framework infrastructure and should not be used by application code. + /// + /// The type of value assoicated with the compatibility switch. + public class CompatibilitySwitch : ICompatibilitySwitch where TValue : struct + { + private TValue _value; + + /// + /// Creates a new compatiblity switch with the provided name. + /// + /// + /// The compatiblity switch name. The name must match a property name on an options type. + /// + public CompatibilitySwitch(string name) + : this(name, default) + { + } + + /// + /// Creates a new compatiblity switch with the provided name and initial value. + /// + /// + /// The compatiblity switch name. The name must match a property name on an options type. + /// + /// + /// The initial value to assign to the switch. + /// + public CompatibilitySwitch(string name, TValue initialValue) + { + if (name == null) + { + throw new ArgumentNullException(nameof(name)); + } + + Name = name; + _value = initialValue; + } + + /// + /// Gets a value indicating whether the property has been set. + /// + /// + /// This is used by the compatibility infrastructure to determine whether the application developer + /// has set explicitly set the value associated with this switch. + /// + public bool IsValueSet { get; private set; } + + /// + /// Gets the name of the compatibility switch. + /// + public string Name { get; } + + /// + /// Gets or set the value associated with the compatibility switch. + /// + /// + /// Setting the switch value using will set to true. + /// As a consequence, the compatibility infrastructure will consider this switch explicitly configured by + /// the application developer, and will not apply a default value based on the compatibility version. + /// + public TValue Value + { + get => _value; + set + { + IsValueSet = true; + _value = value; + } + } + + // Called by the compatibility infrastructure to set a default value when IsValueSet is false. + object ICompatibilitySwitch.Value + { + get => Value; + set => Value = (TValue)value; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ConfigureCompatibilityOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ConfigureCompatibilityOptions.cs new file mode 100644 index 0000000000..c46a3529d0 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ConfigureCompatibilityOptions.cs @@ -0,0 +1,105 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// A base class for infrastructure that implements ASP.NET Core MVC's support for + /// . This is framework infrastructure and should not be used + /// by application code. + /// + /// + public abstract class ConfigureCompatibilityOptions : IPostConfigureOptions + where TOptions : class, IEnumerable + { + private readonly ILogger _logger; + + /// + /// Creates a new . + /// + /// The . + /// The . + protected ConfigureCompatibilityOptions( + ILoggerFactory loggerFactory, + IOptions compatibilityOptions) + { + if (loggerFactory == null) + { + throw new ArgumentNullException(nameof(loggerFactory)); + } + + Version = compatibilityOptions.Value.CompatibilityVersion; + _logger = loggerFactory.CreateLogger(); + } + + /// + /// Gets the default values of compatibility switches associated with the applications configured + /// . + /// + protected abstract IReadOnlyDictionary DefaultValues { get; } + + /// + /// Gets the configured for the application. + /// + protected CompatibilityVersion Version { get; } + + /// + public virtual void PostConfigure(string name, TOptions options) + { + if (name == null) + { + throw new ArgumentNullException(nameof(name)); + } + + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + // Evaluate DefaultValues onces so subclasses don't have to cache. + var defaultValues = DefaultValues; + + foreach (var @switch in options) + { + ConfigureSwitch(@switch, defaultValues); + } + } + + private void ConfigureSwitch(ICompatibilitySwitch @switch, IReadOnlyDictionary defaultValues) + { + if (@switch.IsValueSet) + { + _logger.LogDebug( + "Compatibility switch {SwitchName} in type {OptionsType} is using explicitly configured value {Value}", + @switch.Name, + typeof(TOptions).Name, + @switch.Value); + return; + } + + if (!defaultValues.TryGetValue(@switch.Name, out var value)) + { + _logger.LogDebug( + "Compatibility switch {SwitchName} in type {OptionsType} is using default value {Value}", + @switch.Name, + typeof(TOptions).Name, + @switch.Value, + Version); + return; + } + + @switch.Value = value; + _logger.LogDebug( + "Compatibility switch {SwitchName} in type {OptionsType} is using compatibility value {Value} for version {Version}", + @switch.Name, + typeof(TOptions).Name, + @switch.Value, + Version); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ICompatibilitySwitch.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ICompatibilitySwitch.cs new file mode 100644 index 0000000000..6f723db0e2 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ICompatibilitySwitch.cs @@ -0,0 +1,36 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// Defines a compatibility switch. This is framework infrastructure and should not be used + /// by application code. + /// + public interface ICompatibilitySwitch + { + /// + /// Gets a value indicating whether the property has been set. + /// + /// + /// This is used by the compatibility infrastructure to determine whether the application developer + /// has set explicitly set the value associated with this switch. + /// + bool IsValueSet { get; } + + /// + /// Gets the name of the compatibility switch. + /// + string Name { get; } + + /// + /// Gets or set the value associated with the compatibility switch. + /// + /// + /// Setting the switch value using will not set to true. + /// This should be used by the compatibility infrastructure when is false + /// to apply a compatibility value based on . + /// + object Value { get; set; } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcCompatibilityOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcCompatibilityOptions.cs new file mode 100644 index 0000000000..fb78662ed8 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcCompatibilityOptions.cs @@ -0,0 +1,23 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// An options type for configuring the application . + /// + /// + /// The primary way to configure the application's is by + /// calling + /// or . + /// + public class MvcCompatibilityOptions + { + /// + /// Gets or sets the application's configured . + /// + public CompatibilityVersion CompatibilityVersion { get; set; } = CompatibilityVersion.Version_2_0; + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs new file mode 100644 index 0000000000..e722e91935 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs @@ -0,0 +1,35 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + internal class MvcOptionsConfigureCompatibilityOptions : ConfigureCompatibilityOptions + { + public MvcOptionsConfigureCompatibilityOptions( + ILoggerFactory loggerFactory, + IOptions compatibilityOptions) + : base(loggerFactory, compatibilityOptions) + { + } + + protected override IReadOnlyDictionary DefaultValues + { + get + { + var values = new Dictionary(); + + if (Version >= CompatibilityVersion.Version_2_1) + { + values[nameof(MvcOptions.InputFormatterExceptionModelStatePolicy)] = InputFormatterExceptionModelStatePolicy.MalformedInputExceptions; + } + + return values; + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs index 21c3f81627..b6dab32f73 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs @@ -2,10 +2,12 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections; using System.Collections.Generic; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; @@ -15,10 +17,16 @@ namespace Microsoft.AspNetCore.Mvc /// /// Provides programmatic configuration for the MVC framework. /// - public class MvcOptions + public class MvcOptions : IEnumerable { private int _maxModelStateErrors = ModelStateDictionary.DefaultMaxAllowedErrors; + // See CompatibilitySwitch.cs for guide on how to implement these. + private readonly CompatibilitySwitch _allowBindingUndefinedValueToEnumType; + private readonly CompatibilitySwitch _inputFormatterExceptionModelStatePolicy; + private readonly CompatibilitySwitch _suppressJsonDeserializationExceptionMessagesInModelState; + private readonly ICompatibilitySwitch[] _switches; + public MvcOptions() { CacheProfiles = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -32,6 +40,16 @@ namespace Microsoft.AspNetCore.Mvc ModelMetadataDetailsProviders = new List(); ModelValidatorProviders = new List(); ValueProviderFactories = new List(); + + _allowBindingUndefinedValueToEnumType = new CompatibilitySwitch(nameof(AllowBindingUndefinedValueToEnumType)); + _inputFormatterExceptionModelStatePolicy = new CompatibilitySwitch(nameof(InputFormatterExceptionModelStatePolicy), InputFormatterExceptionModelStatePolicy.AllExceptions); + _suppressJsonDeserializationExceptionMessagesInModelState = new CompatibilitySwitch(nameof(SuppressJsonDeserializationExceptionMessagesInModelState)); + _switches = new ICompatibilitySwitch[] + { + _allowBindingUndefinedValueToEnumType, + _inputFormatterExceptionModelStatePolicy, + _suppressJsonDeserializationExceptionMessagesInModelState, + }; } /// @@ -167,7 +185,11 @@ namespace Microsoft.AspNetCore.Mvc /// Gets or sets an indication whether the model binding system will bind undefined values to enumeration types. /// by default. /// - public bool AllowBindingUndefinedValueToEnumType { get; set; } + public bool AllowBindingUndefinedValueToEnumType + { + get => _allowBindingUndefinedValueToEnumType.Value; + set => _allowBindingUndefinedValueToEnumType.Value = value; + } /// /// Gets or sets the option to determine if model binding should convert all exceptions (including ones not related to bad input) @@ -175,7 +197,11 @@ namespace Microsoft.AspNetCore.Mvc /// This option applies only to custom s. /// Default is . /// - public InputFormatterExceptionModelStatePolicy InputFormatterExceptionModelStatePolicy { get; set; } + public InputFormatterExceptionModelStatePolicy InputFormatterExceptionModelStatePolicy + { + get => _inputFormatterExceptionModelStatePolicy.Value; + set => _inputFormatterExceptionModelStatePolicy.Value = value; + } /// /// Gets or sets a flag to determine whether, if an action receives invalid JSON in @@ -184,6 +210,17 @@ namespace Microsoft.AspNetCore.Mvc /// by default, meaning that clients may receive details about /// why the JSON they posted is considered invalid. /// - public bool SuppressJsonDeserializationExceptionMessagesInModelState { get; set; } = false; + public bool SuppressJsonDeserializationExceptionMessagesInModelState + { + get => _suppressJsonDeserializationExceptionMessagesInModelState.Value; + set => _suppressJsonDeserializationExceptionMessagesInModelState.Value = value; + } + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)_switches).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator(); } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs index 59ddee1789..237eb47046 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; @@ -241,6 +242,13 @@ namespace Microsoft.AspNetCore.Mvc typeof(MvcCoreMvcOptionsSetup), } }, + { + typeof(IPostConfigureOptions), + new Type[] + { + typeof(MvcOptionsConfigureCompatibilityOptions), + } + }, { typeof(IConfigureOptions), new Type[] diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/CompatibilitySwitchTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/CompatibilitySwitchTest.cs new file mode 100644 index 0000000000..119113f668 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/CompatibilitySwitchTest.cs @@ -0,0 +1,60 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + public class CompatibilitySwitchTest + { + [Fact] + public void Constructor_WithName_IsValueSetIsFalse() + { + // Arrange & Act + var @switch = new CompatibilitySwitch("TestProperty"); + + // Assert + Assert.False(@switch.Value); + Assert.False(@switch.IsValueSet); + } + + [Fact] + public void Constructor_WithNameAndInitalValue_IsValueSetIsFalse() + { + // Arrange & Act + var @switch = new CompatibilitySwitch("TestProperty", initialValue: true); + + // Assert + Assert.True(@switch.Value); + Assert.False(@switch.IsValueSet); + } + + [Fact] + public void ValueNonInterface_SettingValue_SetsIsValueSetToTrue() + { + // Arrange + var @switch = new CompatibilitySwitch("TestProperty"); + + // Act + @switch.Value = false; // You don't need to actually change the value, just caling the setting works + + // Assert + Assert.False(@switch.Value); + Assert.True(@switch.IsValueSet); + } + + [Fact] + public void ValueInterface_SettingValue_SetsIsValueSetToTrue() + { + // Arrange + var @switch = new CompatibilitySwitch("TestProperty"); + + // Act + ((ICompatibilitySwitch)@switch).Value = true; + + // Assert + Assert.True(@switch.Value); + Assert.True(@switch.IsValueSet); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ConfigureCompatibilityOptionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ConfigureCompatibilityOptionsTest.cs new file mode 100644 index 0000000000..a666303be8 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ConfigureCompatibilityOptionsTest.cs @@ -0,0 +1,117 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections; +using System.Collections.Generic; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + public class ConfigureCompatibilityOptionsTest + { + [Fact] + public void PostConfigure_NoValueForProperty_DoesNothing() + { + // Arrange + var configure = Create(CompatibilityVersion.Version_2_0, new Dictionary()); + + var options = new TestOptions(); + + // Act + configure.PostConfigure(Options.DefaultName, options); + + // Assert + Assert.False(options.TestProperty); + } + + [Fact] + public void PostConfigure_ValueIsSet_DoesNothing() + { + // Arrange + var configure = Create(CompatibilityVersion.Version_2_0, new Dictionary() + { + { nameof(TestOptions.TestProperty), true }, + }); + + var options = new TestOptions() + { + TestProperty = false, + }; + + // Act + configure.PostConfigure(Options.DefaultName, options); + + // Assert + Assert.False(options.TestProperty); + } + + [Fact] + public void PostConfigure_ValueNotSet_SetsValue() + { + // Arrange + var configure = Create(CompatibilityVersion.Version_2_0, new Dictionary() + { + { nameof(TestOptions.TestProperty), true }, + }); + + var options = new TestOptions(); + + // Act + configure.PostConfigure(Options.DefaultName, options); + + // Assert + Assert.True(options.TestProperty); + } + + private static ConfigureCompatibilityOptions Create( + CompatibilityVersion version, + IReadOnlyDictionary defaultValues) + { + var compatibilityOptions = Options.Create(new MvcCompatibilityOptions() { CompatibilityVersion = version }); + return new TestConfigure(NullLoggerFactory.Instance, compatibilityOptions, defaultValues); + } + + private class TestOptions : IEnumerable + { + private readonly CompatibilitySwitch _testProperty; + + private readonly ICompatibilitySwitch[] _switches; + + public TestOptions() + { + _testProperty = new CompatibilitySwitch(nameof(TestProperty)); + _switches = new ICompatibilitySwitch[] { _testProperty }; + } + + public bool TestProperty + { + get => _testProperty.Value; + set => _testProperty.Value = value; + } + + public IEnumerator GetEnumerator() + { + return ((IEnumerable)_switches).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator(); + } + + private class TestConfigure : ConfigureCompatibilityOptions + { + public TestConfigure( + ILoggerFactory loggerFactory, + IOptions compatibilityOptions, + IReadOnlyDictionary defaultValues) + : base(loggerFactory, compatibilityOptions) + { + DefaultValues = defaultValues; + } + + protected override IReadOnlyDictionary DefaultValues { get; } + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs new file mode 100644 index 0000000000..102fd5a316 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs @@ -0,0 +1,83 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.ObjectPool; +using Microsoft.Extensions.Options; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.IntegrationTest +{ + // Integration tests for compatibility switches. These tests verify which compatibility + // values apply to each supported version. + // + // If you add a new compatibility switch, make sure to update ALL of these tests. Each test + // here should include verification for all of the switches. + public class CompatibilitySwitchIntegrationTest + { + [Fact(Skip = "#7157 - some settings have the wrong values, this test should pass once #7157 is fixed")] + public void CompatibilitySwitches_Version_2_0() + { + // Arrange + var serviceCollection = new ServiceCollection(); + AddHostingServices(serviceCollection); + serviceCollection.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_0); + + var services = serviceCollection.BuildServiceProvider(); + + // Act + var mvcOptions = services.GetRequiredService>().Value; + + // Assert + Assert.False(mvcOptions.AllowBindingUndefinedValueToEnumType); + Assert.Equal(InputFormatterExceptionModelStatePolicy.AllExceptions, mvcOptions.InputFormatterExceptionModelStatePolicy); + Assert.False(mvcOptions.SuppressJsonDeserializationExceptionMessagesInModelState); // This name needs to be inverted in #7157 + } + + [Fact(Skip = "#7157 - some settings have the wrong values, this test should pass once #7157 is fixed")] + public void CompatibilitySwitches_Version_2_1() + { + // Arrange + var serviceCollection = new ServiceCollection(); + AddHostingServices(serviceCollection); + serviceCollection.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_0); + + var services = serviceCollection.BuildServiceProvider(); + + // Act + var mvcOptions = services.GetRequiredService>().Value; + + // Assert + Assert.True(mvcOptions.AllowBindingUndefinedValueToEnumType); + Assert.Equal(InputFormatterExceptionModelStatePolicy.MalformedInputExceptions, mvcOptions.InputFormatterExceptionModelStatePolicy); + Assert.True(mvcOptions.SuppressJsonDeserializationExceptionMessagesInModelState); // This name needs to be inverted in #7157 + } + + [Fact(Skip = "#7157 - some settings have the wrong values, this test should pass once #7157 is fixed")] + public void CompatibilitySwitches_Version_Latest() + { + // Arrange + var serviceCollection = new ServiceCollection(); + AddHostingServices(serviceCollection); + serviceCollection.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_0); + + var services = serviceCollection.BuildServiceProvider(); + + // Act + var mvcOptions = services.GetRequiredService>().Value; + + // Assert + Assert.True(mvcOptions.AllowBindingUndefinedValueToEnumType); + Assert.Equal(InputFormatterExceptionModelStatePolicy.MalformedInputExceptions, mvcOptions.InputFormatterExceptionModelStatePolicy); + Assert.True(mvcOptions.SuppressJsonDeserializationExceptionMessagesInModelState); // This name needs to be inverted in #7157 + } + + // This just does the minimum needed to be able to resolve these options. + private static void AddHostingServices(IServiceCollection serviceCollection) + { + serviceCollection.AddLogging(); + serviceCollection.AddSingleton(); + } + } +}