diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index 9ccd734c08..c9d982a388 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -1011,6 +1011,41 @@ public void ValidateEntityConfiguration(RuntimeConfig runtimeConfig) ValidateNameRequirements(entity.GraphQL.Singular); ValidateNameRequirements(entity.GraphQL.Plural); } + + } + } + + /// + /// Validates that no stored-procedure entity in the config declares duplicate parameter names. + /// Duplicate names produce inconsistent behavior across GraphQL, OpenAPI, and MCP because each + /// consumer resolves duplicates differently (first-wins vs. last-wins). This check runs in both + /// development and production mode so that ambiguous configs are rejected at startup regardless + /// of the host mode. + /// + /// The runtime configuration. + public void ValidateStoredProcedureDuplicateParameters(RuntimeConfig runtimeConfig) + { + foreach ((string entityName, Entity entity) in runtimeConfig.Entities) + { + if (entity.Source.Type is not EntitySourceType.StoredProcedure + || entity.Source.Parameters is null) + { + continue; + } + + HashSet seenParamNames = new(StringComparer.Ordinal); + foreach (ParameterMetadata param in entity.Source.Parameters) + { + if (!seenParamNames.Add(param.Name)) + { + HandleOrRecordException(new DataApiBuilderException( + message: $"Entity '{entityName}' has duplicate parameter name '{param.Name}' in its stored procedure parameters configuration. " + + "Parameter names must be unique.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); + break; + } + } } } @@ -1915,6 +1950,9 @@ private static bool IsLoggerFilterValid(string loggerFilter) /// The runtime configuration. public void ValidateEntityAndAutoentityConfigurations(RuntimeConfig runtimeConfig) { + // Runs in both modes: duplicate SP parameter names cause silent inconsistency at runtime. + ValidateStoredProcedureDuplicateParameters(runtimeConfig); + if (runtimeConfig.IsDevelopmentMode()) { ValidateEntityConfiguration(runtimeConfig); diff --git a/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs b/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs index 4052198efd..8b8338fcd1 100644 --- a/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs +++ b/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs @@ -83,13 +83,17 @@ public static FieldDefinitionNode GenerateStoredProcedureSchema( parameterTypeNode = new NonNullTypeNode((INullableTypeNode)parameterTypeNode); } + string parameterDescription = !string.IsNullOrWhiteSpace(paramMetadata?.Description) + ? paramMetadata.Description + : !string.IsNullOrWhiteSpace(definition.Description) + ? definition.Description + : $"parameters for {name.Value} stored-procedure"; + inputValues.Add( new( location: null, name: new(param), - description: definition.Description != null - ? new StringValueNode(definition.Description) - : new StringValueNode($"parameters for {name.Value} stored-procedure"), + description: new StringValueNode(parameterDescription), type: parameterTypeNode, defaultValue: defaultValueNode, directives: new List()) diff --git a/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderDescriptionMsSqlIntegrationTests.cs b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderDescriptionMsSqlIntegrationTests.cs new file mode 100644 index 0000000000..4f86ca3bb2 --- /dev/null +++ b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderDescriptionMsSqlIntegrationTests.cs @@ -0,0 +1,163 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Config.DatabasePrimitives; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.Configurations; +using Azure.DataApiBuilder.Service.GraphQLBuilder; +using Azure.DataApiBuilder.Service.Tests.SqlTests; +using HotChocolate.Language; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Azure.DataApiBuilder.Service.Tests.GraphQLBuilder.Sql +{ + /// + /// Integration tests that verify stored-procedure parameter descriptions flow + /// end-to-end through the full production pipeline: + /// config parameters.description + /// → SqlMetadataProvider.FillSchemaForStoredProcedureAsync (merges onto ParameterDefinition) + /// → GraphQLStoredProcedureBuilder.GenerateStoredProcedureSchema (reads description) + /// → GraphQL argument description + /// + [TestClass, TestCategory(TestCategory.MSSQL)] + public class StoredProcedureBuilderDescriptionMsSqlIntegrationTests : SqlTestBase + { + private static RuntimeConfig _baseConfig; + + [ClassInitialize] + public static async Task SetupAsync(TestContext context) + { + DatabaseEngine = TestCategory.MSSQL; + await InitializeTestFixture(); + _baseConfig = SqlTestHelper.SetupRuntimeConfig(); + } + + /// + /// Verifies that a description configured on a stored-procedure parameter in the + /// runtime config is propagated through the SQL metadata provider and reflected in + /// the generated GraphQL argument description. + /// + /// Uses the existing get_book_by_id stored procedure (defined in the MsSql + /// test schema) with a config-side description override on its id parameter. + /// + [TestMethod] + public async Task StoredProcedure_GraphQLArgDescription_UsesConfigDescriptionAfterMetadataInit() + { + const string entityName = "GetBookWithParamDesc"; + const string configDescription = "The unique identifier for the book (from config)"; + + Entity tamperedEntity = new( + Source: new( + "get_book_by_id", + EntitySourceType.StoredProcedure, + Parameters: new List + { + new() { Name = "id", Description = configDescription } + }, + KeyFields: null), + GraphQL: new(entityName, entityName, Enabled: true, Operation: GraphQLOperation.Query), + Rest: new(Enabled: false), + Fields: null, + Permissions: new[] + { + new EntityPermission( + Role: "anonymous", + Actions: new[] + { + new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) + }) + }, + Relationships: null, + Mappings: null, + Mcp: null); + + Dictionary entityMap = new() { [entityName] = tamperedEntity }; + RuntimeConfig tamperedConfig = _baseConfig with { Entities = new(entityMap) }; + RuntimeConfigProvider tamperedProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(tamperedConfig); + try + { + SetUpSQLMetadataProvider(tamperedProvider); + await _sqlMetadataProvider.InitializeAsync(); + + DatabaseObject dbObject = _sqlMetadataProvider.EntityToDatabaseObject[entityName]; + FieldDefinitionNode field = GraphQLStoredProcedureBuilder.GenerateStoredProcedureSchema( + name: new NameNode(entityName), + entity: tamperedEntity, + dbObject: dbObject); + + InputValueDefinitionNode idArg = field.Arguments.First(a => a.Name.Value == "id"); + Assert.IsNotNull(idArg.Description); + Assert.AreEqual(expected: configDescription, actual: idArg.Description!.Value); + } + finally + { + RuntimeConfigProvider sharedProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(_baseConfig); + SetUpSQLMetadataProvider(sharedProvider); + await _sqlMetadataProvider.InitializeAsync(); + } + } + + /// + /// Verifies that when no description is set on a stored-procedure parameter in the + /// runtime config the generated GraphQL argument falls back to the default + /// description text. Exercises the same full pipeline as the positive-case test. + /// + [TestMethod] + public async Task StoredProcedure_GraphQLArgDescription_FallsBackToDefaultTextWhenNoConfigDescription() + { + const string entityName = "GetBookNoDesc"; + + Entity tamperedEntity = new( + Source: new( + "get_book_by_id", + EntitySourceType.StoredProcedure, + Parameters: new List { new() { Name = "id" } }, + KeyFields: null), + GraphQL: new(entityName, entityName, Enabled: true, Operation: GraphQLOperation.Query), + Rest: new(Enabled: false), + Fields: null, + Permissions: new[] + { + new EntityPermission( + Role: "anonymous", + Actions: new[] + { + new EntityAction(Action: EntityActionOperation.Execute, Fields: null, Policy: null) + }) + }, + Relationships: null, + Mappings: null, + Mcp: null); + + Dictionary entityMap = new() { [entityName] = tamperedEntity }; + RuntimeConfig tamperedConfig = _baseConfig with { Entities = new(entityMap) }; + RuntimeConfigProvider tamperedProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(tamperedConfig); + try + { + SetUpSQLMetadataProvider(tamperedProvider); + await _sqlMetadataProvider.InitializeAsync(); + + DatabaseObject dbObject = _sqlMetadataProvider.EntityToDatabaseObject[entityName]; + FieldDefinitionNode field = GraphQLStoredProcedureBuilder.GenerateStoredProcedureSchema( + name: new NameNode(entityName), + entity: tamperedEntity, + dbObject: dbObject); + + InputValueDefinitionNode idArg = field.Arguments.First(a => a.Name.Value == "id"); + Assert.IsNotNull(idArg.Description); + Assert.AreEqual( + expected: $"parameters for {entityName} stored-procedure", + actual: idArg.Description!.Value); + } + finally + { + RuntimeConfigProvider sharedProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(_baseConfig); + SetUpSQLMetadataProvider(sharedProvider); + await _sqlMetadataProvider.InitializeAsync(); + } + } + } +} diff --git a/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs index afe9e2591b..f1b481acf4 100644 --- a/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs +++ b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs @@ -397,6 +397,142 @@ public void StoredProcedure_Description_UsesDefaultWhenEntityDescriptionIsNull() Assert.AreEqual(expectedDescription, field.Description?.Value); } + [TestMethod] + public void StoredProcedure_ParameterDescription_FallsBackToDefinitionDescriptionWhenNoConfigDescription() + { + const string parameterName = "title"; + const string definitionDescription = "Title description on the parameter definition"; + + DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spParamDescFallback") + { + SourceType = EntitySourceType.StoredProcedure, + StoredProcedureDefinition = new() + { + Parameters = new() + { + { parameterName, new() { SystemType = typeof(string), Description = definitionDescription } } + } + } + }; + spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) }); + + FieldDefinitionNode field = BuildSchemaAndGetExecuteField( + spDbObj: spDbObj, + configParameters: new List(), + graphQLTypeName: "SpParamDescFallbackType", + entityName: "SpParamDescFallback"); + + InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName); + Assert.IsNotNull(arg.Description); + Assert.AreEqual(definitionDescription, arg.Description!.Value); + } + + [TestMethod] + public void StoredProcedure_ParameterDescription_FallsBackToDefaultText() + { + const string parameterName = "title"; + const string graphQLTypeName = "SpParamDescDefaultTextType"; + const string entityName = "SpParamDescDefaultText"; + + DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spParamDescDefaultText") + { + SourceType = EntitySourceType.StoredProcedure, + StoredProcedureDefinition = new() + { + Parameters = new() { { parameterName, new() { SystemType = typeof(string) } } } + } + }; + spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) }); + + FieldDefinitionNode field = BuildSchemaAndGetExecuteField( + spDbObj: spDbObj, + configParameters: new List(), + graphQLTypeName: graphQLTypeName, + entityName: entityName); + + InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName); + Assert.IsNotNull(arg.Description); + Assert.AreEqual($"parameters for {graphQLTypeName} stored-procedure", arg.Description!.Value); + } + + [DataTestMethod] + [DataRow("", DisplayName = "Empty config description falls back to definition description")] + [DataRow(" ", DisplayName = "Whitespace config description falls back to definition description")] + public void StoredProcedure_ParameterDescription_WhitespaceConfigDescriptionFallsBackToDefinitionDescription(string whitespaceDescription) + { + const string parameterName = "title"; + const string definitionDescription = "Title description on the parameter definition"; + + DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spParamDescWhitespace") + { + SourceType = EntitySourceType.StoredProcedure, + StoredProcedureDefinition = new() + { + Parameters = new() + { + { parameterName, new() { SystemType = typeof(string), Description = definitionDescription } } + } + } + }; + spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) }); + + List configParameters = new() + { + new ParameterMetadata { Name = parameterName, Description = whitespaceDescription } + }; + + FieldDefinitionNode field = BuildSchemaAndGetExecuteField( + spDbObj: spDbObj, + configParameters: configParameters, + graphQLTypeName: "SpParamDescWhitespaceType", + entityName: "SpParamDescWhitespace"); + + InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName); + Assert.IsNotNull(arg.Description); + Assert.AreEqual(definitionDescription, arg.Description!.Value); + } + + [DataTestMethod] + [DataRow("", "", DisplayName = "Both empty — falls back to default text")] + [DataRow(" ", " ", DisplayName = "Both whitespace — falls back to default text")] + [DataRow("", " ", DisplayName = "Empty config, whitespace definition — falls back to default text")] + [DataRow(" ", "", DisplayName = "Whitespace config, empty definition — falls back to default text")] + public void StoredProcedure_ParameterDescription_BothWhitespaceFallsBackToDefaultText( + string whitespaceConfigDescription, string whitespaceDefinitionDescription) + { + const string parameterName = "title"; + const string graphQLTypeName = "SpParamDescBothWhitespaceType"; + const string entityName = "SpParamDescBothWhitespace"; + + DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spParamDescBothWhitespace") + { + SourceType = EntitySourceType.StoredProcedure, + StoredProcedureDefinition = new() + { + Parameters = new() + { + { parameterName, new() { SystemType = typeof(string), Description = whitespaceDefinitionDescription } } + } + } + }; + spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) }); + + List configParameters = new() + { + new ParameterMetadata { Name = parameterName, Description = whitespaceConfigDescription } + }; + + FieldDefinitionNode field = BuildSchemaAndGetExecuteField( + spDbObj: spDbObj, + configParameters: configParameters, + graphQLTypeName: graphQLTypeName, + entityName: entityName); + + InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName); + Assert.IsNotNull(arg.Description); + Assert.AreEqual($"parameters for {graphQLTypeName} stored-procedure", arg.Description!.Value); + } + /// /// Helper that builds a query schema for a stored-procedure entity and returns /// the generated execute* field so individual tests can assert on its argument