Fixing issues with function target invalidation on return type changes

fabiocav · fabiocav · commit 8c4c2d632409 · 2016-11-07T19:33:05.000-08:00
diff --git a/src/WebJobs.Script/Description/DotNet/CSharp/CSharpCompilation.cs b/src/WebJobs.Script/Description/DotNet/CSharp/CSharpCompilation.cs
@@ -51,10 +51,17 @@ public FunctionSignature GetEntryPointSignature(IFunctionEntryPointResolver entr
                 .Select(m => new MethodReference<IMethodSymbol>(m.Name, m.DeclaredAccessibility == Accessibility.Public, m));
 
             IMethodSymbol entryPointReference = entryPointResolver.GetFunctionEntryPoint(methods).Value;
-            bool hasLocalTypeReferences = entryPointReference.Parameters.Any(p => IsOrUsesAssemblyType(p.Type, entryPointReference.ContainingAssembly));
+            bool hasLocalTypeReferences = HasLocalTypeReferences(entryPointReference);
             var functionParameters = entryPointReference.Parameters.Select(p => new FunctionParameter(p.Name, GetFullTypeName(p.Type), p.IsOptional, p.RefKind));
 
-            return new FunctionSignature(entryPointReference.ContainingType.Name, entryPointReference.Name, ImmutableArray.CreateRange(functionParameters.ToArray()), hasLocalTypeReferences);
+            return new FunctionSignature(entryPointReference.ContainingType.Name, entryPointReference.Name,
+                ImmutableArray.CreateRange(functionParameters.ToArray()), GetFullTypeName(entryPointReference.ReturnType), hasLocalTypeReferences);
+        }
+
+        private static bool HasLocalTypeReferences(IMethodSymbol entryPointReference)
+        {
+            return IsOrUsesAssemblyType(entryPointReference.ReturnType, entryPointReference.ContainingAssembly)
+                || entryPointReference.Parameters.Any(p => IsOrUsesAssemblyType(p.Type, entryPointReference.ContainingAssembly));
         }
 
         private static string GetFullTypeName(ITypeSymbol type)
diff --git a/src/WebJobs.Script/Description/DotNet/FSharp/FSharpCompilation.cs b/src/WebJobs.Script/Description/DotNet/FSharp/FSharpCompilation.cs
@@ -64,8 +64,9 @@ public FunctionSignature GetEntryPointSignature(IFunctionEntryPointResolver entr
             // For F#, we always set this to true for now.
             bool hasLocalTypeReference = true;
 
-            var signature = new FunctionSignature(entryPointReference.DeclaringType.Name, entryPointReference.Name, parameters.ToImmutableArray(), hasLocalTypeReference);
-            
+            var signature = new FunctionSignature(entryPointReference.DeclaringType.Name, entryPointReference.Name,
+                parameters.ToImmutableArray(), entryPointReference.ReturnType.Name, hasLocalTypeReference);
+
             return signature;
         }
 
diff --git a/src/WebJobs.Script/Description/DotNet/FunctionSignature.cs b/src/WebJobs.Script/Description/DotNet/FunctionSignature.cs
@@ -2,13 +2,9 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Globalization;
 using System.Linq;
-using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
 
 namespace Microsoft.Azure.WebJobs.Script.Description
 {
@@ -22,50 +18,30 @@ public sealed class FunctionSignature : IEquatable<FunctionSignature>
         private readonly bool _hasLocalTypeReference;
         private readonly string _parentTypeName;
         private readonly string _methodName;
+        private readonly string _returnTypeName;
 
-        public FunctionSignature(string parentTypeName, string methodName, ImmutableArray<FunctionParameter> parameters, bool hasLocalTypeReference)
+        public FunctionSignature(string parentTypeName, string methodName, ImmutableArray<FunctionParameter> parameters, string returnTypeName, bool hasLocalTypeReference)
         {
             _parameters = parameters;
             _hasLocalTypeReference = hasLocalTypeReference;
             _parentTypeName = parentTypeName;
+            _returnTypeName = returnTypeName;
             _methodName = methodName;
         }
 
         /// <summary>
         /// Returns true if the function uses locally defined types (i.e. types defined in the function assembly) in its parameters;
         /// otherwise, false.
         /// </summary>
-        public bool HasLocalTypeReference
-        {
-            get
-            {
-                return _hasLocalTypeReference;
-            }
-        }
+        public bool HasLocalTypeReference => _hasLocalTypeReference;
 
-        public ImmutableArray<FunctionParameter> Parameters
-        {
-            get
-            {
-                return _parameters;
-            }
-        }
+        public ImmutableArray<FunctionParameter> Parameters => _parameters;
 
-        public string ParentTypeName
-        {
-            get
-            {
-                return _parentTypeName;
-            }
-        }
+        public string ParentTypeName => _parentTypeName;
 
-        public string MethodName
-        {
-            get
-            {
-                return _methodName;
-            }
-        }
+        public string MethodName => _methodName;
+
+        public string ReturnTypeName => _returnTypeName;
 
         public bool Equals(FunctionSignature other)
         {
@@ -74,7 +50,11 @@ public bool Equals(FunctionSignature other)
                 return false;
             }
 
-            if (_parameters.Count() != other._parameters.Count())
+            if (!string.Equals(ParentTypeName, other.ParentTypeName) ||
+                !string.Equals(MethodName, other.MethodName) ||
+                !string.Equals(ReturnTypeName, other.ReturnTypeName) ||
+                HasLocalTypeReference != other.HasLocalTypeReference ||
+                _parameters.Count() != other._parameters.Count())
             {
                 return false;
             }
@@ -84,6 +64,12 @@ public bool Equals(FunctionSignature other)
 
         public override bool Equals(object obj) => Equals(obj as FunctionSignature);
 
-        public override int GetHashCode() => _parameters.Aggregate(0, (a, p) => a ^ p.GetHashCode());
+        public override int GetHashCode() => GetPropertyHashCode(ParentTypeName) ^
+            GetPropertyHashCode(MethodName) ^
+            GetPropertyHashCode(ReturnTypeName) ^
+            HasLocalTypeReference.GetHashCode() ^
+            _parameters.Aggregate(0, (a, p) => a ^ p.GetHashCode());
+
+        private static int GetPropertyHashCode(string value) => value?.GetHashCode() ?? 0;
     }
 }
