EnableDataCollection and DisplayBreakingChangeWarnings configs (#318)

* configs: data collection; survey; warning message

* revert survey config

* do not obsolete AzurePSDataCollectionProfile(bool enable)

* remove s in DisplayBreakingChangeWarnings

* Add a config scope: Environment

* refactor breaking change / preview attribute

* add scope to config filter

Author: isra-fel

src/Authentication.Abstractions/AzurePSDataCollectionProfile.cs

@@ -12,7 +12,7 @@
 // limitations under the License.
 // ----------------------------------------------------------------------------------
 
-using Newtonsoft.Json;
+using Microsoft.Azure.PowerShell.Common.Config;
 using System;
 
 namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions
@@ -23,16 +23,40 @@ public class AzurePSDataCollectionProfile
             OldDefaultFileName = "AzureDataCollectionProfile.json",
             DefaultFileName = "AzurePSDataCollectionProfile.json";
 
+        private bool? _enableAzureDataCollection = null;
+
+        /// <summary>
+        /// Creates a data collection profile who relies on config API to determine whether it is enabled.
+        /// </summary>
         public AzurePSDataCollectionProfile()
         {
         }
 
+        /// <summary>
+        /// Creates a data collection profile with a predefined state of whether it is enabled.
+        /// </summary>
         public AzurePSDataCollectionProfile(bool enable)
         {
-            EnableAzureDataCollection = enable;
+            _enableAzureDataCollection = enable;
         }
 
-        [JsonProperty(PropertyName = "enableAzureDataCollection")]
-        public bool? EnableAzureDataCollection { get; set; }
+        public bool? EnableAzureDataCollection {
+            get
+            {
+                if (_enableAzureDataCollection.HasValue)
+                {
+                    return _enableAzureDataCollection.Value;
+                }
+                if (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager))
+                {
+                    _enableAzureDataCollection = configManager.GetConfigValue<bool>(ConfigKeysForCommon.EnableDataCollection);
+                }
+                return _enableAzureDataCollection;
+            }
+            set
+            {
+                throw new NotSupportedException("Setting data collection directly is not supported. Use Config API instead.");
+            }
+        }
     }
 }

src/Authentication.Abstractions/DataCollectionController.cs

@@ -12,9 +12,8 @@
 // limitations under the License.
 // ----------------------------------------------------------------------------------
 
-using Newtonsoft.Json;
+using Microsoft.Azure.PowerShell.Common.Config;
 using System;
