From 46b66100ba8cb2aa83645e2c23476d582bbcca57 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Mon, 21 Oct 2024 11:18:37 -0700 Subject: [PATCH 1/4] Address language options null by consuming from parent services --- .../ScriptStartupTypeLocator.cs | 7 ++- .../Host/FunctionMetadataManager.cs | 51 ++++++++++--------- .../Host/IFunctionMetadataManager.cs | 5 +- src/WebJobs.Script/Host/ScriptHost.cs | 8 ++- .../TestFunctionHost.cs | 7 ++- .../TestFunctionMetadataManager.cs | 3 +- .../Description/FunctionInvokerBaseTests.cs | 4 +- .../ScriptStartupTypeDiscovererTests.cs | 16 +++--- 8 files changed, 50 insertions(+), 51 deletions(-) diff --git a/src/WebJobs.Script/DependencyInjection/ScriptStartupTypeLocator.cs b/src/WebJobs.Script/DependencyInjection/ScriptStartupTypeLocator.cs index 443e2ba807..3ec99cbb8b 100644 --- a/src/WebJobs.Script/DependencyInjection/ScriptStartupTypeLocator.cs +++ b/src/WebJobs.Script/DependencyInjection/ScriptStartupTypeLocator.cs @@ -84,7 +84,6 @@ public async Task> GetExtensionsStartupTypesAsync() bool isPrecompiledFunctionApp = false; // dotnet app precompiled -> Do not use bundles - var workerConfigs = _languageWorkerOptions.CurrentValue.WorkerConfigs; ExtensionRequirementsInfo extensionRequirements = GetExtensionRequirementsInfo(); ImmutableArray functionMetadataCollection = ImmutableArray.Empty; if (bundleConfigured) @@ -92,10 +91,10 @@ public async Task> GetExtensionsStartupTypesAsync() ExtensionBundleDetails bundleDetails = await _extensionBundleManager.GetExtensionBundleDetails(); ValidateBundleRequirements(bundleDetails, extensionRequirements); - functionMetadataCollection = _functionMetadataManager.GetFunctionMetadata(forceRefresh: true, includeCustomProviders: false, workerConfigs: workerConfigs); + functionMetadataCollection = _functionMetadataManager.GetFunctionMetadata(forceRefresh: true, includeCustomProviders: false); bindingsSet = new HashSet(StringComparer.OrdinalIgnoreCase); - // Generate a Hashset of all the binding types used in the function app + // Generate a HashSet of all the binding types used in the function app foreach (var functionMetadata in functionMetadataCollection) { foreach (var binding in functionMetadata.Bindings) @@ -113,7 +112,7 @@ public async Task> GetExtensionsStartupTypesAsync() if (SystemEnvironment.Instance.IsPlaceholderModeEnabled()) { // Do not move this. - // Calling this log statement in the placeholder mode to avoid jitting during specializtion + // Calling this log statement in the placeholder mode to avoid jitting during specialization _logger.ScriptStartNotLoadingExtensionBundle("WARMUP_LOG_ONLY", bundleConfigured, isPrecompiledFunctionApp, isLegacyExtensionBundle, isDotnetIsolatedApp, isLogicApp); } diff --git a/src/WebJobs.Script/Host/FunctionMetadataManager.cs b/src/WebJobs.Script/Host/FunctionMetadataManager.cs index 7b6f55678e..a2fcb78548 100644 --- a/src/WebJobs.Script/Host/FunctionMetadataManager.cs +++ b/src/WebJobs.Script/Host/FunctionMetadataManager.cs @@ -6,12 +6,10 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; -using System.Threading; using System.Threading.Tasks; using Microsoft.Azure.WebJobs.Logging; using Microsoft.Azure.WebJobs.Script.Description; using Microsoft.Azure.WebJobs.Script.Diagnostics.Extensions; -using Microsoft.Azure.WebJobs.Script.Workers; using Microsoft.Azure.WebJobs.Script.Workers.Http; using Microsoft.Azure.WebJobs.Script.Workers.Rpc; using Microsoft.Extensions.DependencyInjection; @@ -20,24 +18,31 @@ namespace Microsoft.Azure.WebJobs.Script { - public class FunctionMetadataManager : IFunctionMetadataManager + public sealed class FunctionMetadataManager : IFunctionMetadataManager, IDisposable { private const string FunctionConfigurationErrorMessage = "Unable to determine the primary function script. Make sure at least one script file is present. Try renaming your entry point script to 'run' or alternatively you can specify the name of the entry point script explicitly by adding a 'scriptFile' property to your function metadata."; private const string MetadataProviderName = "Custom"; private readonly IServiceProvider _serviceProvider; - private IFunctionMetadataProvider _functionMetadataProvider; + private readonly IFunctionMetadataProvider _functionMetadataProvider; + private readonly IEnvironment _environment; + private readonly IDisposable _onChangeSubscription; + private readonly IOptionsMonitor _languageOptions; + private IOptions _scriptOptions; + private ILogger _logger; private bool _isHttpWorker; - private IEnvironment _environment; private bool _servicesReset = false; - private ILogger _logger; - private IOptions _scriptOptions; private ImmutableArray _functionMetadataArray; private Dictionary> _functionErrors = new Dictionary>(); private ConcurrentDictionary _functionMetadataMap = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - public FunctionMetadataManager(IOptions scriptOptions, IFunctionMetadataProvider functionMetadataProvider, - IOptions httpWorkerOptions, IScriptHostManager scriptHostManager, ILoggerFactory loggerFactory, - IEnvironment environment) + public FunctionMetadataManager( + IOptions scriptOptions, + IFunctionMetadataProvider functionMetadataProvider, + IOptions httpWorkerOptions, + IScriptHostManager scriptHostManager, + ILoggerFactory loggerFactory, + IEnvironment environment, + IOptionsMonitor languageOptions) { _scriptOptions = scriptOptions; _serviceProvider = scriptHostManager as IServiceProvider; @@ -46,6 +51,9 @@ public FunctionMetadataManager(IOptions scriptOptions, IFu _isHttpWorker = httpWorkerOptions?.Value?.Description != null; _environment = environment; + _languageOptions = languageOptions; + _onChangeSubscription = _languageOptions.OnChange(_ => _servicesReset = true); + // Every time script host is re-initializing, we also need to re-initialize // services that change with the scope of the script host. scriptHostManager.ActiveHostChanged += (s, e) => @@ -57,8 +65,10 @@ public FunctionMetadataManager(IOptions scriptOptions, IFu }; } + /// public ImmutableDictionary> Errors { get; private set; } + /// public bool TryGetFunctionMetadata(string functionName, out FunctionMetadata functionMetadata, bool forceRefresh) { if (forceRefresh) @@ -82,11 +92,11 @@ public bool TryGetFunctionMetadata(string functionName, out FunctionMetadata fun /// Apply functions allow list filter. /// Include any metadata provided by IFunctionProvider when loading the metadata. /// An Immutable array of FunctionMetadata. - public ImmutableArray GetFunctionMetadata(bool forceRefresh, bool applyAllowList = true, bool includeCustomProviders = true, IList workerConfigs = null) + public ImmutableArray GetFunctionMetadata(bool forceRefresh, bool applyAllowList = true, bool includeCustomProviders = true) { if (forceRefresh || _servicesReset || _functionMetadataArray.IsDefaultOrEmpty) { - _functionMetadataArray = LoadFunctionMetadata(forceRefresh, includeCustomProviders, workerConfigs: workerConfigs); + _functionMetadataArray = LoadFunctionMetadata(forceRefresh, includeCustomProviders); _logger.FunctionMetadataManagerFunctionsLoaded(ApplyAllowList(_functionMetadataArray).Count()); _servicesReset = false; } @@ -94,6 +104,9 @@ public ImmutableArray GetFunctionMetadata(bool forceRefresh, b return applyAllowList ? ApplyAllowList(_functionMetadataArray) : _functionMetadataArray; } + /// + public void Dispose() => _onChangeSubscription.Dispose(); + private ImmutableArray ApplyAllowList(ImmutableArray metadataList) { var allowList = _scriptOptions.Value?.Functions; @@ -119,22 +132,12 @@ private void InitializeServices() _servicesReset = true; } - /// - /// This is the worker configuration created in the jobhost scope during placeholder initialization - /// This is used as a fallback incase the config is not passed down from previous method call. - /// - private IList GetFallbackWorkerConfig() - { - return _serviceProvider.GetService>().CurrentValue.WorkerConfigs; - } - /// /// Read all functions and populate function metadata. /// - internal ImmutableArray LoadFunctionMetadata(bool forceRefresh = false, bool includeCustomProviders = true, IFunctionInvocationDispatcher dispatcher = null, IList workerConfigs = null) + internal ImmutableArray LoadFunctionMetadata(bool forceRefresh = false, bool includeCustomProviders = true) { - workerConfigs ??= GetFallbackWorkerConfig(); - + var workerConfigs = _languageOptions.CurrentValue.WorkerConfigs; _functionMetadataMap.Clear(); ICollection functionsAllowList = _scriptOptions?.Value?.Functions; diff --git a/src/WebJobs.Script/Host/IFunctionMetadataManager.cs b/src/WebJobs.Script/Host/IFunctionMetadataManager.cs index bf42513863..94c4b273cc 100644 --- a/src/WebJobs.Script/Host/IFunctionMetadataManager.cs +++ b/src/WebJobs.Script/Host/IFunctionMetadataManager.cs @@ -1,11 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for license information. -using System.Collections.Generic; using System.Collections.Immutable; using Microsoft.Azure.WebJobs.Script.Description; -using Microsoft.Azure.WebJobs.Script.Workers; -using Microsoft.Azure.WebJobs.Script.Workers.Rpc; namespace Microsoft.Azure.WebJobs.Script { @@ -13,7 +10,7 @@ public interface IFunctionMetadataManager { ImmutableDictionary> Errors { get; } - ImmutableArray GetFunctionMetadata(bool forceRefresh = false, bool applyAllowlist = true, bool includeCustomProviders = true, IList workerConfigs = null); + ImmutableArray GetFunctionMetadata(bool forceRefresh = false, bool applyAllowlist = true, bool includeCustomProviders = true); bool TryGetFunctionMetadata(string functionName, out FunctionMetadata functionMetadata, bool forceRefresh = false); } diff --git a/src/WebJobs.Script/Host/ScriptHost.cs b/src/WebJobs.Script/Host/ScriptHost.cs index ed82d21ce9..abc76bec4c 100644 --- a/src/WebJobs.Script/Host/ScriptHost.cs +++ b/src/WebJobs.Script/Host/ScriptHost.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Collections.ObjectModel; using System.Diagnostics; using System.Globalization; @@ -373,13 +374,10 @@ private void LogHostFunctionErrors() /// A metadata collection of functions and proxies configured. private IEnumerable GetFunctionsMetadata() { - IEnumerable functionMetadata; - - functionMetadata = _functionMetadataManager.GetFunctionMetadata(forceRefresh: false, workerConfigs: _languageWorkerOptions.CurrentValue.WorkerConfigs); - + ImmutableArray functionMetadata = _functionMetadataManager.GetFunctionMetadata(forceRefresh: false); foreach (var error in _functionMetadataManager.Errors) { - FunctionErrors.Add(error.Key, error.Value.ToArray()); + FunctionErrors.Add(error.Key, error.Value); } return functionMetadata; diff --git a/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs b/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs index 4527267f81..f9c68342ea 100644 --- a/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs +++ b/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs @@ -35,6 +35,7 @@ using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Microsoft.WebJobs.Script.Tests; +using Moq; using Newtonsoft.Json.Linq; using IApplicationLifetime = Microsoft.AspNetCore.Hosting.IApplicationLifetime; @@ -480,12 +481,16 @@ private FunctionMetadataManager GetMetadataManager(IOptionsMonitor>(); + mockOptions.Setup(o => o.CurrentValue).Returns(workerOptions); + mockOptions.Setup(o => o.OnChange(It.IsAny>())).Returns(Mock.Of()); + var managerServiceProvider = manager as IServiceProvider; var metadataProvider = new HostFunctionMetadataProvider(optionsMonitor, NullLogger.Instance, new TestMetricsLogger(), SystemEnvironment.Instance); var defaultProvider = new FunctionMetadataProvider(NullLogger.Instance, null, metadataProvider, new OptionsWrapper(new FunctionsHostingConfigOptions()), SystemEnvironment.Instance); var metadataManager = new FunctionMetadataManager(managerServiceProvider.GetService>(), defaultProvider, - managerServiceProvider.GetService>(), manager, factory, environment); + managerServiceProvider.GetService>(), manager, factory, environment, mockOptions.Object); return metadataManager; } diff --git a/test/WebJobs.Script.Tests.Shared/TestFunctionMetadataManager.cs b/test/WebJobs.Script.Tests.Shared/TestFunctionMetadataManager.cs index 28a2767e0c..cd16c99ac5 100644 --- a/test/WebJobs.Script.Tests.Shared/TestFunctionMetadataManager.cs +++ b/test/WebJobs.Script.Tests.Shared/TestFunctionMetadataManager.cs @@ -39,7 +39,6 @@ public static FunctionMetadataManager GetFunctionMetadataManager(IOptions