Fix concurrency issues and double lookups in SkillCollection

dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SKContextExtensionsTests.cs

@@ -93,9 +93,9 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsAsync()
                 x.SearchAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<int>(), It.IsAny<double>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
             .Returns(asyncEnumerable);
 
-        skills.Setup(x => x.HasFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
-        skills.Setup(x => x.HasSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
-        skills.Setup(x => x.HasNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
+        skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
+        skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
+        skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
         skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
         skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
         skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
@@ -163,9 +163,9 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsWithRelevancyAsync()
                 x.SearchAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<int>(), It.IsAny<double>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
             .Returns(asyncEnumerable);
 
-        skills.Setup(x => x.HasFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
-        skills.Setup(x => x.HasSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
-        skills.Setup(x => x.HasNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
+        skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
+        skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
+        skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
         skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
         skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
         skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);

dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlanParserTests.cs

@@ -94,13 +94,15 @@ private static Mock<ISKFunction> CreateMockFunction(FunctionView functionView, s
                 {
                     skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
                         .Returns(mockFunction.Object);
-                    skills.Setup(x => x.HasSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
+                    ISKFunction? outFunc = mockFunction.Object;
+                    skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
                 }
                 else
                 {
                     skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
                         .Returns(mockFunction.Object);
-                    skills.Setup(x => x.HasNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
+                    ISKFunction? outFunc = mockFunction.Object;
+                    skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
                 }
             }
         }

dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlannerTests.cs

@@ -57,13 +57,15 @@ public async Task ItCanCreatePlanAsync(string goal)
             {
                 skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
                     .Returns(mockFunction.Object);
-                skills.Setup(x => x.HasSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
+                ISKFunction? outFunc = mockFunction.Object;
+                skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
             }
             else
             {
                 skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
                     .Returns(mockFunction.Object);
-                skills.Setup(x => x.HasNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
+                ISKFunction? outFunc = mockFunction.Object;
+                skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
             }
         }
 

dotnet/src/SemanticKernel.Abstractions/Orchestration/SKContext.cs

@@ -100,12 +100,7 @@ public ISKFunction Func(string skillName, string functionName)
                 "Skill collection not found in the context");
         }
 
-        if (this.Skills.HasNativeFunction(skillName, functionName))
-        {
-            return this.Skills.GetNativeFunction(skillName, functionName);
-        }
-
-        return this.Skills.GetSemanticFunction(skillName, functionName);
+        return this.Skills.GetFunction(skillName, functionName);
     }
 
     /// <summary>

dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs

@@ -7,92 +7,110 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
 /// <summary>
 /// Read-only skill collection interface.
 /// </summary>
-[SuppressMessage("Naming", "CA1710:Identifiers should have correct suffix", Justification = "It is a collection")]
-[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
+[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
 public interface IReadOnlySkillCollection
 {
     /// <summary>
-    /// Check if the collection contains the specified function in the global skill, regardless of the function type
+    /// Gets the function stored in the collection.
     /// </summary>
-    /// <param name="skillName">Skill name</param>
-    /// <param name="functionName">Function name</param>
-    /// <returns>True if the function exists, false otherwise</returns>
-    bool HasFunction(string skillName, string functionName);
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <returns>The function retrieved from the collection.</returns>
+    /// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
+    ISKFunction GetFunction(string functionName);
 
     /// <summary>
-    /// Check if the collection contains the specified function, regardless of the function type
+    /// Gets the function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <returns>True if the function exists, false otherwise</returns>
-    bool HasFunction(string functionName);
+    /// <param name="skillName">The name of the skill with which the function is associated.</param>
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <returns>The function retrieved from the collection.</returns>
+    /// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
+    ISKFunction GetFunction(string skillName, string functionName);
 
     /// <summary>
-    /// Check if a semantic function is registered
+    /// Gets the function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <param name="skillName">Skill name</param>
-    /// <returns>True if the function exists</returns>
-    bool HasSemanticFunction(string skillName, string functionName);
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
+    /// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
+    bool TryGetFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
 
     /// <summary>
-    /// Check if a native function is registered
+    /// Gets the function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <param name="skillName">Skill name</param>
-    /// <returns>True if the function exists</returns>
-    bool HasNativeFunction(string skillName, string functionName);
+    /// <param name="skillName">The name of the skill with which the function is associated.</param>
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
+    /// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
+    bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
 
     /// <summary>
-    /// Check if a native function is registered in the global skill
+    /// Gets the semantic function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <returns>True if the function exists</returns>
-    bool HasNativeFunction(string functionName);
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <returns>The function retrieved from the collection.</returns>
+    /// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
+    ISKFunction GetSemanticFunction(string functionName);
 
     /// <summary>
-    /// Return the function delegate stored in the collection, regardless of the function type
+    /// Gets the semantic function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <returns>Function delegate</returns>
-    ISKFunction GetFunction(string functionName);
+    /// <param name="skillName">The name of the skill with which the function is associated.</param>
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <returns>The function retrieved from the collection.</returns>
+    /// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
+    ISKFunction GetSemanticFunction(string skillName, string functionName);
 
     /// <summary>
-    /// Return the function delegate stored in the collection, regardless of the function type
+    /// Gets the semantic function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <param name="skillName">Skill name</param>
-    /// <returns>Function delegate</returns>
-    ISKFunction GetFunction(string skillName, string functionName);
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
+    /// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
+    bool TryGetSemanticFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
 
     /// <summary>
-    /// Return the semantic function delegate stored in the collection
+    /// Gets the semantic function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <returns>Semantic function delegate</returns>
-    ISKFunction GetSemanticFunction(string functionName);
+    /// <param name="skillName">The name of the skill with which the function is associated.</param>
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
+    /// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
+    bool TryGetSemanticFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
 
     /// <summary>
-    /// Return the semantic function delegate stored in the collection
+    /// Gets the native function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <param name="skillName">Skill name</param>
-    /// <returns>Semantic function delegate</returns>
-    ISKFunction GetSemanticFunction(string skillName, string functionName);
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <returns>The function retrieved from the collection.</returns>
+    /// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
+    ISKFunction GetNativeFunction(string functionName);
 
     /// <summary>
-    /// Return the native function delegate stored in the collection
+    /// Gets the native function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <param name="skillName">Skill name</param>
-    /// <returns>Native function delegate</returns>
+    /// <param name="skillName">The name of the skill with which the function is associated.</param>
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <returns>The function retrieved from the collection.</returns>
+    /// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
     ISKFunction GetNativeFunction(string skillName, string functionName);
 
     /// <summary>
-    /// Return the native function delegate stored in the collection
+    /// Gets the native function stored in the collection.
     /// </summary>
-    /// <param name="functionName">Function name</param>
-    /// <returns>Native function delegate</returns>
-    ISKFunction GetNativeFunction(string functionName);
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
+    /// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
+    bool TryGetNativeFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
+
+    /// <summary>
+    /// Gets the native function stored in the collection.
+    /// </summary>
+    /// <param name="skillName">The name of the skill with which the function is associated.</param>
+    /// <param name="functionName">The name of the function to retrieve.</param>
+    /// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
+    /// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
+    bool TryGetNativeFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
 
     /// <summary>
     /// Get all registered functions details, minus the delegates

dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs

@@ -7,8 +7,7 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
 /// <summary>
 /// Skill collection interface.
 /// </summary>
-[SuppressMessage("Naming", "CA1710:Identifiers should have correct suffix", Justification = "It is a collection")]
-[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
+[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
 public interface ISkillCollection : IReadOnlySkillCollection
 {
     /// <summary>

dotnet/src/SemanticKernel.UnitTests/KernelTests.cs

@@ -132,7 +132,8 @@ public void ItAllowsToImportSkillsInTheGlobalNamespace()
 
         // Assert
         Assert.Equal(3, skill.Count);
-        Assert.True(kernel.Skills.HasNativeFunction("GetAnyValue"));
+        Assert.True(kernel.Skills.TryGetNativeFunction("GetAnyValue", out ISKFunction? functionInstance));
+        Assert.NotNull(functionInstance);
     }
 
     [Fact]

dotnet/src/SemanticKernel.UnitTests/Planning/PlanSerializationTests.cs

@@ -441,7 +441,8 @@ public async Task CanStepAndSerializeAndDeserializePlanWithStepsAndContextAsync(
             }
         });
 
-        skills.Setup(x => x.HasNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
+        ISKFunction? outFunc = mockFunction.Object;
+        skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out outFunc)).Returns(true);
         skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockFunction.Object);
 
         plan.AddSteps(mockFunction.Object, mockFunction.Object);

dotnet/src/SemanticKernel.UnitTests/TemplateEngine/Blocks/CodeBlockTests.cs

@@ -33,7 +33,7 @@ public async Task ItThrowsIfAFunctionDoesntExistAsync()
     {
         // Arrange
         var context = new SKContext(new ContextVariables(), NullMemory.Instance, this._skills.Object, this._log.Object);
-        this._skills.Setup(x => x.HasFunction("functionName")).Returns(false);
+        this._skills.Setup(x => x.TryGetFunction("functionName", out It.Ref<ISKFunction?>.IsAny)).Returns(false);
         var target = new CodeBlock("functionName", this._log.Object);
 
         // Act
@@ -52,7 +52,8 @@ public async Task ItThrowsIfAFunctionCallThrowsAsync()
         function
             .Setup(x => x.InvokeAsync(It.IsAny<SKContext?>(), It.IsAny<CompleteRequestSettings?>(), It.IsAny<ILogger?>(), It.IsAny<CancellationToken>()))
             .Throws(new RuntimeWrappedException("error"));
-        this._skills.Setup(x => x.HasFunction("functionName")).Returns(true);
+        ISKFunction? outFunc = function.Object;
+        this._skills.Setup(x => x.TryGetFunction("functionName", out outFunc)).Returns(true);
         this._skills.Setup(x => x.GetFunction("functionName")).Returns(function.Object);
         var target = new CodeBlock("functionName", this._log.Object);
 
@@ -209,7 +210,8 @@ public async Task ItInvokesFunctionCloningAllVariablesAsync()
                 ctx["var2"] = "overridden";
             });
 
-        this._skills.Setup(x => x.HasFunction(Func)).Returns(true);
+        ISKFunction? outFunc = function.Object;
+        this._skills.Setup(x => x.TryGetFunction(Func, out outFunc)).Returns(true);
         this._skills.Setup(x => x.GetFunction(Func)).Returns(function.Object);
 
         // Act
@@ -249,7 +251,8 @@ public async Task ItInvokesFunctionWithCustomVariableAsync()
                 canary = ctx!["input"];
             });
 
-        this._skills.Setup(x => x.HasFunction(Func)).Returns(true);
+        ISKFunction? outFunc = function.Object;
+        this._skills.Setup(x => x.TryGetFunction(Func, out outFunc)).Returns(true);
         this._skills.Setup(x => x.GetFunction(Func)).Returns(function.Object);
 
         // Act
@@ -281,7 +284,8 @@ public async Task ItInvokesFunctionWithCustomValueAsync()
                 canary = ctx!["input"];
             });
 
-        this._skills.Setup(x => x.HasFunction(Func)).Returns(true);
+        ISKFunction? outFunc = function.Object;
+        this._skills.Setup(x => x.TryGetFunction(Func, out outFunc)).Returns(true);
         this._skills.Setup(x => x.GetFunction(Func)).Returns(function.Object);
 
         // Act