-using System.IO;
 
 namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions
 {
@@ -25,60 +24,12 @@ public abstract class DataCollectionController
 
         static AzurePSDataCollectionProfile Initialize(IAzureSession session)
         {
-            AzurePSDataCollectionProfile result = new AzurePSDataCollectionProfile(true);
-            try
-            {
-                var environmentValue = Environment.GetEnvironmentVariable(AzurePSDataCollectionProfile.EnvironmentVariableName);
-                bool enabled = true;
-                if (!string.IsNullOrWhiteSpace(environmentValue) && bool.TryParse(environmentValue, out enabled))
-                {
-                    result.EnableAzureDataCollection = enabled;
-                }
-                else
-                {
-                    var store = session.DataStore;
-                    string dataPath = Path.Combine(session.ProfileDirectory, AzurePSDataCollectionProfile.DefaultFileName);
-                    if (store.FileExists(dataPath))
-                    {
-                        string contents = store.ReadFileAsText(dataPath);
-                        var localResult = JsonConvert.DeserializeObject<AzurePSDataCollectionProfile>(contents);
-                        if (localResult != null && localResult.EnableAzureDataCollection.HasValue)
-                        {
-                            result = localResult;
-                        }
-                    }
-                    else
-                    {
-                        WritePSDataCollectionProfile(session, new AzurePSDataCollectionProfile(true));
-                    }
-                }
-            }
-            catch
-            {
-                // do not throw for i/o or serialization errors
-            }
-
-            return result;
+            return new AzurePSDataCollectionProfile();
         }
 
+        [Obsolete("Use config API to update data collection settings.")]
         public static void WritePSDataCollectionProfile(IAzureSession session, AzurePSDataCollectionProfile profile)
         {
-            try
-            {
-                var store = session.DataStore;
-                string dataPath = Path.Combine(session.ProfileDirectory, AzurePSDataCollectionProfile.DefaultFileName);
-                if (!store.DirectoryExists(session.ProfileDirectory))
-                {
-                    store.CreateDirectory(session.ProfileDirectory);
-                }
-
-                string contents = JsonConvert.SerializeObject(profile);
-                store.WriteFile(dataPath, contents);
-            }
-            catch
-            {
-                // do not throw for i/o or serialization errors
-            }
         }
 
         public static DataCollectionController Create(IAzureSession session)
@@ -93,33 +44,17 @@ public static DataCollectionController Create(AzurePSDataCollectionProfile profi
 
         class MemoryDataCollectionController : DataCollectionController
         {
-            object _lock;
-            bool? _enabled;
-
             public MemoryDataCollectionController()
             {
-                _lock = new object();
-                _enabled = null;
             }
 
             public MemoryDataCollectionController(AzurePSDataCollectionProfile enabled)
             {
-                _lock = new object();
-                _enabled = enabled?.EnableAzureDataCollection;
             }
 
             public override AzurePSDataCollectionProfile GetProfile(Action warningWriter)
             {
-                lock (_lock)
-                {
-                    if (!_enabled.HasValue)
-                    {
-                        _enabled = true;
-                        warningWriter();
-                    }
-
-                    return new AzurePSDataCollectionProfile(_enabled.Value);
-                }
+                return new AzurePSDataCollectionProfile();
             }
         }
     }

src/Authentication.Abstractions/Interfaces/IConfigManager.cs

@@ -12,7 +12,6 @@
 // limitations under the License.
 // ----------------------------------------------------------------------------------
 
-using System;
 using System.Collections.Generic;
 
 namespace Microsoft.Azure.PowerShell.Common.Config

src/Authentication.Abstractions/Models/ConfigDefinition.cs

@@ -43,7 +43,35 @@ public abstract class ConfigDefinition
         /// <summary>
         /// Gets the name of the environment variable that can control this config.
         /// </summary>
-        public virtual string EnvironmentVariableName { get; } = null;
+        /// <remarks>
+        /// For most cases, where the config is connected to only 1 environment variable,
+        /// and the value can be parsed without extra logic, use this property,
+        /// else override <see cref="ParseFromEnvironmentVariables(IReadOnlyDictionary{string, string})"/>.
+        /// </remarks>
+        protected virtual string EnvironmentVariableName { get; } = null;
+
+        /// <summary>
+        /// Customizes how to parse config value from environment variables.
+        /// </summary>
+        /// <param name="environmentVariables">All the environment variables.</param>
+        /// <returns>The parsed config value, in string. Returns null if the environment variable of this config is not set.</returns>
+        /// <remarks>
+        /// Note: use <see cref="EnvironmentVariableName"/> if there's no need for customized parsing logic. <br />
+        /// The return type is string because it will be further parsed into other types (int, bool...) by the config provider.
+        /// Make sure the return value matches <see cref="ValueType"/>.
+        /// For example if <see cref="ValueType"/> is bool, the return value cannot be like "Disabled".
+        /// </remarks>
+        public virtual string ParseFromEnvironmentVariables(IReadOnlyDictionary<string, string> environmentVariables)
+        {
+            if (string.IsNullOrEmpty(EnvironmentVariableName))
+            {
+                return null;
+            }
+            else
+            {
+                return environmentVariables.TryGetValue(EnvironmentVariableName, out string value) ? value : null;
+            }
+        }
 
         /// <summary>
         /// Gets how the config can be applied to.

src/Authentication.Abstractions/Models/ConfigFilter.cs

@@ -42,5 +42,11 @@ public class ConfigFilter
         /// - Name of a cmdlet: the config applies to a certain cmdlet of Azure PowerShell. For example, "Get-AzKeyVault".
         /// </remarks>
         public string AppliesTo { get; set; } = null;
+
+        /// <summary>
+        /// Scope of the configs to filter. By default all scopes are included.
+        /// </summary>
+        /// <value></value>
+        public ConfigScope? Scope { get; set; } = null;
     }
 }

