From 0a4c986b420124cc9d7b82c1d878924bf5e5b582 Mon Sep 17 00:00:00 2001 From: souvikghosh04 Date: Thu, 22 Jan 2026 13:36:08 +0530 Subject: [PATCH 1/6] [MCP] Prevent duplicating tool list between custom tools and describe_entities --- .../BuiltInTools/DescribeEntitiesTool.cs | 7 + .../Mcp/DescribeEntitiesFilteringTests.cs | 353 ++++++++++++++++++ 2 files changed, 360 insertions(+) create mode 100644 src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs diff --git a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs index c5b283214f..eea238d2dd 100644 --- a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs +++ b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs @@ -155,6 +155,13 @@ public Task ExecuteAsync( string entityName = entityEntry.Key; Entity entity = entityEntry.Value; + // Skip stored procedures exposed as custom tools - they appear in tools/list instead + if (entity.Source.Type == EntitySourceType.StoredProcedure && + entity.Mcp?.CustomToolEnabled == true) + { + continue; + } + if (!ShouldIncludeEntity(entityName, entityFilter)) { continue; diff --git a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs new file mode 100644 index 0000000000..24f3aba387 --- /dev/null +++ b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs @@ -0,0 +1,353 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.IO.Abstractions.TestingHelpers; +using System.Linq; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Auth; +using Azure.DataApiBuilder.Config; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.Authorization; +using Azure.DataApiBuilder.Core.Configurations; +using Azure.DataApiBuilder.Mcp.BuiltInTools; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using ModelContextProtocol.Protocol; +using Moq; + +namespace Azure.DataApiBuilder.Service.Tests.Mcp +{ + /// + /// Tests for DescribeEntitiesTool filtering logic, ensuring custom-tool stored procedures + /// are excluded from describe_entities results while regular entities remain visible. + /// + [TestClass] + public class DescribeEntitiesFilteringTests + { + /// + /// Test that custom-tool stored procedures are excluded from describe_entities results. + /// + [TestMethod] + public async Task DescribeEntities_ExcludesCustomToolStoredProcedures() + { + // Arrange + RuntimeConfig config = CreateConfigWithCustomToolSP(); + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + + // Act + CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); + + // Assert + // When all entities are custom-tool SPs, they're all filtered out, so we get an error + Assert.IsTrue(result.IsError == true); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("error", out JsonElement error)); + Assert.IsTrue(error.TryGetProperty("type", out JsonElement errorType)); + Assert.AreEqual("NoEntitiesConfigured", errorType.GetString()); + } + + /// + /// Test that regular stored procedures (without custom-tool) still appear in describe_entities. + /// + [TestMethod] + public async Task DescribeEntities_IncludesRegularStoredProcedures() + { + // Arrange + RuntimeConfig config = CreateConfigWithMixedStoredProcedures(); + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + + // Act + CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); + + // Assert + Assert.IsTrue(result.IsError == false || result.IsError == null); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); + + List entityNames = entities.EnumerateArray() + .Select(e => e.GetProperty("name").GetString()!) + .ToList(); + + // CountBooks has no custom-tool config, should be included + Assert.IsTrue(entityNames.Contains("CountBooks")); + // GetBook has custom-tool enabled, should be excluded + Assert.IsFalse(entityNames.Contains("GetBook")); + } + + /// + /// Test that tables and views are unaffected by custom-tool filtering. + /// + [TestMethod] + public async Task DescribeEntities_TablesAndViewsUnaffectedByFiltering() + { + // Arrange + RuntimeConfig config = CreateConfigWithMixedEntityTypes(); + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + + // Act + CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); + + // Assert + Assert.IsTrue(result.IsError == false || result.IsError == null); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); + + List entityNames = entities.EnumerateArray() + .Select(e => e.GetProperty("name").GetString()!) + .ToList(); + + // Tables and views should always appear + Assert.IsTrue(entityNames.Contains("Book")); + Assert.IsTrue(entityNames.Contains("BookView")); + // Custom-tool SP should be excluded + Assert.IsFalse(entityNames.Contains("GetBook")); + } + + /// + /// Test that entity count reflects filtered list (excludes custom-tool SPs). + /// + [TestMethod] + public async Task DescribeEntities_CountReflectsFilteredList() + { + // Arrange + RuntimeConfig config = CreateConfigWithMixedEntityTypes(); + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + + // Act + CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); + + // Assert + Assert.IsTrue(result.IsError == false || result.IsError == null); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); + + int entityCount = entities.GetArrayLength(); + + // Config has 3 entities: Book (table), BookView (view), GetBook (custom-tool SP) + // Only 2 should be returned (custom-tool SP excluded) + Assert.AreEqual(2, entityCount); + } + + /// + /// Test that nameOnly parameter works correctly with custom-tool filtering. + /// + [TestMethod] + public async Task DescribeEntities_NameOnlyWorksWithFiltering() + { + // Arrange + RuntimeConfig config = CreateConfigWithMixedEntityTypes(); + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + + JsonDocument arguments = JsonDocument.Parse("{\"nameOnly\": true}"); + + // Act + CallToolResult result = await tool.ExecuteAsync(arguments, serviceProvider, CancellationToken.None); + + // Assert + Assert.IsTrue(result.IsError == false || result.IsError == null); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); + + List entityNames = entities.EnumerateArray() + .Select(e => e.GetProperty("name").GetString()!) + .ToList(); + + // Should still exclude custom-tool SP even with nameOnly=true + Assert.IsTrue(entityNames.Contains("Book")); + Assert.IsTrue(entityNames.Contains("BookView")); + Assert.IsFalse(entityNames.Contains("GetBook")); + Assert.AreEqual(2, entities.GetArrayLength()); + } + + #region Helper Methods + + private static RuntimeConfig CreateConfigWithCustomToolSP() + { + Dictionary entities = new() + { + ["GetBook"] = new Entity( + Source: new("get_book", EntitySourceType.StoredProcedure, null, null), + GraphQL: new("GetBook", "GetBook"), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null, + Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: null) + ) + }; + + return new RuntimeConfig( + Schema: "test-schema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), + Runtime: new( + Rest: new(), + GraphQL: new(), + Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), + Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) + ), + Entities: new(entities) + ); + } + + private static RuntimeConfig CreateConfigWithMixedStoredProcedures() + { + Dictionary entities = new() + { + // Regular SP without custom-tool config + ["CountBooks"] = new Entity( + Source: new("count_books", EntitySourceType.StoredProcedure, null, null), + GraphQL: new("CountBooks", "CountBooks"), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null + ), + // SP with custom-tool enabled + ["GetBook"] = new Entity( + Source: new("get_book", EntitySourceType.StoredProcedure, null, null), + GraphQL: new("GetBook", "GetBook"), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null, + Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: null) + ) + }; + + return new RuntimeConfig( + Schema: "test-schema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), + Runtime: new( + Rest: new(), + GraphQL: new(), + Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), + Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) + ), + Entities: new(entities) + ); + } + + private static RuntimeConfig CreateConfigWithMixedEntityTypes() + { + Dictionary entities = new() + { + // Table + ["Book"] = new Entity( + Source: new("books", EntitySourceType.Table, null, null), + GraphQL: new("Book", "Books"), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Read, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null + ), + // View + ["BookView"] = new Entity( + Source: new("book_view", EntitySourceType.View, null, null), + GraphQL: new("BookView", "BookViews"), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Read, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null + ), + // Custom-tool SP + ["GetBook"] = new Entity( + Source: new("get_book", EntitySourceType.StoredProcedure, null, null), + GraphQL: new("GetBook", "GetBook"), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null, + Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: null) + ) + }; + + return new RuntimeConfig( + Schema: "test-schema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), + Runtime: new( + Rest: new(), + GraphQL: new(), + Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), + Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) + ), + Entities: new(entities) + ); + } + + private static IServiceProvider CreateServiceProvider(RuntimeConfig config) + { + ServiceCollection services = new(); + + // Create RuntimeConfigProvider with a test loader + MockFileSystem fileSystem = new(); + FileSystemRuntimeConfigLoader loader = new(fileSystem); + TestRuntimeConfigProvider configProvider = new(config, loader); + services.AddSingleton(configProvider); + + // Mock IAuthorizationResolver + Mock mockAuthResolver = new(); + mockAuthResolver.Setup(x => x.IsValidRoleContext(It.IsAny())).Returns(true); + services.AddSingleton(mockAuthResolver.Object); + + // Mock HttpContext with anonymous role + Mock mockHttpContext = new(); + Mock mockRequest = new(); + mockRequest.Setup(x => x.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns("anonymous"); + mockHttpContext.Setup(x => x.Request).Returns(mockRequest.Object); + + Mock mockHttpContextAccessor = new(); + mockHttpContextAccessor.Setup(x => x.HttpContext).Returns(mockHttpContext.Object); + services.AddSingleton(mockHttpContextAccessor.Object); + + // Add logging + services.AddLogging(); + + return services.BuildServiceProvider(); + } + + private static JsonElement GetContentFromResult(CallToolResult result) + { + Assert.IsNotNull(result.Content); + Assert.IsTrue(result.Content.Count > 0); + + ModelContextProtocol.Protocol.TextContentBlock firstContent = (ModelContextProtocol.Protocol.TextContentBlock)result.Content[0]; + Assert.IsNotNull(firstContent.Text); + + return JsonDocument.Parse(firstContent.Text).RootElement; + } + + #endregion + } + + /// + /// Test implementation of RuntimeConfigProvider that returns a fixed config. + /// + internal class TestRuntimeConfigProvider : RuntimeConfigProvider + { + private readonly RuntimeConfig _config; + + public TestRuntimeConfigProvider(RuntimeConfig config, FileSystemRuntimeConfigLoader loader) + : base(loader) + { + _config = config; + } + + public override RuntimeConfig GetConfig() => _config; + } +} From e701a87d70abe074570f184f13259a85f0a2f999 Mon Sep 17 00:00:00 2001 From: souvikghosh04 Date: Thu, 22 Jan 2026 14:14:38 +0530 Subject: [PATCH 2/6] Minor fixes and refactoring --- .../BuiltInTools/DescribeEntitiesTool.cs | 20 ++- .../Mcp/DescribeEntitiesFilteringTests.cs | 118 ++++++++++++++++-- 2 files changed, 127 insertions(+), 11 deletions(-) diff --git a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs index eea238d2dd..89742e23f1 100644 --- a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs +++ b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs @@ -146,6 +146,10 @@ public Task ExecuteAsync( List> entityList = new(); + // Track how many stored procedures were filtered out because they're exposed as custom tools. + // This helps provide a more specific error message when all entities are filtered. + int filteredCustomToolCount = 0; + if (runtimeConfig.Entities != null) { foreach (KeyValuePair entityEntry in runtimeConfig.Entities) @@ -155,10 +159,13 @@ public Task ExecuteAsync( string entityName = entityEntry.Key; Entity entity = entityEntry.Value; - // Skip stored procedures exposed as custom tools - they appear in tools/list instead + // Filter out stored procedures that are exposed as custom tools to prevent duplication. + // These entities appear in the MCP tools/list endpoint as dedicated tools (e.g., get_books, insert_book) + // and should not also appear in describe_entities which is for data entities (tables, views, regular SPs). if (entity.Source.Type == EntitySourceType.StoredProcedure && entity.Mcp?.CustomToolEnabled == true) { + filteredCustomToolCount++; continue; } @@ -184,6 +191,7 @@ public Task ExecuteAsync( if (entityList.Count == 0) { + // No entities matched the filter criteria if (entityFilter != null && entityFilter.Count > 0) { return Task.FromResult(McpResponseBuilder.BuildErrorResult( @@ -192,6 +200,16 @@ public Task ExecuteAsync( $"No entities found matching the filter: {string.Join(", ", entityFilter)}", logger)); } + // All entities were filtered because they're custom tools - provide specific guidance + else if (filteredCustomToolCount > 0) + { + return Task.FromResult(McpResponseBuilder.BuildErrorResult( + toolName, + "AllEntitiesFilteredAsCustomTools", + $"All {filteredCustomToolCount} configured entities are stored procedures exposed as custom tools. Custom tools appear in tools/list, not describe_entities. Use the tools/list endpoint to discover available custom tools.", + logger)); + } + // Truly no entities configured in the runtime config else { return Task.FromResult(McpResponseBuilder.BuildErrorResult( diff --git a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs index 24f3aba387..0a650bdf46 100644 --- a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs +++ b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs @@ -23,14 +23,18 @@ namespace Azure.DataApiBuilder.Service.Tests.Mcp { /// - /// Tests for DescribeEntitiesTool filtering logic, ensuring custom-tool stored procedures - /// are excluded from describe_entities results while regular entities remain visible. + /// Tests for DescribeEntitiesTool filtering logic (GitHub issue #3043). + /// Ensures stored procedures with custom-tool enabled are excluded from describe_entities results + /// to prevent duplication (they appear in tools/list instead). + /// Regular entities (tables, views, non-custom-tool SPs) remain visible in describe_entities. /// [TestClass] public class DescribeEntitiesFilteringTests { /// - /// Test that custom-tool stored procedures are excluded from describe_entities results. + /// Verifies that when ALL entities are stored procedures with custom-tool enabled, + /// describe_entities returns an AllEntitiesFilteredAsCustomTools error with guidance + /// to use tools/list instead. This ensures users understand why describe_entities is empty. /// [TestMethod] public async Task DescribeEntities_ExcludesCustomToolStoredProcedures() @@ -44,16 +48,24 @@ public async Task DescribeEntities_ExcludesCustomToolStoredProcedures() CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); // Assert - // When all entities are custom-tool SPs, they're all filtered out, so we get an error + // When all entities are custom-tool SPs, they're all filtered out, so we get a specific error Assert.IsTrue(result.IsError == true); JsonElement content = GetContentFromResult(result); Assert.IsTrue(content.TryGetProperty("error", out JsonElement error)); Assert.IsTrue(error.TryGetProperty("type", out JsonElement errorType)); - Assert.AreEqual("NoEntitiesConfigured", errorType.GetString()); + Assert.AreEqual("AllEntitiesFilteredAsCustomTools", errorType.GetString()); + + // Verify the error message is helpful + Assert.IsTrue(error.TryGetProperty("message", out JsonElement errorMessage)); + string message = errorMessage.GetString() ?? string.Empty; + Assert.IsTrue(message.Contains("custom tools")); + Assert.IsTrue(message.Contains("tools/list")); } /// - /// Test that regular stored procedures (without custom-tool) still appear in describe_entities. + /// Verifies that stored procedures WITHOUT custom-tool enabled still appear in describe_entities, + /// while stored procedures WITH custom-tool enabled are filtered out. + /// This ensures filtering is selective and only applies to custom-tool SPs. /// [TestMethod] public async Task DescribeEntities_IncludesRegularStoredProcedures() @@ -82,7 +94,9 @@ public async Task DescribeEntities_IncludesRegularStoredProcedures() } /// - /// Test that tables and views are unaffected by custom-tool filtering. + /// Verifies that custom-tool filtering ONLY applies to stored procedures. + /// Tables and views always appear in describe_entities regardless of any custom-tool configuration. + /// This ensures filtering doesn't accidentally hide non-SP entities. /// [TestMethod] public async Task DescribeEntities_TablesAndViewsUnaffectedByFiltering() @@ -112,7 +126,9 @@ public async Task DescribeEntities_TablesAndViewsUnaffectedByFiltering() } /// - /// Test that entity count reflects filtered list (excludes custom-tool SPs). + /// Verifies that the 'count' field in describe_entities response accurately reflects + /// the number of entities AFTER filtering (excludes custom-tool stored procedures). + /// This ensures count matches the actual entities array length. /// [TestMethod] public async Task DescribeEntities_CountReflectsFilteredList() @@ -138,7 +154,9 @@ public async Task DescribeEntities_CountReflectsFilteredList() } /// - /// Test that nameOnly parameter works correctly with custom-tool filtering. + /// Verifies that custom-tool filtering is applied consistently regardless of the nameOnly parameter. + /// When nameOnly=true (lightweight response), custom-tool SPs are still filtered out. + /// This ensures filtering behavior is consistent across both response modes. /// [TestMethod] public async Task DescribeEntities_NameOnlyWorksWithFiltering() @@ -169,8 +187,41 @@ public async Task DescribeEntities_NameOnlyWorksWithFiltering() Assert.AreEqual(2, entities.GetArrayLength()); } + /// + /// Test that NoEntitiesConfigured error is returned when runtime config truly has no entities. + /// This is different from AllEntitiesFilteredAsCustomTools where entities exist but are filtered. + /// + [TestMethod] + public async Task DescribeEntities_ReturnsNoEntitiesConfigured_WhenConfigHasNoEntities() + { + // Arrange + RuntimeConfig config = CreateConfigWithNoEntities(); + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + + // Act + CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); + + // Assert - Expect NoEntitiesConfigured (not AllEntitiesFilteredAsCustomTools) + // because the config truly has NO entities, not filtered entities + Assert.IsTrue(result.IsError == true); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("error", out JsonElement error)); + Assert.IsTrue(error.TryGetProperty("type", out JsonElement errorType)); + Assert.AreEqual("NoEntitiesConfigured", errorType.GetString()); + + // Verify the error message indicates no entities configured + Assert.IsTrue(error.TryGetProperty("message", out JsonElement errorMessage)); + string message = errorMessage.GetString() ?? string.Empty; + Assert.IsTrue(message.Contains("No entities are configured")); + } + #region Helper Methods + /// + /// Creates a runtime config with only custom-tool stored procedures. + /// Used to test the AllEntitiesFilteredAsCustomTools error scenario. + /// private static RuntimeConfig CreateConfigWithCustomToolSP() { Dictionary entities = new() @@ -200,6 +251,11 @@ private static RuntimeConfig CreateConfigWithCustomToolSP() ); } + /// + /// Creates a runtime config with mixed stored procedures: + /// one regular SP (CountBooks) and one custom-tool SP (GetBook). + /// Used to test that filtering is selective. + /// private static RuntimeConfig CreateConfigWithMixedStoredProcedures() { Dictionary entities = new() @@ -240,6 +296,11 @@ private static RuntimeConfig CreateConfigWithMixedStoredProcedures() ); } + /// + /// Creates a runtime config with mixed entity types: + /// table (Book), view (BookView), and custom-tool SP (GetBook). + /// Used to test that filtering only affects stored procedures. + /// private static RuntimeConfig CreateConfigWithMixedEntityTypes() { Dictionary entities = new() @@ -290,6 +351,30 @@ private static RuntimeConfig CreateConfigWithMixedEntityTypes() ); } + /// + /// Creates a runtime config with an empty entities dictionary. + /// Used to test the NoEntitiesConfigured error when no entities are configured at all. + /// + private static RuntimeConfig CreateConfigWithNoEntities() + { + // Create a config with no entities at all (empty dictionary) + return new RuntimeConfig( + Schema: "test-schema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), + Runtime: new( + Rest: new(), + GraphQL: new(), + Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), + Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) + ), + Entities: new(new Dictionary()) + ); + } + + /// + /// Creates a service provider with mocked dependencies for testing DescribeEntitiesTool. + /// Configures anonymous role and necessary DAB services. + /// private static IServiceProvider CreateServiceProvider(RuntimeConfig config) { ServiceCollection services = new(); @@ -321,6 +406,10 @@ private static IServiceProvider CreateServiceProvider(RuntimeConfig config) return services.BuildServiceProvider(); } + /// + /// Extracts and parses the JSON content from an MCP tool call result. + /// Returns the root JsonElement for assertion purposes. + /// private static JsonElement GetContentFromResult(CallToolResult result) { Assert.IsNotNull(result.Content); @@ -336,18 +425,27 @@ private static JsonElement GetContentFromResult(CallToolResult result) } /// - /// Test implementation of RuntimeConfigProvider that returns a fixed config. + /// Test implementation of RuntimeConfigProvider that returns a fixed configuration. + /// Used in unit tests to provide controlled RuntimeConfig instances without file system dependencies. /// internal class TestRuntimeConfigProvider : RuntimeConfigProvider { private readonly RuntimeConfig _config; + /// + /// Initializes a new instance with the specified configuration. + /// + /// The RuntimeConfig to return from GetConfig(). + /// The FileSystemRuntimeConfigLoader (required by base class). public TestRuntimeConfigProvider(RuntimeConfig config, FileSystemRuntimeConfigLoader loader) : base(loader) { _config = config; } + /// + /// Returns the fixed configuration provided during construction. + /// public override RuntimeConfig GetConfig() => _config; } } From c7937122b54c99387ca78c2d8ea458c0661099de Mon Sep 17 00:00:00 2001 From: souvikghosh04 Date: Thu, 22 Jan 2026 14:26:56 +0530 Subject: [PATCH 3/6] Copilot review fixes --- .../Mcp/DescribeEntitiesFilteringTests.cs | 41 ++++--------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs index 0a650bdf46..b0641fb534 100644 --- a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs +++ b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs @@ -3,13 +3,11 @@ using System; using System.Collections.Generic; -using System.IO.Abstractions.TestingHelpers; using System.Linq; using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Azure.DataApiBuilder.Auth; -using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Authorization; using Azure.DataApiBuilder.Core.Configurations; @@ -379,11 +377,9 @@ private static IServiceProvider CreateServiceProvider(RuntimeConfig config) { ServiceCollection services = new(); - // Create RuntimeConfigProvider with a test loader - MockFileSystem fileSystem = new(); - FileSystemRuntimeConfigLoader loader = new(fileSystem); - TestRuntimeConfigProvider configProvider = new(config, loader); - services.AddSingleton(configProvider); + // Use shared test helper to create RuntimeConfigProvider + RuntimeConfigProvider configProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(config); + services.AddSingleton(configProvider); // Mock IAuthorizationResolver Mock mockAuthResolver = new(); @@ -415,7 +411,11 @@ private static JsonElement GetContentFromResult(CallToolResult result) Assert.IsNotNull(result.Content); Assert.IsTrue(result.Content.Count > 0); - ModelContextProtocol.Protocol.TextContentBlock firstContent = (ModelContextProtocol.Protocol.TextContentBlock)result.Content[0]; + // Verify the content block is the expected type before casting + Assert.IsInstanceOfType(result.Content[0], typeof(TextContentBlock), + "Expected first content block to be TextContentBlock"); + + TextContentBlock firstContent = (TextContentBlock)result.Content[0]; Assert.IsNotNull(firstContent.Text); return JsonDocument.Parse(firstContent.Text).RootElement; @@ -423,29 +423,4 @@ private static JsonElement GetContentFromResult(CallToolResult result) #endregion } - - /// - /// Test implementation of RuntimeConfigProvider that returns a fixed configuration. - /// Used in unit tests to provide controlled RuntimeConfig instances without file system dependencies. - /// - internal class TestRuntimeConfigProvider : RuntimeConfigProvider - { - private readonly RuntimeConfig _config; - - /// - /// Initializes a new instance with the specified configuration. - /// - /// The RuntimeConfig to return from GetConfig(). - /// The FileSystemRuntimeConfigLoader (required by base class). - public TestRuntimeConfigProvider(RuntimeConfig config, FileSystemRuntimeConfigLoader loader) - : base(loader) - { - _config = config; - } - - /// - /// Returns the fixed configuration provided during construction. - /// - public override RuntimeConfig GetConfig() => _config; - } } From b9fd10664ff666eacb00878fd25702772c0c9411 Mon Sep 17 00:00:00 2001 From: souvikghosh04 Date: Thu, 22 Jan 2026 15:17:35 +0530 Subject: [PATCH 4/6] Copilot review fixes --- .../BuiltInTools/DescribeEntitiesTool.cs | 10 +++++++--- .../Mcp/DescribeEntitiesFilteringTests.cs | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs index 89742e23f1..18cbe4315d 100644 --- a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs +++ b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs @@ -200,8 +200,12 @@ public Task ExecuteAsync( $"No entities found matching the filter: {string.Join(", ", entityFilter)}", logger)); } - // All entities were filtered because they're custom tools - provide specific guidance - else if (filteredCustomToolCount > 0) + // All entities were filtered because they're custom tools - only return this specific error + // if ALL configured entities were filtered (not just some). This prevents misleading errors + // when entities fail to build for other reasons (e.g., exceptions in Build*EntityInfo). + else if (filteredCustomToolCount > 0 && + runtimeConfig.Entities != null && + filteredCustomToolCount == runtimeConfig.Entities.Entities.Count) { return Task.FromResult(McpResponseBuilder.BuildErrorResult( toolName, @@ -209,7 +213,7 @@ public Task ExecuteAsync( $"All {filteredCustomToolCount} configured entities are stored procedures exposed as custom tools. Custom tools appear in tools/list, not describe_entities. Use the tools/list endpoint to discover available custom tools.", logger)); } - // Truly no entities configured in the runtime config + // Truly no entities configured in the runtime config, or entities failed to build for other reasons else { return Task.FromResult(McpResponseBuilder.BuildErrorResult( diff --git a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs index b0641fb534..c040b1a4f9 100644 --- a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs +++ b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs @@ -149,6 +149,10 @@ public async Task DescribeEntities_CountReflectsFilteredList() // Config has 3 entities: Book (table), BookView (view), GetBook (custom-tool SP) // Only 2 should be returned (custom-tool SP excluded) Assert.AreEqual(2, entityCount); + + // Verify the count field in the response matches the filtered entity array length + Assert.IsTrue(content.TryGetProperty("count", out JsonElement countElement)); + Assert.AreEqual(entityCount, countElement.GetInt32()); } /// From e2cc3e8c18d03a62b28951be9e302f80c011a2ff Mon Sep 17 00:00:00 2001 From: souvikghosh04 Date: Thu, 22 Jan 2026 23:35:19 +0530 Subject: [PATCH 5/6] Fix behavior of dmltools and custom tools combinations --- .../BuiltInTools/DescribeEntitiesTool.cs | 16 ++-- .../Mcp/DescribeEntitiesFilteringTests.cs | 75 ++++++++++++++++++- 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs index 18cbe4315d..3248f99757 100644 --- a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs +++ b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs @@ -146,7 +146,7 @@ public Task ExecuteAsync( List> entityList = new(); - // Track how many stored procedures were filtered out because they're exposed as custom tools. + // Track how many stored procedures were filtered out because they're exposed as custom-tool-only. // This helps provide a more specific error message when all entities are filtered. int filteredCustomToolCount = 0; @@ -159,11 +159,13 @@ public Task ExecuteAsync( string entityName = entityEntry.Key; Entity entity = entityEntry.Value; - // Filter out stored procedures that are exposed as custom tools to prevent duplication. - // These entities appear in the MCP tools/list endpoint as dedicated tools (e.g., get_books, insert_book) - // and should not also appear in describe_entities which is for data entities (tables, views, regular SPs). + // Filter out stored procedures when dml-tools is explicitly disabled (false). + // This applies regardless of custom-tool setting: + // - custom-tool: true, dml-tools: false → Filtered (appears only in tools/list) + // - custom-tool: false, dml-tools: false → Filtered (entity fully disabled for MCP) + // When dml-tools is true (or default), the entity appears in describe_entities. if (entity.Source.Type == EntitySourceType.StoredProcedure && - entity.Mcp?.CustomToolEnabled == true) + entity.Mcp?.DmlToolEnabled == false) { filteredCustomToolCount++; continue; @@ -200,7 +202,7 @@ public Task ExecuteAsync( $"No entities found matching the filter: {string.Join(", ", entityFilter)}", logger)); } - // All entities were filtered because they're custom tools - only return this specific error + // All entities were filtered because they're custom-tool-only - only return this specific error // if ALL configured entities were filtered (not just some). This prevents misleading errors // when entities fail to build for other reasons (e.g., exceptions in Build*EntityInfo). else if (filteredCustomToolCount > 0 && @@ -210,7 +212,7 @@ public Task ExecuteAsync( return Task.FromResult(McpResponseBuilder.BuildErrorResult( toolName, "AllEntitiesFilteredAsCustomTools", - $"All {filteredCustomToolCount} configured entities are stored procedures exposed as custom tools. Custom tools appear in tools/list, not describe_entities. Use the tools/list endpoint to discover available custom tools.", + $"All {filteredCustomToolCount} configured entities are stored procedures exposed as custom-tool-only (dml-tools: false). Custom tools appear in tools/list, not describe_entities. Use the tools/list endpoint to discover available custom tools.", logger)); } // Truly no entities configured in the runtime config, or entities failed to build for other reasons diff --git a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs index c040b1a4f9..c71b754cca 100644 --- a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs +++ b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs @@ -218,6 +218,42 @@ public async Task DescribeEntities_ReturnsNoEntitiesConfigured_WhenConfigHasNoEn Assert.IsTrue(message.Contains("No entities are configured")); } + /// + /// CRITICAL TEST: Verifies that stored procedures with BOTH custom-tool AND dml-tools enabled + /// appear in describe_entities. This validates the truth table scenario: + /// custom-tool: true, dml-tools: true → ✔ describe_entities + ✔ tools/list + /// + /// This test ensures the filtering logic only filters when dml-tools is FALSE, + /// not just when custom-tool is TRUE. + /// + [TestMethod] + public async Task DescribeEntities_IncludesCustomToolWithDmlEnabled() + { + // Arrange + RuntimeConfig config = CreateConfigWithCustomToolAndDmlEnabled(); + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + + // Act + CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); + + // Assert + Assert.IsTrue(result.IsError == false || result.IsError == null); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); + + List entityNames = entities.EnumerateArray() + .Select(e => e.GetProperty("name").GetString()!) + .ToList(); + + // GetBook has custom-tool: true AND dml-tools: true, so it should APPEAR in describe_entities + Assert.IsTrue(entityNames.Contains("GetBook"), + "SP with custom-tool:true + dml-tools:true should appear in describe_entities"); + + // Should have exactly 1 entity + Assert.AreEqual(1, entities.GetArrayLength()); + } + #region Helper Methods /// @@ -236,7 +272,7 @@ private static RuntimeConfig CreateConfigWithCustomToolSP() Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, Mappings: null, Relationships: null, - Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: null) + Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false) ) }; @@ -281,7 +317,7 @@ private static RuntimeConfig CreateConfigWithMixedStoredProcedures() Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, Mappings: null, Relationships: null, - Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: null) + Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false) ) }; @@ -336,7 +372,7 @@ private static RuntimeConfig CreateConfigWithMixedEntityTypes() Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, Mappings: null, Relationships: null, - Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: null) + Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false) ) }; @@ -373,6 +409,39 @@ private static RuntimeConfig CreateConfigWithNoEntities() ); } + /// + /// Creates a runtime config with a stored procedure that has BOTH custom-tool and dml-tools enabled. + /// Used to test the truth table scenario: custom-tool:true + dml-tools:true → should appear in describe_entities. + /// + private static RuntimeConfig CreateConfigWithCustomToolAndDmlEnabled() + { + Dictionary entities = new() + { + ["GetBook"] = new Entity( + Source: new("get_book", EntitySourceType.StoredProcedure, null, null), + GraphQL: new("GetBook", "GetBook"), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null, + Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: true) + ) + }; + + return new RuntimeConfig( + Schema: "test-schema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), + Runtime: new( + Rest: new(), + GraphQL: new(), + Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), + Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) + ), + Entities: new(entities) + ); + } + /// /// Creates a service provider with mocked dependencies for testing DescribeEntitiesTool. /// Configures anonymous role and necessary DAB services. From 4376db7592d9c8c950fd1f301b407df65a6befab Mon Sep 17 00:00:00 2001 From: souvikghosh04 Date: Thu, 29 Jan 2026 17:09:44 +0530 Subject: [PATCH 6/6] Refactoring tests and implementing review comment suggesstions --- .../BuiltInTools/DescribeEntitiesTool.cs | 44 +- .../Mcp/DescribeEntitiesFilteringTests.cs | 443 +++++++++--------- 2 files changed, 251 insertions(+), 236 deletions(-) diff --git a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs index 3248f99757..00d72881ee 100644 --- a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs +++ b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/DescribeEntitiesTool.cs @@ -146,9 +146,9 @@ public Task ExecuteAsync( List> entityList = new(); - // Track how many stored procedures were filtered out because they're exposed as custom-tool-only. + // Track how many entities were filtered out because DML tools are disabled (dml-tools: false). // This helps provide a more specific error message when all entities are filtered. - int filteredCustomToolCount = 0; + int filteredDmlDisabledCount = 0; if (runtimeConfig.Entities != null) { @@ -159,15 +159,13 @@ public Task ExecuteAsync( string entityName = entityEntry.Key; Entity entity = entityEntry.Value; - // Filter out stored procedures when dml-tools is explicitly disabled (false). - // This applies regardless of custom-tool setting: - // - custom-tool: true, dml-tools: false → Filtered (appears only in tools/list) - // - custom-tool: false, dml-tools: false → Filtered (entity fully disabled for MCP) - // When dml-tools is true (or default), the entity appears in describe_entities. - if (entity.Source.Type == EntitySourceType.StoredProcedure && - entity.Mcp?.DmlToolEnabled == false) + // Filter out entities when dml-tools is explicitly disabled (false). + // This applies to all entity types (tables, views, stored procedures). + // When dml-tools is false, the entity is not exposed via DML tools + // (read_records, create_record, etc.) and should not appear in describe_entities. + if (entity.Mcp?.DmlToolEnabled == false) { - filteredCustomToolCount++; + filteredDmlDisabledCount++; continue; } @@ -202,17 +200,17 @@ public Task ExecuteAsync( $"No entities found matching the filter: {string.Join(", ", entityFilter)}", logger)); } - // All entities were filtered because they're custom-tool-only - only return this specific error - // if ALL configured entities were filtered (not just some). This prevents misleading errors - // when entities fail to build for other reasons (e.g., exceptions in Build*EntityInfo). - else if (filteredCustomToolCount > 0 && + // Return a specific error when ALL configured entities have dml-tools: false. + // Only show this error when every entity was intentionally filtered by the dml-tools check above, + // not when some entities failed to build due to exceptions in BuildBasicEntityInfo() or BuildFullEntityInfo() functions. + else if (filteredDmlDisabledCount > 0 && runtimeConfig.Entities != null && - filteredCustomToolCount == runtimeConfig.Entities.Entities.Count) + filteredDmlDisabledCount == runtimeConfig.Entities.Entities.Count) { return Task.FromResult(McpResponseBuilder.BuildErrorResult( toolName, - "AllEntitiesFilteredAsCustomTools", - $"All {filteredCustomToolCount} configured entities are stored procedures exposed as custom-tool-only (dml-tools: false). Custom tools appear in tools/list, not describe_entities. Use the tools/list endpoint to discover available custom tools.", + "AllEntitiesFilteredDmlDisabled", + $"All {filteredDmlDisabledCount} configured entities have DML tools disabled (dml-tools: false). Entities with dml-tools disabled do not appear in describe_entities. For stored procedures, check tools/list if custom-tool is enabled.", logger)); } // Truly no entities configured in the runtime config, or entities failed to build for other reasons @@ -238,6 +236,18 @@ public Task ExecuteAsync( ["count"] = finalEntityList.Count }; + // Log when entities were filtered due to DML tools disabled for visibility + if (filteredDmlDisabledCount > 0) + { + logger?.LogInformation( + "DescribeEntitiesTool: {FilteredCount} entity(ies) filtered with DML tools disabled (dml-tools: false). " + + "These entities are not exposed via DML tools and do not appear in describe_entities response. " + + "Returned {ReturnedCount} entities, filtered {FilteredCount}.", + filteredDmlDisabledCount, + finalEntityList.Count, + filteredDmlDisabledCount); + } + logger?.LogInformation( "DescribeEntitiesTool returned {EntityCount} entities. Response type: {ResponseType} (nameOnly={NameOnly}).", finalEntityList.Count, diff --git a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs index c71b754cca..2c509adb6a 100644 --- a/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs +++ b/src/Service.Tests/Mcp/DescribeEntitiesFilteringTests.cs @@ -31,7 +31,7 @@ public class DescribeEntitiesFilteringTests { /// /// Verifies that when ALL entities are stored procedures with custom-tool enabled, - /// describe_entities returns an AllEntitiesFilteredAsCustomTools error with guidance + /// describe_entities returns an AllEntitiesFilteredDmlDisabled error with guidance /// to use tools/list instead. This ensures users understand why describe_entities is empty. /// [TestMethod] @@ -46,18 +46,15 @@ public async Task DescribeEntities_ExcludesCustomToolStoredProcedures() CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); // Assert - // When all entities are custom-tool SPs, they're all filtered out, so we get a specific error - Assert.IsTrue(result.IsError == true); - JsonElement content = GetContentFromResult(result); - Assert.IsTrue(content.TryGetProperty("error", out JsonElement error)); - Assert.IsTrue(error.TryGetProperty("type", out JsonElement errorType)); - Assert.AreEqual("AllEntitiesFilteredAsCustomTools", errorType.GetString()); - + AssertErrorResult(result, "AllEntitiesFilteredDmlDisabled"); + // Verify the error message is helpful + JsonElement content = GetContentFromResult(result); + content.TryGetProperty("error", out JsonElement error); Assert.IsTrue(error.TryGetProperty("message", out JsonElement errorMessage)); string message = errorMessage.GetString() ?? string.Empty; - Assert.IsTrue(message.Contains("custom tools")); - Assert.IsTrue(message.Contains("tools/list")); + Assert.IsTrue(message.Contains("DML tools disabled") || message.Contains("dml-tools")); + Assert.IsTrue(message.Contains("tools/list") || message.Contains("custom-tool")); } /// @@ -70,25 +67,10 @@ public async Task DescribeEntities_IncludesRegularStoredProcedures() { // Arrange RuntimeConfig config = CreateConfigWithMixedStoredProcedures(); - IServiceProvider serviceProvider = CreateServiceProvider(config); - DescribeEntitiesTool tool = new(); - - // Act - CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); - // Assert - Assert.IsTrue(result.IsError == false || result.IsError == null); - JsonElement content = GetContentFromResult(result); - Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); - - List entityNames = entities.EnumerateArray() - .Select(e => e.GetProperty("name").GetString()!) - .ToList(); - - // CountBooks has no custom-tool config, should be included - Assert.IsTrue(entityNames.Contains("CountBooks")); - // GetBook has custom-tool enabled, should be excluded - Assert.IsFalse(entityNames.Contains("GetBook")); + // Act & Assert + CallToolResult result = await ExecuteToolAsync(config); + AssertSuccessResultWithEntityNames(result, new[] { "CountBooks" }, new[] { "GetBook" }); } /// @@ -99,28 +81,10 @@ public async Task DescribeEntities_IncludesRegularStoredProcedures() [TestMethod] public async Task DescribeEntities_TablesAndViewsUnaffectedByFiltering() { - // Arrange + // Arrange & Act & Assert RuntimeConfig config = CreateConfigWithMixedEntityTypes(); - IServiceProvider serviceProvider = CreateServiceProvider(config); - DescribeEntitiesTool tool = new(); - - // Act - CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); - - // Assert - Assert.IsTrue(result.IsError == false || result.IsError == null); - JsonElement content = GetContentFromResult(result); - Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); - - List entityNames = entities.EnumerateArray() - .Select(e => e.GetProperty("name").GetString()!) - .ToList(); - - // Tables and views should always appear - Assert.IsTrue(entityNames.Contains("Book")); - Assert.IsTrue(entityNames.Contains("BookView")); - // Custom-tool SP should be excluded - Assert.IsFalse(entityNames.Contains("GetBook")); + CallToolResult result = await ExecuteToolAsync(config); + AssertSuccessResultWithEntityNames(result, new[] { "Book", "BookView" }, new[] { "GetBook" }); } /// @@ -133,26 +97,19 @@ public async Task DescribeEntities_CountReflectsFilteredList() { // Arrange RuntimeConfig config = CreateConfigWithMixedEntityTypes(); - IServiceProvider serviceProvider = CreateServiceProvider(config); - DescribeEntitiesTool tool = new(); // Act - CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); + CallToolResult result = await ExecuteToolAsync(config); // Assert Assert.IsTrue(result.IsError == false || result.IsError == null); JsonElement content = GetContentFromResult(result); Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); - - int entityCount = entities.GetArrayLength(); - - // Config has 3 entities: Book (table), BookView (view), GetBook (custom-tool SP) - // Only 2 should be returned (custom-tool SP excluded) - Assert.AreEqual(2, entityCount); - - // Verify the count field in the response matches the filtered entity array length Assert.IsTrue(content.TryGetProperty("count", out JsonElement countElement)); - Assert.AreEqual(entityCount, countElement.GetInt32()); + + int entityCount = entities.GetArrayLength(); + Assert.AreEqual(2, entityCount, "Config has 3 entities but only 2 should be returned (custom-tool SP excluded)"); + Assert.AreEqual(entityCount, countElement.GetInt32(), "Count field should match filtered entity array length"); } /// @@ -167,26 +124,13 @@ public async Task DescribeEntities_NameOnlyWorksWithFiltering() RuntimeConfig config = CreateConfigWithMixedEntityTypes(); IServiceProvider serviceProvider = CreateServiceProvider(config); DescribeEntitiesTool tool = new(); - JsonDocument arguments = JsonDocument.Parse("{\"nameOnly\": true}"); // Act CallToolResult result = await tool.ExecuteAsync(arguments, serviceProvider, CancellationToken.None); // Assert - Assert.IsTrue(result.IsError == false || result.IsError == null); - JsonElement content = GetContentFromResult(result); - Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); - - List entityNames = entities.EnumerateArray() - .Select(e => e.GetProperty("name").GetString()!) - .ToList(); - - // Should still exclude custom-tool SP even with nameOnly=true - Assert.IsTrue(entityNames.Contains("Book")); - Assert.IsTrue(entityNames.Contains("BookView")); - Assert.IsFalse(entityNames.Contains("GetBook")); - Assert.AreEqual(2, entities.GetArrayLength()); + AssertSuccessResultWithEntityNames(result, new[] { "Book", "BookView" }, new[] { "GetBook" }); } /// @@ -196,23 +140,16 @@ public async Task DescribeEntities_NameOnlyWorksWithFiltering() [TestMethod] public async Task DescribeEntities_ReturnsNoEntitiesConfigured_WhenConfigHasNoEntities() { - // Arrange + // Arrange & Act RuntimeConfig config = CreateConfigWithNoEntities(); - IServiceProvider serviceProvider = CreateServiceProvider(config); - DescribeEntitiesTool tool = new(); - - // Act - CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); - - // Assert - Expect NoEntitiesConfigured (not AllEntitiesFilteredAsCustomTools) - // because the config truly has NO entities, not filtered entities - Assert.IsTrue(result.IsError == true); - JsonElement content = GetContentFromResult(result); - Assert.IsTrue(content.TryGetProperty("error", out JsonElement error)); - Assert.IsTrue(error.TryGetProperty("type", out JsonElement errorType)); - Assert.AreEqual("NoEntitiesConfigured", errorType.GetString()); + CallToolResult result = await ExecuteToolAsync(config); + // Assert + AssertErrorResult(result, "NoEntitiesConfigured"); + // Verify the error message indicates no entities configured + JsonElement content = GetContentFromResult(result); + content.TryGetProperty("error", out JsonElement error); Assert.IsTrue(error.TryGetProperty("message", out JsonElement errorMessage)); string message = errorMessage.GetString() ?? string.Empty; Assert.IsTrue(message.Contains("No entities are configured")); @@ -229,8 +166,46 @@ public async Task DescribeEntities_ReturnsNoEntitiesConfigured_WhenConfigHasNoEn [TestMethod] public async Task DescribeEntities_IncludesCustomToolWithDmlEnabled() { - // Arrange + // Arrange & Act RuntimeConfig config = CreateConfigWithCustomToolAndDmlEnabled(); + CallToolResult result = await ExecuteToolAsync(config); + + // Assert + AssertSuccessResultWithEntityNames(result, new[] { "GetBook" }, Array.Empty()); + } + + /// + /// Verifies that when some (but not all) entities are filtered as custom-tool-only, + /// the filtering is logged but does not affect the response content. + /// The response should contain only the non-filtered entities. + /// + [TestMethod] + public async Task DescribeEntities_LogsFilteringInfo_WhenSomeEntitiesFiltered() + { + // Arrange & Act + RuntimeConfig config = CreateConfigWithMixedEntityTypes(); + CallToolResult result = await ExecuteToolAsync(config); + + // Assert + AssertSuccessResultWithEntityNames(result, new[] { "Book", "BookView" }, new[] { "GetBook" }); + + // Verify count matches + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("count", out JsonElement countElement)); + Assert.AreEqual(2, countElement.GetInt32()); + } + + /// + /// Verifies that entities with DML tools disabled (dml-tools: false) are filtered from describe_entities. + /// This ensures the filtering applies to all entity types, not just stored procedures. + /// + [DataTestMethod] + [DataRow(EntitySourceType.Table, "Publisher", "Book", DisplayName = "Filters Table with DML disabled")] + [DataRow(EntitySourceType.View, "Book", "BookView", DisplayName = "Filters View with DML disabled")] + public async Task DescribeEntities_FiltersEntityWithDmlToolsDisabled(EntitySourceType filteredEntityType, string includedEntityName, string filteredEntityName) + { + // Arrange + RuntimeConfig config = CreateConfigWithEntityDmlDisabled(filteredEntityType, includedEntityName, filteredEntityName); IServiceProvider serviceProvider = CreateServiceProvider(config); DescribeEntitiesTool tool = new(); @@ -238,6 +213,49 @@ public async Task DescribeEntities_IncludesCustomToolWithDmlEnabled() CallToolResult result = await tool.ExecuteAsync(null, serviceProvider, CancellationToken.None); // Assert + AssertSuccessResultWithEntityNames(result, new[] { includedEntityName }, new[] { filteredEntityName }); + } + + /// + /// Verifies that when ALL entities have dml-tools disabled, the appropriate error is returned. + /// This tests the error scenario applies to all entity types, not just stored procedures. + /// + [TestMethod] + public async Task DescribeEntities_ReturnsAllEntitiesFilteredDmlDisabled_WhenAllEntitiesHaveDmlDisabled() + { + // Arrange & Act + RuntimeConfig config = CreateConfigWithAllEntitiesDmlDisabled(); + CallToolResult result = await ExecuteToolAsync(config); + + // Assert + AssertErrorResult(result, "AllEntitiesFilteredDmlDisabled"); + + // Verify the error message is helpful + JsonElement content = GetContentFromResult(result); + content.TryGetProperty("error", out JsonElement error); + Assert.IsTrue(error.TryGetProperty("message", out JsonElement errorMessage)); + string message = errorMessage.GetString() ?? string.Empty; + Assert.IsTrue(message.Contains("DML tools disabled"), "Error message should mention DML tools disabled"); + Assert.IsTrue(message.Contains("dml-tools: false"), "Error message should mention the config syntax"); + } + + #region Helper Methods + + /// + /// Executes the DescribeEntitiesTool with the given config. + /// + private static async Task ExecuteToolAsync(RuntimeConfig config, JsonDocument arguments = null) + { + IServiceProvider serviceProvider = CreateServiceProvider(config); + DescribeEntitiesTool tool = new(); + return await tool.ExecuteAsync(arguments, serviceProvider, CancellationToken.None); + } + + /// + /// Runs the DescribeEntitiesTool and asserts successful execution with expected entity names. + /// + private static void AssertSuccessResultWithEntityNames(CallToolResult result, string[] includedEntities, string[] excludedEntities) + { Assert.IsTrue(result.IsError == false || result.IsError == null); JsonElement content = GetContentFromResult(result); Assert.IsTrue(content.TryGetProperty("entities", out JsonElement entities)); @@ -246,36 +264,57 @@ public async Task DescribeEntities_IncludesCustomToolWithDmlEnabled() .Select(e => e.GetProperty("name").GetString()!) .ToList(); - // GetBook has custom-tool: true AND dml-tools: true, so it should APPEAR in describe_entities - Assert.IsTrue(entityNames.Contains("GetBook"), - "SP with custom-tool:true + dml-tools:true should appear in describe_entities"); + foreach (string includedEntity in includedEntities) + { + Assert.IsTrue(entityNames.Contains(includedEntity), $"{includedEntity} should be included"); + } + + foreach (string excludedEntity in excludedEntities) + { + Assert.IsFalse(entityNames.Contains(excludedEntity), $"{excludedEntity} should be excluded"); + } - // Should have exactly 1 entity - Assert.AreEqual(1, entities.GetArrayLength()); + Assert.AreEqual(includedEntities.Length, entities.GetArrayLength()); } - #region Helper Methods + /// + /// Asserts that the result contains an error with the specified type. + /// + private static void AssertErrorResult(CallToolResult result, string expectedErrorType) + { + Assert.IsTrue(result.IsError == true); + JsonElement content = GetContentFromResult(result); + Assert.IsTrue(content.TryGetProperty("error", out JsonElement error)); + Assert.IsTrue(error.TryGetProperty("type", out JsonElement errorType)); + Assert.AreEqual(expectedErrorType, errorType.GetString()); + } /// - /// Creates a runtime config with only custom-tool stored procedures. - /// Used to test the AllEntitiesFilteredAsCustomTools error scenario. + /// Creates a basic entity with standard permissions. /// - private static RuntimeConfig CreateConfigWithCustomToolSP() + private static Entity CreateEntity(string sourceName, EntitySourceType sourceType, string singularName, string pluralName, EntityMcpOptions mcpOptions = null) { - Dictionary entities = new() - { - ["GetBook"] = new Entity( - Source: new("get_book", EntitySourceType.StoredProcedure, null, null), - GraphQL: new("GetBook", "GetBook"), - Fields: null, - Rest: new(Enabled: true), - Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, - Mappings: null, - Relationships: null, - Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false) - ) - }; + EntityActionOperation action = sourceType == EntitySourceType.StoredProcedure + ? EntityActionOperation.Execute + : EntityActionOperation.Read; + + return new Entity( + Source: new(sourceName, sourceType, null, null), + GraphQL: new(singularName, pluralName), + Fields: null, + Rest: new(Enabled: true), + Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: action, Fields: null, Policy: null) }) }, + Mappings: null, + Relationships: null, + Mcp: mcpOptions + ); + } + /// + /// Creates a runtime config with the specified entities. + /// + private static RuntimeConfig CreateRuntimeConfig(Dictionary entities) + { return new RuntimeConfig( Schema: "test-schema", DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), @@ -289,6 +328,21 @@ private static RuntimeConfig CreateConfigWithCustomToolSP() ); } + /// + /// Creates a runtime config with only custom-tool stored procedures. + /// Used to test the AllEntitiesFilteredAsCustomTools error scenario. + /// + private static RuntimeConfig CreateConfigWithCustomToolSP() + { + Dictionary entities = new() + { + ["GetBook"] = CreateEntity("get_book", EntitySourceType.StoredProcedure, "GetBook", "GetBook", + new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false)) + }; + + return CreateRuntimeConfig(entities); + } + /// /// Creates a runtime config with mixed stored procedures: /// one regular SP (CountBooks) and one custom-tool SP (GetBook). @@ -298,40 +352,12 @@ private static RuntimeConfig CreateConfigWithMixedStoredProcedures() { Dictionary entities = new() { - // Regular SP without custom-tool config - ["CountBooks"] = new Entity( - Source: new("count_books", EntitySourceType.StoredProcedure, null, null), - GraphQL: new("CountBooks", "CountBooks"), - Fields: null, - Rest: new(Enabled: true), - Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, - Mappings: null, - Relationships: null - ), - // SP with custom-tool enabled - ["GetBook"] = new Entity( - Source: new("get_book", EntitySourceType.StoredProcedure, null, null), - GraphQL: new("GetBook", "GetBook"), - Fields: null, - Rest: new(Enabled: true), - Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, - Mappings: null, - Relationships: null, - Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false) - ) + ["CountBooks"] = CreateEntity("count_books", EntitySourceType.StoredProcedure, "CountBooks", "CountBooks"), + ["GetBook"] = CreateEntity("get_book", EntitySourceType.StoredProcedure, "GetBook", "GetBook", + new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false)) }; - return new RuntimeConfig( - Schema: "test-schema", - DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), - Runtime: new( - Rest: new(), - GraphQL: new(), - Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), - Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) - ), - Entities: new(entities) - ); + return CreateRuntimeConfig(entities); } /// @@ -343,50 +369,13 @@ private static RuntimeConfig CreateConfigWithMixedEntityTypes() { Dictionary entities = new() { - // Table - ["Book"] = new Entity( - Source: new("books", EntitySourceType.Table, null, null), - GraphQL: new("Book", "Books"), - Fields: null, - Rest: new(Enabled: true), - Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Read, Fields: null, Policy: null) }) }, - Mappings: null, - Relationships: null - ), - // View - ["BookView"] = new Entity( - Source: new("book_view", EntitySourceType.View, null, null), - GraphQL: new("BookView", "BookViews"), - Fields: null, - Rest: new(Enabled: true), - Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Read, Fields: null, Policy: null) }) }, - Mappings: null, - Relationships: null - ), - // Custom-tool SP - ["GetBook"] = new Entity( - Source: new("get_book", EntitySourceType.StoredProcedure, null, null), - GraphQL: new("GetBook", "GetBook"), - Fields: null, - Rest: new(Enabled: true), - Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, - Mappings: null, - Relationships: null, - Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false) - ) + ["Book"] = CreateEntity("books", EntitySourceType.Table, "Book", "Books"), + ["BookView"] = CreateEntity("book_view", EntitySourceType.View, "BookView", "BookViews"), + ["GetBook"] = CreateEntity("get_book", EntitySourceType.StoredProcedure, "GetBook", "GetBook", + new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: false)) }; - return new RuntimeConfig( - Schema: "test-schema", - DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), - Runtime: new( - Rest: new(), - GraphQL: new(), - Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), - Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) - ), - Entities: new(entities) - ); + return CreateRuntimeConfig(entities); } /// @@ -395,18 +384,7 @@ private static RuntimeConfig CreateConfigWithMixedEntityTypes() /// private static RuntimeConfig CreateConfigWithNoEntities() { - // Create a config with no entities at all (empty dictionary) - return new RuntimeConfig( - Schema: "test-schema", - DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), - Runtime: new( - Rest: new(), - GraphQL: new(), - Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), - Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) - ), - Entities: new(new Dictionary()) - ); + return CreateRuntimeConfig(new Dictionary()); } /// @@ -417,29 +395,56 @@ private static RuntimeConfig CreateConfigWithCustomToolAndDmlEnabled() { Dictionary entities = new() { - ["GetBook"] = new Entity( - Source: new("get_book", EntitySourceType.StoredProcedure, null, null), - GraphQL: new("GetBook", "GetBook"), - Fields: null, - Rest: new(Enabled: true), - Permissions: new[] { new EntityPermission(Role: "anonymous", Actions: new[] { new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) }) }, - Mappings: null, - Relationships: null, - Mcp: new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: true) - ) + ["GetBook"] = CreateEntity("get_book", EntitySourceType.StoredProcedure, "GetBook", "GetBook", + new EntityMcpOptions(customToolEnabled: true, dmlToolsEnabled: true)) }; - return new RuntimeConfig( - Schema: "test-schema", - DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), - Runtime: new( - Rest: new(), - GraphQL: new(), - Mcp: new(Enabled: true, Path: "/mcp", DmlTools: null), - Host: new(Cors: null, Authentication: null, Mode: HostMode.Development) - ), - Entities: new(entities) - ); + return CreateRuntimeConfig(entities); + } + + /// + /// Creates a runtime config with an entity that has dml-tools disabled. + /// Used to test that entities with dml-tools: false are filtered from describe_entities. + /// + private static RuntimeConfig CreateConfigWithEntityDmlDisabled(EntitySourceType filteredEntityType, string includedEntityName, string filteredEntityName) + { + Dictionary entities = new(); + + // Add the included entity (different type based on what's being filtered) + if (filteredEntityType == EntitySourceType.Table) + { + entities[includedEntityName] = CreateEntity("publishers", EntitySourceType.Table, includedEntityName, $"{includedEntityName}s", + new EntityMcpOptions(customToolEnabled: null, dmlToolsEnabled: true)); + entities[filteredEntityName] = CreateEntity("books", EntitySourceType.Table, filteredEntityName, $"{filteredEntityName}s", + new EntityMcpOptions(customToolEnabled: null, dmlToolsEnabled: false)); + } + else if (filteredEntityType == EntitySourceType.View) + { + entities[includedEntityName] = CreateEntity("books", EntitySourceType.Table, includedEntityName, $"{includedEntityName}s"); + entities[filteredEntityName] = CreateEntity("book_view", EntitySourceType.View, filteredEntityName, $"{filteredEntityName}s", + new EntityMcpOptions(customToolEnabled: null, dmlToolsEnabled: false)); + } + + return CreateRuntimeConfig(entities); + } + + /// + /// Creates a runtime config where all entities have dml-tools disabled. + /// Used to test the AllEntitiesFilteredDmlDisabled error scenario. + /// + private static RuntimeConfig CreateConfigWithAllEntitiesDmlDisabled() + { + Dictionary entities = new() + { + ["Book"] = CreateEntity("books", EntitySourceType.Table, "Book", "Books", + new EntityMcpOptions(customToolEnabled: null, dmlToolsEnabled: false)), + ["BookView"] = CreateEntity("book_view", EntitySourceType.View, "BookView", "BookViews", + new EntityMcpOptions(customToolEnabled: null, dmlToolsEnabled: false)), + ["GetBook"] = CreateEntity("get_book", EntitySourceType.StoredProcedure, "GetBook", "GetBook", + new EntityMcpOptions(customToolEnabled: false, dmlToolsEnabled: false)) + }; + + return CreateRuntimeConfig(entities); } ///