From 05b4195ca8b1d22016b57be320975d8c5f03af9c Mon Sep 17 00:00:00 2001 From: Mike Stall Date: Wed, 12 Jul 2017 12:01:52 -0700 Subject: [PATCH] Allow direct Load webjobs dll. This is similar to precompiled, but is enabled if Function.json includes a "configurationSource" : "attributes". All functions within an assembly need the same setting. Resolves https://github.com/Azure/azure-webjobs-sdk-script/issues/1508 Add test, which runs locally but disabled since it fails in appveyor. --- .../Description/FunctionGenerator.cs | 5 + .../Description/FunctionMetadata.cs | 5 + src/WebJobs.Script/GlobalSuppressions.cs | 4 +- src/WebJobs.Script/Host/ScriptHost.cs | 145 +++++++++++++--- src/WebJobs.Script/Utility.cs | 9 + .../DirectLoadEndToEndTests.cs | 162 ++++++++++++++++++ .../TestFiles/CSharp-DirectLoad/function.json | 14 ++ .../TestFiles/CSharp-DirectLoad/run.cs | 31 ++++ .../WebJobs.Script.Tests.Integration.csproj | 3 + 9 files changed, 352 insertions(+), 26 deletions(-) create mode 100644 test/WebJobs.Script.Tests.Integration/DirectLoadEndToEndTests.cs create mode 100644 test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/function.json create mode 100644 test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/run.cs diff --git a/src/WebJobs.Script/Description/FunctionGenerator.cs b/src/WebJobs.Script/Description/FunctionGenerator.cs index 3a271d665e..92be424690 100644 --- a/src/WebJobs.Script/Description/FunctionGenerator.cs +++ b/src/WebJobs.Script/Description/FunctionGenerator.cs @@ -51,6 +51,11 @@ public static Type Generate(string functionAssemblyName, string typeName, Collec foreach (FunctionDescriptor function in functions) { + if (function.Metadata.IsDirect) + { + continue; + } + MethodBuilder methodBuilder = tb.DefineMethod(function.Name, MethodAttributes.Public | MethodAttributes.Static); Type[] types = function.Parameters.Select(p => p.Type).ToArray(); methodBuilder.SetParameters(types); diff --git a/src/WebJobs.Script/Description/FunctionMetadata.cs b/src/WebJobs.Script/Description/FunctionMetadata.cs index 5fbcd147e0..b2648ed131 100644 --- a/src/WebJobs.Script/Description/FunctionMetadata.cs +++ b/src/WebJobs.Script/Description/FunctionMetadata.cs @@ -54,6 +54,11 @@ public FunctionMetadata() /// public bool IsExcluded { get; set; } + /// + /// Gets or sets a value indicating whether that this function is a direct invoke. + /// + public bool IsDirect { get; set; } + public Collection Bindings { get; } public IEnumerable InputBindings diff --git a/src/WebJobs.Script/GlobalSuppressions.cs b/src/WebJobs.Script/GlobalSuppressions.cs index e040bdba50..6d70118b16 100644 --- a/src/WebJobs.Script/GlobalSuppressions.cs +++ b/src/WebJobs.Script/GlobalSuppressions.cs @@ -202,6 +202,8 @@ [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1709:IdentifiersShouldBeCasedCorrectly", MessageId = "Ms", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.Description.FunctionLogger.#LogFunctionResult(System.Boolean,System.String,System.Int64)")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.Binding.HttpBinding.#SetResponse(System.Net.Http.HttpRequestMessage,System.Object)")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.Description.FunctionLogger.#CreateUserTraceWriter(Microsoft.Azure.WebJobs.Host.TraceWriter)")] - [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1648:inheritdoc must be used with inheriting class", Justification = "", Scope = "member", Target = "~M:Microsoft.Azure.WebJobs.Script.Binding.MobileAppsScriptBindingProvider.#ctor(Microsoft.Azure.WebJobs.JobHostConfiguration,Newtonsoft.Json.Linq.JObject,Microsoft.Azure.WebJobs.Host.TraceWriter)")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1721:PropertyNamesShouldNotMatchGetMethods", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.Extensibility.ScriptBindingContext.#Type")] +[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Sku", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.ScriptConstants.#DynamicSkuConnectionLimit")] +[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods", MessageId = "System.Reflection.Assembly.LoadFrom", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.ScriptHost.#AddDirectTypes(System.Collections.Generic.List`1,System.Collections.ObjectModel.Collection`1)")] +[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods", MessageId = "System.Reflection.Assembly.LoadFrom", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.ScriptHost.#LoadCustomExtensions(System.String)")] diff --git a/src/WebJobs.Script/Host/ScriptHost.cs b/src/WebJobs.Script/Host/ScriptHost.cs index 5e46ad04d8..645e0cc7bf 100644 --- a/src/WebJobs.Script/Host/ScriptHost.cs +++ b/src/WebJobs.Script/Host/ScriptHost.cs @@ -160,6 +160,7 @@ public string InstanceId /// public virtual Collection Functions { get; private set; } + // Maps from FunctionName to a set of errors for that function. public virtual Dictionary> FunctionErrors { get; private set; } public virtual bool IsPrimary @@ -515,6 +516,8 @@ protected virtual void Initialize() List types = new List(); types.Add(type); + AddDirectTypes(types, functions); + hostConfig.TypeLocator = new TypeLocator(types); Functions = functions; @@ -563,6 +566,78 @@ private static IEnumerable DiscoverBindingTypes(IEnumerable functions) + { + HashSet illegalScriptAssemblies = new HashSet(); + + Dictionary mapAssemblySettings = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var function in functions) + { + var metadata = function.Metadata; + var scriptFile = metadata.ScriptFile; + if (scriptFile.EndsWith(".dll", StringComparison.OrdinalIgnoreCase)) + { + bool isDirect = metadata.IsDirect; + bool prevIsDirect; + if (mapAssemblySettings.TryGetValue(scriptFile, out prevIsDirect)) + { + if (prevIsDirect != isDirect) + { + illegalScriptAssemblies.Add(scriptFile); + } + } + mapAssemblySettings[scriptFile] = isDirect; + } + } + + foreach (var function in functions) + { + var metadata = function.Metadata; + var scriptFile = metadata.ScriptFile; + + if (illegalScriptAssemblies.Contains(scriptFile)) + { + // Error. All entries pointing to the same dll must have the same value for IsDirect + string msg = string.Format(CultureInfo.InvariantCulture, "Configuration error: All functions in {0} must have the same config precedence,", + scriptFile); + + // Adding a function error will cause this function to get ignored + AddFunctionError(this.FunctionErrors, metadata.Name, msg); + + TraceWriter.Info(msg); + _startupLogger?.LogInformation(msg); + } + + return; + } + } + + private static void AddDirectTypes(List types, Collection functions) + { + HashSet visitedTypes = new HashSet(); + + foreach (var function in functions) + { + var metadata = function.Metadata; + if (!metadata.IsDirect) + { + continue; + } + + string path = metadata.ScriptFile; + var typeName = Utility.GetFullClassName(metadata.EntryPoint); + + Assembly assembly = Assembly.LoadFrom(path); + var type = assembly.GetType(typeName); + + if (visitedTypes.Add(type)) + { + types.Add(type); + } + } + } + private IMetricsLogger CreateMetricsLogger() { IMetricsLogger metricsLogger = ScriptConfig.HostConfig.GetService(); @@ -581,30 +656,7 @@ private void LoadCustomExtensions() { foreach (var dir in Directory.EnumerateDirectories(extensionsPath)) { - foreach (var path in Directory.EnumerateFiles(dir, "*.dll")) - { - // We don't want to load and reflect over every dll. - // By convention, restrict to based on filenames. - var filename = Path.GetFileName(path); - if (!filename.ToLowerInvariant().Contains("extension")) - { - continue; - } - - try - { - // See GetNugetPackagesPath() for details - // Script runtime is already setup with assembly resolution hooks, so use LoadFrom - Assembly assembly = Assembly.LoadFrom(path); - LoadExtensions(assembly, path); - } - catch (Exception e) - { - string msg = $"Failed to load custom extension from '{path}'."; - TraceWriter.Error(msg, e); - _startupLogger.LogError(0, e, msg); - } - } + LoadCustomExtensions(dir); } } @@ -616,6 +668,34 @@ private void LoadCustomExtensions() generalProvider.CompleteInitialization(metadataProvider); } + private void LoadCustomExtensions(string extensionsPath) + { + foreach (var path in Directory.EnumerateFiles(extensionsPath, "*.dll")) + { + // We don't want to load and reflect over every dll. + // By convention, restrict to based on filenames. + var filename = Path.GetFileName(path); + if (!filename.ToLowerInvariant().Contains("extension")) + { + continue; + } + + try + { + // See GetNugetPackagesPath() for details + // Script runtime is already setup with assembly resolution hooks, so use LoadFrom + Assembly assembly = Assembly.LoadFrom(path); + LoadExtensions(assembly, path); + } + catch (Exception e) + { + string msg = $"Failed to load custom extension from '{path}'."; + TraceWriter.Error(msg, e); + _startupLogger.LogError(0, e, msg); + } + } + } + private void LoadExtensions(Assembly assembly, string locationHint) { foreach (var type in assembly.ExportedTypes) @@ -930,6 +1010,20 @@ private static FunctionMetadata ParseFunctionMetadata(string functionName, JObje functionMetadata.IsExcluded = (bool)value; } + JToken isDirect; + if (configMetadata.TryGetValue("configurationSource", StringComparison.OrdinalIgnoreCase, out isDirect)) + { + var isDirectValue = isDirect.ToString(); + if (string.Equals(isDirectValue, "attributes", StringComparison.OrdinalIgnoreCase)) + { + functionMetadata.IsDirect = true; + } + else if (!string.Equals(isDirectValue, "config", StringComparison.OrdinalIgnoreCase)) + { + throw new FormatException($"Illegal value '{isDirectValue}' for 'configurationSource' property in {functionMetadata.Name}'."); + } + } + return functionMetadata; } @@ -1032,7 +1126,6 @@ internal static bool TryParseFunctionMetadata(string functionName, JObject funct // determine the script type based on the primary script file extension functionMetadata.ScriptType = ParseScriptType(functionMetadata.ScriptFile); - functionMetadata.EntryPoint = (string)functionConfig["entryPoint"]; return true; @@ -1207,6 +1300,8 @@ internal Collection GetFunctionDescriptors(IEnumerable ToStringValues(this IReadOnlyD return data.ToDictionary(p => p.Key, p => p.Value != null ? p.Value.ToString() : null, StringComparer.OrdinalIgnoreCase); } + // "Namespace.Class.Method" --> "Method" public static string GetFunctionShortName(string functionName) { int idx = functionName.LastIndexOf('.'); @@ -144,6 +145,14 @@ public static string GetFunctionShortName(string functionName) return functionName; } + // "Namespace.Class.Method" --> "Namespace.Class" + public static string GetFullClassName(string fullFunctionName) + { + int i = fullFunctionName.LastIndexOf('.'); + var typeName = fullFunctionName.Substring(0, i); + return typeName; + } + internal static string GetDefaultHostId(ScriptSettingsManager settingsManager, ScriptHostConfiguration scriptConfig) { // We're setting the default here on the newly created configuration diff --git a/test/WebJobs.Script.Tests.Integration/DirectLoadEndToEndTests.cs b/test/WebJobs.Script.Tests.Integration/DirectLoadEndToEndTests.cs new file mode 100644 index 0000000000..cbc2df1f2d --- /dev/null +++ b/test/WebJobs.Script.Tests.Integration/DirectLoadEndToEndTests.cs @@ -0,0 +1,162 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Net.Http.Headers; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Azure.WebJobs.Host; +using Microsoft.Azure.WebJobs.Script.Tests.Properties; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Script.Tests +{ + public class DirectLoadEndToEndTests : EndToEndTestsBase + { + public DirectLoadEndToEndTests(TestFixture fixture) : base(fixture) + { + } + + [Fact(Skip ="Fails in AppVeyor")] + public async Task Invoke() + { + // Verify the type is ls in the typelocator. + JobHostConfiguration config = this.Fixture.Host.ScriptConfig.HostConfig; + var tl = config.TypeLocator; + var userType = tl.GetTypes().Where(type => type.FullName == "TestFunction.DirectLoadFunction").First(); + AssertUserType(userType); + + await InvokeDotNetFunction("DotNetDirectFunction", "Hello from .NET DirectInvoker"); + } + + public async Task InvokeDotNetFunction(string functionName, string expectedResult) + { + var request = new HttpRequestMessage(HttpMethod.Get, "http://functions/myfunc"); + Dictionary arguments = new Dictionary() + { + { "req", request } + }; + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/plain")); + request.SetConfiguration(Fixture.RequestConfiguration); + + await Fixture.Host.CallAsync(functionName, arguments); + + HttpResponseMessage response = (HttpResponseMessage)request.Properties[ScriptConstants.AzureFunctionsHttpResponseKey]; + + Assert.Equal(expectedResult, await response.Content.ReadAsStringAsync()); + } + + // Do validation on the type we compiled. + // Verify that it loads and binds against the propery runtime types. + private static void AssertUserType(Type type) + { + var method = type.GetMethod("Run"); + var functionNameAttr = method.GetCustomAttribute(); + Assert.NotNull(functionNameAttr); + Assert.Equal("DotNetDirectFunction", functionNameAttr.Name); + + var parameters = method.GetParameters(); + var p1 = parameters[0]; + Assert.Equal(typeof(HttpRequestMessage), p1.ParameterType); + var parameterAttr = p1.GetCustomAttribute(); + Assert.NotNull(parameterAttr); + } + + public class TestFixture : EndToEndTestFixture + { + private static readonly string ScriptRoot = @"TestScripts\DotNetDirect\" + Guid.NewGuid().ToString(); + private static readonly string Function1Path; + + static TestFixture() + { + Function1Path = Path.Combine(ScriptRoot, "DotNetDirectFunction"); + CreateFunctionAssembly(); + } + + public TestFixture() : base(ScriptRoot, "dotnet") + { + } + + public override void Dispose() + { + base.Dispose(); + + FileUtility.DeleteDirectoryAsync(ScriptRoot, true).Wait(); + } + + // Create the artifacts in the ScriptRoot folder. + private static void CreateFunctionAssembly() + { + Directory.CreateDirectory(Function1Path); + + uint rand = (uint)Guid.NewGuid().GetHashCode(); + string assemblyName = "Asm" + rand; + string assemblyNamePath = assemblyName + ".dll"; + + var source = GetResource("run.cs"); + + var syntaxTree = CSharpSyntaxTree.ParseText(source); + Compilation compilation = CSharpCompilation.Create(assemblyName, new[] { syntaxTree }) + .WithOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) + .WithReferences( + MetadataReference.CreateFromFile(typeof(TraceWriter).Assembly.Location), + MetadataReference.CreateFromFile(typeof(FunctionNameAttribute).Assembly.Location), + MetadataReference.CreateFromFile(typeof(HttpRequestMessage).Assembly.Location), + MetadataReference.CreateFromFile(typeof(HttpTriggerAttribute).Assembly.Location), + MetadataReference.CreateFromFile(typeof(HttpStatusCode).Assembly.Location), + MetadataReference.CreateFromFile(typeof(Enumerable).Assembly.Location), + MetadataReference.CreateFromFile(typeof(object).Assembly.Location)); + + var assemblyFullPath = Path.Combine(Function1Path, assemblyNamePath); + var result = compilation.Emit(assemblyFullPath); + Assert.True(result.Success); + + var hostJson = @" +{ + 'id': 'function-tests-dotnet-direct' +}"; + File.WriteAllText(Path.Combine(ScriptRoot, "host.json"), hostJson); + + CreateFunctionMetadata(Function1Path, assemblyNamePath); + + // Verify that assembly loads and binds against hte same runtime types. + { + Assembly assembly = Assembly.LoadFrom(assemblyFullPath); + var type = assembly.GetType("TestFunction.DirectLoadFunction"); + AssertUserType(type); + } + } + + private static void CreateFunctionMetadata( + string path, + string scriptFilePath, + string entrypoint = "TestFunction.DirectLoadFunction.Run") + { + var content = GetResource("function.json"); + content = string.Format(content, scriptFilePath, entrypoint); + + File.WriteAllText(Path.Combine(path, "function.json"), content); + } + + private static string GetResource(string name) + { + var resourceNamespace = "Microsoft.Azure.WebJobs.Script.Tests.TestFiles.CSharp_DirectLoad."; + var fullName = resourceNamespace + name; + using (var stream = Assembly.GetExecutingAssembly().GetManifestResourceStream(fullName)) + using (var reader = new StreamReader(stream)) + { + return reader.ReadToEnd(); + } + } + } + } +} diff --git a/test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/function.json b/test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/function.json new file mode 100644 index 0000000000..02e42f927f --- /dev/null +++ b/test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/function.json @@ -0,0 +1,14 @@ +{{ + "Comments" : "This is a for a direct load scenario. Notice it only has a trigger binding and has the 'configurationSource' set.", + "scriptFile":"{0}", + "entryPoint": "{1}", + "configurationSource" : "attributes", + "bindings": [ + {{ + "type": "httpTrigger", + "name": "req", + "direction": "in", + "methods": [ "get" ] + }} + ] +}} \ No newline at end of file diff --git a/test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/run.cs b/test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/run.cs new file mode 100644 index 0000000000..ebe995fac0 --- /dev/null +++ b/test/WebJobs.Script.Tests.Integration/TestFiles/CSharp-DirectLoad/run.cs @@ -0,0 +1,31 @@ +// This is content for a test file! +// Not actually part of the test build. + +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.Azure.WebJobs; +using Microsoft.Azure.WebJobs.Host; +using Microsoft.Azure.WebJobs.Extensions.Http; + +namespace TestFunction +{ + // Test Functions directly invoking WebJobs. + public class DirectLoadFunction + { + [FunctionName("DotNetDirectFunction")] + public static Task Run( + [HttpTrigger(AuthorizationLevel.Anonymous, "get", "post")] HttpRequestMessage req, + TraceWriter log) + { + log.Info("Test"); + + var res = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("Hello from .NET DirectInvoker") + }; + + return Task.FromResult(res); + } + } +} diff --git a/test/WebJobs.Script.Tests.Integration/WebJobs.Script.Tests.Integration.csproj b/test/WebJobs.Script.Tests.Integration/WebJobs.Script.Tests.Integration.csproj index 7b9cc7b36f..bf83d224d9 100644 --- a/test/WebJobs.Script.Tests.Integration/WebJobs.Script.Tests.Integration.csproj +++ b/test/WebJobs.Script.Tests.Integration/WebJobs.Script.Tests.Integration.csproj @@ -391,6 +391,7 @@ + @@ -444,6 +445,8 @@ + + Always