diff --git a/test/WebJobs.Script.Tests/CSharpFunctionSignatureTests.cs b/test/WebJobs.Script.Tests/CSharpFunctionSignatureTests.cs
@@ -1,6 +1,7 @@
 ﻿// 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.Linq;
 using Microsoft.Azure.WebJobs.Script.Description;
 using Microsoft.CodeAnalysis;
@@ -14,7 +15,7 @@ public class CSharpFunctionSignatureTests
         [Fact]
         public void Matches_IsTrue_WhenParametersAreEqual()
         {
-          var function1 = @"using System;
+            var function1 = @"using System;
 public static void Run(string id, out string output)
 {
     output = string.Empty;
@@ -27,22 +28,33 @@ public static void Run(string id, out string output)
     output = result;
 }";
 
-            var tree1 = CSharpSyntaxTree.ParseText(function1, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
-            var tree2 = CSharpSyntaxTree.ParseText(function2, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
+            Tuple<FunctionSignature, FunctionSignature> signatures = GetFunctionSignatures(function1, function2);
 
-            var references = new MetadataReference[] { MetadataReference.CreateFromFile(typeof(string).Assembly.Location) };
+            Assert.True(signatures.Item1.Equals(signatures.Item2));
+            Assert.Equal(signatures.Item1.GetHashCode(), signatures.Item2.GetHashCode());
+        }
 
-            var compilation1 = new Script.Description.CSharpCompilation(CodeAnalysis.CSharp.CSharpCompilation.Create("test1", references: references)
-                   .AddSyntaxTrees(tree1));
+        [Fact]
+        public void Matches_IsFalse_WhenReturnTypesAreDifferent()
+        {
+            var function1 = @"using System;
+public static string Run(string id, out string output)
+{
+    output = string.Empty;
+    return string.Empty;
+}";
 
-            var compilation2 = new Script.Description.CSharpCompilation(CodeAnalysis.CSharp.CSharpCompilation.Create("test2", references: references)
-                .AddSyntaxTrees(tree2));
+            var function2 = @"using System;
+public static void Run(string id, out string output)
+{
+    string result = string.Empty;
+    output = result;
+}";
 
-            var signature1 = compilation1.GetEntryPointSignature(new FunctionEntryPointResolver());
-            var signature2 = compilation2.GetEntryPointSignature(new FunctionEntryPointResolver());
+            Tuple<FunctionSignature, FunctionSignature> signatures = GetFunctionSignatures(function1, function2);
 
-            Assert.True(signature1.Equals(signature2));
-            Assert.Equal(signature1.GetHashCode(), signature2.GetHashCode());
+            Assert.False(signatures.Item1.Equals(signatures.Item2));
+            Assert.NotEqual(signatures.Item1.GetHashCode(), signatures.Item2.GetHashCode());
         }
 
         [Fact]
@@ -61,6 +73,14 @@ public static void Run(string id, out string output)
     output = result;
 }";
 
+            Tuple<FunctionSignature, FunctionSignature> signatures = GetFunctionSignatures(function1, function2);
+
+            Assert.False(signatures.Item1.Equals(signatures.Item2));
+            Assert.NotEqual(signatures.Item1.GetHashCode(), signatures.Item2.GetHashCode());
+        }
+
+        private Tuple<FunctionSignature, FunctionSignature> GetFunctionSignatures(string function1, string function2)
+        {
             var tree1 = CSharpSyntaxTree.ParseText(function1, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
             var tree2 = CSharpSyntaxTree.ParseText(function2, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
 
@@ -75,8 +95,7 @@ public static void Run(string id, out string output)
             var signature1 = compilation1.GetEntryPointSignature(new FunctionEntryPointResolver());
             var signature2 = compilation2.GetEntryPointSignature(new FunctionEntryPointResolver());
 
-            Assert.False(signature1.Equals(signature2));
-            Assert.NotEqual(signature1.GetHashCode(), signature2.GetHashCode());
+            return Tuple.Create(signature1, signature2);
         }
 
         [Fact]
@@ -97,22 +116,10 @@ public static void Run( System.String id ,
     output = result;
 }";
 
-            var tree1 = CSharpSyntaxTree.ParseText(function1, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
-            var tree2 = CSharpSyntaxTree.ParseText(function2, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
-
-            var references = new MetadataReference[] { MetadataReference.CreateFromFile(typeof(string).Assembly.Location) };
-
-            var compilation1 = new Script.Description.CSharpCompilation(CodeAnalysis.CSharp.CSharpCompilation.Create("test1", references: references)
-                .AddSyntaxTrees(tree1));
-
-            var compilation2 = new Script.Description.CSharpCompilation(CodeAnalysis.CSharp.CSharpCompilation.Create("test2", references: references)
-                .AddSyntaxTrees(tree2));
+            Tuple<FunctionSignature, FunctionSignature> signatures = GetFunctionSignatures(function1, function2);
 
-            var signature1 = compilation1.GetEntryPointSignature(new FunctionEntryPointResolver());
-            var signature2 = compilation2.GetEntryPointSignature(new FunctionEntryPointResolver());
-            
-            Assert.True(signature1.Equals(signature2));
-            Assert.Equal(signature1.GetHashCode(), signature2.GetHashCode());
+            Assert.True(signatures.Item1.Equals(signatures.Item2));
+            Assert.Equal(signatures.Item1.GetHashCode(), signatures.Item2.GetHashCode());
         }
 
         [Fact]
@@ -148,6 +155,54 @@ public static void Run(string id, ICollection<Test> test1)
     
 }
 
+public class Test 
+{
+    public string Id { get; set; }
+}";
+
+            var tree = CSharpSyntaxTree.ParseText(function1, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
+            var references = new MetadataReference[] { MetadataReference.CreateFromFile(typeof(string).Assembly.Location) };
+            var compilation = CodeAnalysis.CSharp.CSharpCompilation.Create("test1", references: references).AddSyntaxTrees(tree);
+
+            var signature1 = new Script.Description.CSharpCompilation(compilation).GetEntryPointSignature(new FunctionEntryPointResolver());
+
+            Assert.True(signature1.HasLocalTypeReference);
+        }
+
+        [Fact]
+        public void Matches_IsTrue_WhenUsingLocalTypesInReturn()
+        {
+            var function1 = @"using System;
+using System.Collections.Generic;
+public static Test Run(string id)
+{
+    return null;
+}
+
+public class Test 
+{
+    public string Id { get; set; }
+}";
+
+            var tree = CSharpSyntaxTree.ParseText(function1, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));
+            var references = new MetadataReference[] { MetadataReference.CreateFromFile(typeof(string).Assembly.Location) };
+            var compilation = CodeAnalysis.CSharp.CSharpCompilation.Create("test1", references: references).AddSyntaxTrees(tree);
+
+            var signature1 = new Script.Description.CSharpCompilation(compilation).GetEntryPointSignature(new FunctionEntryPointResolver());
+
+            Assert.True(signature1.HasLocalTypeReference);
+        }
+
+        [Fact]
+        public void Matches_IsTrue_WhenUsingLocalTypesInGenericReturn()
+        {
+            var function1 = @"using System;
+using System.Collections.Generic;
+public static List<Test> Run(string id)
+{
+    return null;
+}
+
 public class Test 
 {
     public string Id { get; set; }
diff --git a/test/WebJobs.Script.Tests/Description/DotNet/DotNetFunctionInvokerTests.cs b/test/WebJobs.Script.Tests/Description/DotNet/DotNetFunctionInvokerTests.cs
@@ -150,7 +150,7 @@ public void ValidateFunctionBindingArguments_ReturnBinding_Succeeds()
             {
                 new FunctionParameter("input", "String", false, RefKind.None)
             };
-            FunctionSignature signature = new FunctionSignature("Test", "Test", ImmutableArray.CreateRange<FunctionParameter>(parameters), false);
+            FunctionSignature signature = new FunctionSignature("Test", "Test", ImmutableArray.CreateRange<FunctionParameter>(parameters), "Test", false);
 
             Collection<FunctionBinding> inputBindings = new Collection<FunctionBinding>()
             {

Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,7 @@ public void ValidateFunctionBindingArguments_ReturnBinding_Succeeds()`
`150`	`150`	`{`
`151`	`151`	`new FunctionParameter("input", "String", false, RefKind.None)`
`152`	`152`	`};`
`153`		`- FunctionSignature signature = new FunctionSignature("Test", "Test", ImmutableArray.CreateRange<FunctionParameter>(parameters), false);`
	`153`	`+ FunctionSignature signature = new FunctionSignature("Test", "Test", ImmutableArray.CreateRange<FunctionParameter>(parameters), "Test", false);`
`154`	`154`
`155`	`155`	`Collection<FunctionBinding> inputBindings = new Collection<FunctionBinding>()`
`156`	`156`	`{`