src/Authentication.Abstractions/Models/ConfigKeysForCommon.cs

@@ -0,0 +1,31 @@
+// ----------------------------------------------------------------------------------
+//
+// Copyright Microsoft Corporation
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// ----------------------------------------------------------------------------------
+
+namespace Microsoft.Azure.PowerShell.Common.Config
+{
+    /// <summary>
+    /// This class stores keys of pre-defined configs.
+    /// </summary>
+    /// <remarks>
+    /// All keys should be defined in ConfigKeys class in Azure/azure-powershell repo.
+    /// If the key is used in common code, duplicate it here.
+    /// Keys defined here should NEVER be removed or changed to prevent breaking change.
+    /// </remarks>
+    public static class ConfigKeysForCommon
+    {
+        public const string EnableInterceptSurvey = "EnableInterceptSurvey";
+        public const string DisplayBreakingChangeWarning = "DisplayBreakingChangeWarning";
+        public const string EnableDataCollection = "EnableDataCollection";
+    }
+}

src/Authentication.Abstractions/Models/ConfigScope.cs

@@ -20,7 +20,7 @@ namespace Microsoft.Azure.PowerShell.Common.Config
     public enum ConfigScope
     {
         /// <summary>
-        /// Config will be persitent on the disk, available for all the PowerShell sessions initiated by the current user.
+        /// Config will be persistent on the disk, available for all the PowerShell sessions initiated by the current user.
         /// </summary>
         CurrentUser,
 
@@ -33,6 +33,12 @@ public enum ConfigScope
         /// Config is never set.
         /// </summary>
         /// <remarks>This option is not available when updating or clearing a config.</remarks>
-        Default
+        Default,
+
+        /// <summary>
+        /// Config is set by environment variables.
+        /// </summary>
+        /// <remarks>This option is not available when updating or clearing a config.</remarks>
+        Environment
     }
 }

src/Common/AzurePSCmdlet.cs

@@ -14,6 +14,7 @@
 
 using Microsoft.Azure.Commands.Common.Authentication;
 using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
+using Microsoft.Azure.PowerShell.Common.Config;
 using Microsoft.Azure.PowerShell.Common.Share.Survey;
 using Microsoft.Azure.ServiceManagement.Common.Models;
 using Microsoft.WindowsAzure.Commands.Common;
@@ -65,7 +66,7 @@ protected AzurePSDataCollectionProfile _dataCollectionProfile
                     }
                     else if (_cachedProfile == null)
                     {
-                        _cachedProfile = new AzurePSDataCollectionProfile(true);
+                        _cachedProfile = new AzurePSDataCollectionProfile();
                         WriteWarning(DataCollectionWarning);
                     }
 
@@ -361,8 +362,17 @@ protected override void BeginProcessing()
 
             //Now see if the cmdlet has any Breaking change attributes on it and process them if it does
             //This will print any breaking change attribute messages that are applied to the cmdlet
-            BreakingChangeAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
-            PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteDebug);
+            WriteBreakingChangeOrPreviewMessage();
+        }
+
+        private void WriteBreakingChangeOrPreviewMessage()
+        {
+            if (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
+                && configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisplayBreakingChangeWarning))
+            {
+                BreakingChangeAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
+                PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteDebug);
+            }
         }
 
         /// <summary>
@@ -451,7 +461,7 @@ protected void WriteSurvey()
                     Message = "Open-AzSurveyLink",
                     NoNewLine = true,
                 };
-            } 
+            }
             HostInformationMessage action = new HostInformationMessage()
             {
                 Message = " to fill out a short Survey"
@@ -471,7 +481,11 @@ protected void WriteSurvey()
                 _qosEvent.IsSuccess = false;
             }
             base.WriteError(errorRecord);
-            PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
+            if (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
+                && configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisplayBreakingChangeWarning))
+            {
+                PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
+            }
         }
 
         protected new void ThrowTerminatingError(ErrorRecord errorRecord)