removing call to Configure() for FunctionsHostingConfigOptions (#9891)

brettsam · web-flow · commit 1c0a2ef7b44e · 2024-02-28T08:47:36.000-08:00
diff --git a/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs b/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs
@@ -208,11 +208,7 @@ public static void AddWebJobsScriptHost(this IServiceCollection services, IConfi
             services.ConfigureOptionsWithChangeTokenSource<HttpBodyControlOptions, HttpBodyControlOptionsSetup, SpecializationChangeTokenSource<HttpBodyControlOptions>>();
             services.ConfigureOptions<FlexConsumptionMetricsPublisherOptionsSetup>();
             services.ConfigureOptions<ConsoleLoggingOptionsSetup>();
-            services.ConfigureOptions<FunctionsHostingConfigOptionsSetup>();
-            if (configuration != null)
-            {
-                services.Configure<FunctionsHostingConfigOptions>(configuration.GetSection(ScriptConstants.FunctionsHostingConfigSectionName));
-            }
+            services.AddHostingConfigOptions(configuration);
 
             services.TryAddSingleton<IDependencyValidator, DependencyValidator>();
             services.TryAddSingleton<IJobHostMiddlewarePipeline>(s => DefaultMiddlewarePipeline.Empty);
@@ -221,6 +217,18 @@ public static void AddWebJobsScriptHost(this IServiceCollection services, IConfi
             services.AddAzureBlobStorageProvider();
         }
 
+        internal static void AddHostingConfigOptions(this IServiceCollection services, IConfiguration configuration)
+        {
+            services.ConfigureOptions<FunctionsHostingConfigOptionsSetup>();
+
+            if (configuration != null)
+            {
+                // Refresh IOptionsMonitor<FunctionsHostingConfigOptions> when the config section changes
+                var configSection = configuration.GetSection(ScriptConstants.FunctionsHostingConfigSectionName);
+                services.AddSingleton<IOptionsChangeTokenSource<FunctionsHostingConfigOptions>>(new ConfigurationChangeTokenSource<FunctionsHostingConfigOptions>(configSection));
+            }
+        }
+
         private static void AddStandbyServices(this IServiceCollection services)
         {
             services.AddSingleton<IOptionsChangeTokenSource<StandbyOptions>, StandbyChangeTokenSource>();
diff --git a/src/WebJobs.Script/Config/FunctionsHostingConfigOptions.cs b/src/WebJobs.Script/Config/FunctionsHostingConfigOptions.cs
@@ -24,7 +24,7 @@ public FunctionsHostingConfigOptions()
         /// <summary>
         /// Gets a value indicating whether worker concurrency feature is enabled in the hosting config.
         /// </summary>
-        public bool FunctionsWorkerDynamicConcurrencyEnabled
+        internal bool FunctionsWorkerDynamicConcurrencyEnabled
         {
             get
             {
@@ -35,7 +35,7 @@ public bool FunctionsWorkerDynamicConcurrencyEnabled
         /// <summary>
         /// Gets a value indicating whether worker indexing feature is enabled in the hosting config.
         /// </summary>
-        public bool WorkerIndexingEnabled
+        internal bool WorkerIndexingEnabled
         {
             get
             {
@@ -46,11 +46,11 @@ public bool WorkerIndexingEnabled
         /// <summary>
         /// Gets or Sets a value indicating whether the host should shutdown webhost worker channels during shutdown.
         /// </summary>
-        public bool ShutdownWebhostWorkerChannelsOnHostShutdown
+        internal bool ShutdownWebhostWorkerChannelsOnHostShutdown
         {
             get
             {
-                return GetFeatureOrDefault(RpcWorkerConstants.ShutdownWebhostWorkerChannelsOnHostShutdown, "1") == "1";
+                return GetFeatureAsBooleanOrDefault(RpcWorkerConstants.ShutdownWebhostWorkerChannelsOnHostShutdown, true);
             }
 
             set
@@ -62,11 +62,11 @@ public bool ShutdownWebhostWorkerChannelsOnHostShutdown
         /// <summary>
         /// Gets or sets a value indicating whether SWT tokens should be accepted.
         /// </summary>
-        public bool SwtAuthenticationEnabled
+        internal bool SwtAuthenticationEnabled
         {
             get
             {
-                return GetFeatureOrDefault(ScriptConstants.HostingConfigSwtAuthenticationEnabled, "1") == "1";
+                return GetFeatureAsBooleanOrDefault(ScriptConstants.HostingConfigSwtAuthenticationEnabled, true);
             }
 
             set
@@ -78,11 +78,11 @@ public bool SwtAuthenticationEnabled
         /// <summary>
         /// Gets or sets a value indicating whether SWT tokens should be sent on outgoing requests.
         /// </summary>
-        public bool SwtIssuerEnabled
+        internal bool SwtIssuerEnabled
         {
             get
             {
-                return GetFeatureOrDefault(ScriptConstants.HostingConfigSwtIssuerEnabled, "1") == "1";
+                return GetFeatureAsBooleanOrDefault(ScriptConstants.HostingConfigSwtIssuerEnabled, true);
             }
 
             set
@@ -94,7 +94,7 @@ public bool SwtIssuerEnabled
         /// <summary>
         /// Gets a string delimited by '|' that contains the name of the apps with worker indexing disabled.
         /// </summary>
-        public string WorkerIndexingDisabledApps
+        internal string WorkerIndexingDisabledApps
         {
             get
             {
@@ -105,7 +105,7 @@ public string WorkerIndexingDisabledApps
         /// <summary>
         /// Gets a value indicating whether Linux Log Backoff is disabled in the hosting config.
         /// </summary>
-        public bool DisableLinuxAppServiceLogBackoff
+        internal bool DisableLinuxAppServiceLogBackoff
         {
             get
             {
@@ -116,7 +116,7 @@ public bool DisableLinuxAppServiceLogBackoff
         /// <summary>
         /// Gets or sets a value indicating whether Linux AppService/EP Detailed Execution Event is disabled in the hosting config.
         /// </summary>
-        public bool DisableLinuxAppServiceExecutionDetails
+        internal bool DisableLinuxAppServiceExecutionDetails
         {
             get
             {
@@ -129,11 +129,11 @@ public bool DisableLinuxAppServiceExecutionDetails
             }
         }
 
-        public bool EnableOrderedInvocationMessages
+        internal bool EnableOrderedInvocationMessages
         {
             get
             {
-                return GetFeature(ScriptConstants.FeatureFlagEnableOrderedInvocationmessages) == "1";
+                return GetFeatureAsBooleanOrDefault(ScriptConstants.FeatureFlagEnableOrderedInvocationmessages, false);
             }
 
             set
@@ -145,7 +145,7 @@ public bool EnableOrderedInvocationMessages
         /// <summary>
         /// Gets the highest version of extension bundle v3 supported.
         /// </summary>
-        public string MaximumBundleV3Version
+        internal string MaximumBundleV3Version
         {
             get
             {
@@ -156,7 +156,7 @@ public string MaximumBundleV3Version
         /// <summary>
         /// Gets the highest version of extension bundle v4 supported.
         /// </summary>
-        public string MaximumBundleV4Version
+        internal string MaximumBundleV4Version
         {
             get
             {
@@ -167,15 +167,15 @@ public string MaximumBundleV4Version
         /// <summary>
         /// Gets a value indicating whether the host should revert the worker shutdown behavior in the WebHostWorkerChannelManager.
         /// </summary>
-        public bool RevertWorkerShutdownBehavior
+        internal bool RevertWorkerShutdownBehavior
         {
             get
             {
                 return GetFeature(RpcWorkerConstants.RevertWorkerShutdownBehavior) == "1";
             }
         }
 
-        public bool ThrowOnMissingFunctionsWorkerRuntime
+        internal bool ThrowOnMissingFunctionsWorkerRuntime
         {
             get
             {
@@ -207,5 +207,35 @@ public string GetFeatureOrDefault(string name, string defaultValue)
         {
             return GetFeature(name) ?? defaultValue;
         }
+
+        /// <summary>
+        /// Gets a feature by name and attempts to parse it into a boolean.
+        /// Returns "True, False" or ints as booleans. Returns True for any non-zero integer.
+        /// If none of those (or empty), returns default.
+        /// </summary>
+        /// <param name="name">Feature name.</param>
+        /// <param name="defaultValue">The default value to return if the feature value cannot be parsed into a boolean.</param>
+        /// <returns>Boolean value if parse-able from hosting configuration. Otherwise, the defaultValue.</returns>
+        internal bool GetFeatureAsBooleanOrDefault(string name, bool defaultValue)
+        {
+            string featureValue = GetFeature(name);
+
+            if (string.IsNullOrWhiteSpace(featureValue))
+            {
+                return defaultValue;
+            }
+
+            if (bool.TryParse(featureValue, out bool parsedBool))
+            {
+                return parsedBool;
+            }
+
+            if (int.TryParse(featureValue, out int parsedInt))
+            {
+                return parsedInt != 0;
+            }
+
+            return defaultValue;
+        }
     }
-}
+}
diff --git a/test/WebJobs.Script.Tests.Integration/WebHostEndToEnd/SamplesEndToEndTests_Node_MultipleProcessesNoBundle.cs b/test/WebJobs.Script.Tests.Integration/WebHostEndToEnd/SamplesEndToEndTests_Node_MultipleProcessesNoBundle.cs
@@ -6,12 +6,8 @@
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
-using System.Net;
-using System.Net.Http;
-using System.Net.Http.Headers;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.Azure.WebJobs.Script.Config;
 using Microsoft.Azure.WebJobs.Script.Workers.Rpc;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.WebJobs.Script.Tests;
@@ -23,35 +19,31 @@ namespace Microsoft.Azure.WebJobs.Script.Tests.EndToEnd
     [Trait(TestTraits.Group, TestTraits.SamplesEndToEnd)]
     public class SamplesEndToEndTests_Node_MultipleProcessesNoBundle : IClassFixture<SamplesEndToEndTests_Node_MultipleProcessesNoBundle.MultiplepleProcessesTestFixtureNoBundles>
     {
-        private readonly ScriptSettingsManager _settingsManager;
-        private MultiplepleProcessesTestFixtureNoBundles _fixture;
-        private IEnumerable<int> _nodeProcessesBeforeTestStarted;
+        private readonly MultiplepleProcessesTestFixtureNoBundles _fixture;
 
         public SamplesEndToEndTests_Node_MultipleProcessesNoBundle(MultiplepleProcessesTestFixtureNoBundles fixture)
         {
             _fixture = fixture;
-            _nodeProcessesBeforeTestStarted = fixture.NodeProcessesBeforeTestStarted;
-            _settingsManager = ScriptSettingsManager.Instance;
         }
 
         [Fact]
         public async Task NodeProcessNoBundleConfigured_Different_AfterHostRestart()
         {
             await SamplesTestHelpers.InvokeAndValidateHttpTrigger(_fixture, "HttpTrigger");
-            IEnumerable<int> nodeProcessesBeforeHostRestart = Process.GetProcessesByName("node").Select(p => p.Id);
+            IEnumerable<int> nodeProcessesBeforeHostRestart = Process.GetProcessesByName("node").Select(p => p.Id).ToArray();
             // Trigger a restart
             await _fixture.Host.RestartAsync(CancellationToken.None);
 
             await SamplesTestHelpers.InvokeAndValidateHttpTrigger(_fixture, "HttpTrigger");
 
             // Wait for all the 3 process to start
-            await Task.Delay(TimeSpan.FromMinutes(1));
-
-            IEnumerable<int> nodeProcessesAfter = Process.GetProcessesByName("node").Select(p => p.Id);
-
-            // Verify node process is different after host restart
-            var result = nodeProcessesAfter.Where(pId1 => !nodeProcessesBeforeHostRestart.Any(pId2 => pId2 == pId1) && !_fixture.NodeProcessesBeforeTestStarted.Any(pId3 => pId3 == pId1));
-            Assert.Equal(3, result.Count());
+            await TestHelpers.Await(() =>
+                {
+                    IEnumerable<int> nodeProcessesAfter = Process.GetProcessesByName("node").Select(p => p.Id);
+                    // Verify node process is different after host restart
+                    var result = nodeProcessesAfter.Where(pId1 => !nodeProcessesBeforeHostRestart.Any(pId2 => pId2 == pId1) && !_fixture.NodeProcessesBeforeTestStarted.Any(pId3 => pId3 == pId1));
+                    return result.Count() == 3;
+                });
         }
 
         public class MultiplepleProcessesTestFixtureNoBundles : EndToEndTestFixture
diff --git a/test/WebJobs.Script.Tests/Configuration/FunctionsHostingConfigOptionsTest.cs b/test/WebJobs.Script.Tests/Configuration/FunctionsHostingConfigOptionsTest.cs

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ public FunctionsHostingConfigOptions()`
`24`	`24`	`/// <summary>`
`25`	`25`	`/// Gets a value indicating whether worker concurrency feature is enabled in the hosting config.`
`26`	`26`	`/// </summary>`
`27`		`- public bool FunctionsWorkerDynamicConcurrencyEnabled`
	`27`	`+ internal bool FunctionsWorkerDynamicConcurrencyEnabled`
`28`	`28`	`{`
`29`	`29`	`get`
`30`	`30`	`{`
`@@ -35,7 +35,7 @@ public bool FunctionsWorkerDynamicConcurrencyEnabled`
`35`	`35`	`/// <summary>`
`36`	`36`	`/// Gets a value indicating whether worker indexing feature is enabled in the hosting config.`
`37`	`37`	`/// </summary>`
`38`		`- public bool WorkerIndexingEnabled`
	`38`	`+ internal bool WorkerIndexingEnabled`
`39`	`39`	`{`
`40`	`40`	`get`
`41`	`41`	`{`
`@@ -46,11 +46,11 @@ public bool WorkerIndexingEnabled`
`46`	`46`	`/// <summary>`
`47`	`47`	`/// Gets or Sets a value indicating whether the host should shutdown webhost worker channels during shutdown.`
`48`	`48`	`/// </summary>`
`49`		`- public bool ShutdownWebhostWorkerChannelsOnHostShutdown`
	`49`	`+ internal bool ShutdownWebhostWorkerChannelsOnHostShutdown`
`50`	`50`	`{`
`51`	`51`	`get`
`52`	`52`	`{`
`53`		`- return GetFeatureOrDefault(RpcWorkerConstants.ShutdownWebhostWorkerChannelsOnHostShutdown, "1") == "1";`
	`53`	`+ return GetFeatureAsBooleanOrDefault(RpcWorkerConstants.ShutdownWebhostWorkerChannelsOnHostShutdown, true);`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`set`
`@@ -62,11 +62,11 @@ public bool ShutdownWebhostWorkerChannelsOnHostShutdown`
`62`	`62`	`/// <summary>`
`63`	`63`	`/// Gets or sets a value indicating whether SWT tokens should be accepted.`
`64`	`64`	`/// </summary>`
`65`		`- public bool SwtAuthenticationEnabled`
	`65`	`+ internal bool SwtAuthenticationEnabled`
`66`	`66`	`{`
`67`	`67`	`get`
`68`	`68`	`{`
`69`		`- return GetFeatureOrDefault(ScriptConstants.HostingConfigSwtAuthenticationEnabled, "1") == "1";`
	`69`	`+ return GetFeatureAsBooleanOrDefault(ScriptConstants.HostingConfigSwtAuthenticationEnabled, true);`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`set`
`@@ -78,11 +78,11 @@ public bool SwtAuthenticationEnabled`
`78`	`78`	`/// <summary>`
`79`	`79`	`/// Gets or sets a value indicating whether SWT tokens should be sent on outgoing requests.`
`80`	`80`	`/// </summary>`
`81`		`- public bool SwtIssuerEnabled`
	`81`	`+ internal bool SwtIssuerEnabled`
`82`	`82`	`{`
`83`	`83`	`get`
`84`	`84`	`{`
`85`		`- return GetFeatureOrDefault(ScriptConstants.HostingConfigSwtIssuerEnabled, "1") == "1";`
	`85`	`+ return GetFeatureAsBooleanOrDefault(ScriptConstants.HostingConfigSwtIssuerEnabled, true);`
`86`	`86`	`}`
`87`	`87`
`88`	`88`	`set`
`@@ -94,7 +94,7 @@ public bool SwtIssuerEnabled`
`94`	`94`	`/// <summary>`
`95`	`95`	`/// Gets a string delimited by '\|' that contains the name of the apps with worker indexing disabled.`
`96`	`96`	`/// </summary>`
`97`		`- public string WorkerIndexingDisabledApps`
	`97`	`+ internal string WorkerIndexingDisabledApps`
`98`	`98`	`{`
`99`	`99`	`get`
`100`	`100`	`{`
`@@ -105,7 +105,7 @@ public string WorkerIndexingDisabledApps`
`105`	`105`	`/// <summary>`
`106`	`106`	`/// Gets a value indicating whether Linux Log Backoff is disabled in the hosting config.`
`107`	`107`	`/// </summary>`
`108`		`- public bool DisableLinuxAppServiceLogBackoff`
	`108`	`+ internal bool DisableLinuxAppServiceLogBackoff`
`109`	`109`	`{`
`110`	`110`	`get`
`111`	`111`	`{`
`@@ -116,7 +116,7 @@ public bool DisableLinuxAppServiceLogBackoff`
`116`	`116`	`/// <summary>`
`117`	`117`	`/// Gets or sets a value indicating whether Linux AppService/EP Detailed Execution Event is disabled in the hosting config.`
`118`	`118`	`/// </summary>`
`119`		`- public bool DisableLinuxAppServiceExecutionDetails`
	`119`	`+ internal bool DisableLinuxAppServiceExecutionDetails`
`120`	`120`	`{`
`121`	`121`	`get`
`122`	`122`	`{`
`@@ -129,11 +129,11 @@ public bool DisableLinuxAppServiceExecutionDetails`
`129`	`129`	`}`
`130`	`130`	`}`
`131`	`131`
`132`		`- public bool EnableOrderedInvocationMessages`
	`132`	`+ internal bool EnableOrderedInvocationMessages`
`133`	`133`	`{`
`134`	`134`	`get`
`135`	`135`	`{`
`136`		`- return GetFeature(ScriptConstants.FeatureFlagEnableOrderedInvocationmessages) == "1";`
	`136`	`+ return GetFeatureAsBooleanOrDefault(ScriptConstants.FeatureFlagEnableOrderedInvocationmessages, false);`
`137`	`137`	`}`
`138`	`138`
`139`	`139`	`set`
`@@ -145,7 +145,7 @@ public bool EnableOrderedInvocationMessages`
`145`	`145`	`/// <summary>`
`146`	`146`	`/// Gets the highest version of extension bundle v3 supported.`
`147`	`147`	`/// </summary>`
`148`		`- public string MaximumBundleV3Version`
	`148`	`+ internal string MaximumBundleV3Version`
`149`	`149`	`{`
`150`	`150`	`get`
`151`	`151`	`{`
`@@ -156,7 +156,7 @@ public string MaximumBundleV3Version`
`156`	`156`	`/// <summary>`
`157`	`157`	`/// Gets the highest version of extension bundle v4 supported.`
`158`	`158`	`/// </summary>`
`159`		`- public string MaximumBundleV4Version`
	`159`	`+ internal string MaximumBundleV4Version`
`160`	`160`	`{`
`161`	`161`	`get`
`162`	`162`	`{`
`@@ -167,15 +167,15 @@ public string MaximumBundleV4Version`
`167`	`167`	`/// <summary>`
`168`	`168`	`/// Gets a value indicating whether the host should revert the worker shutdown behavior in the WebHostWorkerChannelManager.`
`169`	`169`	`/// </summary>`
`170`		`- public bool RevertWorkerShutdownBehavior`
	`170`	`+ internal bool RevertWorkerShutdownBehavior`
`171`	`171`	`{`
`172`	`172`	`get`
`173`	`173`	`{`
`174`	`174`	`return GetFeature(RpcWorkerConstants.RevertWorkerShutdownBehavior) == "1";`
`175`	`175`	`}`
`176`	`176`	`}`
`177`	`177`
`178`		`- public bool ThrowOnMissingFunctionsWorkerRuntime`
	`178`	`+ internal bool ThrowOnMissingFunctionsWorkerRuntime`
`179`	`179`	`{`
`180`	`180`	`get`
`181`	`181`	`{`
`@@ -207,5 +207,35 @@ public string GetFeatureOrDefault(string name, string defaultValue)`
`207`	`207`	`{`
`208`	`208`	`return GetFeature(name) ?? defaultValue;`
`209`	`209`	`}`
	`210`	`+`
	`211`	`+ /// <summary>`
	`212`	`+ /// Gets a feature by name and attempts to parse it into a boolean.`
	`213`	`+ /// Returns "True, False" or ints as booleans. Returns True for any non-zero integer.`
	`214`	`+ /// If none of those (or empty), returns default.`
	`215`	`+ /// </summary>`
	`216`	`+ /// <param name="name">Feature name.</param>`
	`217`	`+ /// <param name="defaultValue">The default value to return if the feature value cannot be parsed into a boolean.</param>`
	`218`	`+ /// <returns>Boolean value if parse-able from hosting configuration. Otherwise, the defaultValue.</returns>`
	`219`	`+ internal bool GetFeatureAsBooleanOrDefault(string name, bool defaultValue)`
	`220`	`+ {`
	`221`	`+ string featureValue = GetFeature(name);`
	`222`	`+`
	`223`	`+ if (string.IsNullOrWhiteSpace(featureValue))`
	`224`	`+ {`
	`225`	`+ return defaultValue;`
	`226`	`+ }`
	`227`	`+`
	`228`	`+ if (bool.TryParse(featureValue, out bool parsedBool))`
	`229`	`+ {`
	`230`	`+ return parsedBool;`
	`231`	`+ }`
	`232`	`+`
	`233`	`+ if (int.TryParse(featureValue, out int parsedInt))`
	`234`	`+ {`
	`235`	`+ return parsedInt != 0;`
	`236`	`+ }`
	`237`	`+`
	`238`	`+ return defaultValue;`
	`239`	`+ }`
`210`	`240`	`}`
`211`		`-}`
	`241`	`+}`