From f03ebb0a3e84bfd4071f2016457c3bcfc0c92c07 Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Mon, 11 May 2026 15:43:39 -0700 Subject: [PATCH 1/2] Mark required stored-procedure parameters as NON_NULL in GraphQL schema (#3501) --- .../GraphQLStoredProcedureBuilder.cs | 34 ++-- .../Sql/StoredProcedureBuilderTests.cs | 149 ++++++++++++++++++ 2 files changed, 174 insertions(+), 9 deletions(-) diff --git a/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs b/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs index 92d0298f10..ad3786fd36 100644 --- a/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs +++ b/src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs @@ -25,6 +25,12 @@ public static class GraphQLStoredProcedureBuilder /// It uses the parameters to build the arguments and returns a list /// of the StoredProcedure GraphQL object. /// + /// + /// Each input argument's GraphQL type is wrapped in when the + /// parameter is required, so introspection (String! vs String) reflects whether + /// the caller must supply a value. A parameter is treated as required unless the runtime + /// config explicitly sets required: false for it. + /// /// Name used for InputValueDefinition name. /// Entity's runtime config metadata. /// Stored procedure database schema metadata. @@ -55,16 +61,26 @@ public static FieldDefinitionNode GenerateStoredProcedureSchema( // Without database metadata, there is no way to know to cast 1 to a decimal versus an integer. IValueNode? defaultValueNode = null; - if (entity.Source.Parameters is not null) + ParameterMetadata? paramMetadata = entity.Source.Parameters? + .FirstOrDefault(p => p.Name == param); + + if (paramMetadata is not null && paramMetadata.Default is not null) { - ParameterMetadata? paramMetadata = entity.Source.Parameters - .FirstOrDefault(p => p.Name == param); + Tuple defaultGraphQLValue = ConvertValueToGraphQLType(paramMetadata.Default.ToString()!, parameterDefinition: spdef.Parameters[param]); + defaultValueNode = defaultGraphQLValue.Item2; + } - if (paramMetadata is not null && paramMetadata.Default is not null) - { - Tuple defaultGraphQLValue = ConvertValueToGraphQLType(paramMetadata.Default.ToString()!, parameterDefinition: spdef.Parameters[param]); - defaultValueNode = defaultGraphQLValue.Item2; - } + // Default to required so the schema doesn't silently mark a mandatory parameter as + // optional. T-SQL nullability does not indicate whether a caller must supply a value, + // so we only relax this when the config explicitly opts out. + bool isRequired = paramMetadata?.Required ?? true; + + ITypeNode parameterTypeNode = new NamedTypeNode( + SchemaConverter.GetGraphQLTypeFromSystemType(type: definition.SystemType)); + + if (isRequired) + { + parameterTypeNode = new NonNullTypeNode((INullableTypeNode)parameterTypeNode); } inputValues.Add( @@ -74,7 +90,7 @@ public static FieldDefinitionNode GenerateStoredProcedureSchema( description: definition.Description != null ? new StringValueNode(definition.Description) : new StringValueNode($"parameters for {name.Value} stored-procedure"), - type: new NamedTypeNode(SchemaConverter.GetGraphQLTypeFromSystemType(type: definition.SystemType)), + type: parameterTypeNode, defaultValue: defaultValueNode, directives: new List()) ); diff --git a/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs index b07ebcb083..8530a3d6b2 100644 --- a/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs +++ b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs @@ -249,5 +249,154 @@ public static void ValidateStoredProcedureSchema( string mismatchedTypeErrorMsg = $"Failure: Parameter '{parameterName}' is type '{actualGraphQLType}' but should be type '{expectedGraphQLType}'"; Assert.AreEqual(expected: expectedGraphQLType, actual: actualGraphQLType, message: mismatchedTypeErrorMsg); } + + /// + /// Validates that stored-procedure input arguments are emitted with the correct GraphQL + /// nullability based on the parameter's required flag: + /// + /// A parameter listed in config with required: true is wrapped in . + /// A parameter listed in config with required: false stays a nullable . + /// A parameter discovered from database metadata but not declared in config defaults to required and is wrapped in . + /// + /// + [DataTestMethod] + [DataRow("requiredParam", true, false, true, DisplayName = "Config required=true -> NonNull")] + [DataRow("optionalParam", true, true, false, DisplayName = "Config required=false -> nullable")] + [DataRow("dbOnlyParam", false, false, true, DisplayName = "Param not in config -> defaults to NonNull (required)")] + public void StoredProcedure_RequiredFlag_ProducesNonNullType( + string parameterName, + bool listInConfig, + bool configRequiredFalse, + bool expectsNonNull) + { + DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spReqTest") + { + SourceType = EntitySourceType.StoredProcedure, + StoredProcedureDefinition = new() + { + Parameters = new() { { parameterName, new() { SystemType = typeof(string) } } } + } + }; + spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) }); + + List configParameters = new(); + if (listInConfig) + { + configParameters.Add(new ParameterMetadata + { + Name = parameterName, + Required = !configRequiredFalse + }); + } + + FieldDefinitionNode field = BuildSchemaAndGetExecuteField( + spDbObj: spDbObj, + configParameters: configParameters, + graphQLTypeName: "SpReqTestType", + entityName: "SpReqTest"); + + InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName); + + if (expectsNonNull) + { + Assert.IsInstanceOfType(arg.Type, typeof(NonNullTypeNode), + $"Expected '{parameterName}' to be NonNullTypeNode but was '{arg.Type.GetType().Name}'."); + } + else + { + Assert.IsInstanceOfType(arg.Type, typeof(NamedTypeNode), + $"Expected '{parameterName}' to be nullable NamedTypeNode but was '{arg.Type.GetType().Name}'."); + } + + // Underlying named type should remain the same regardless of nullability wrapping. + Assert.AreEqual(STRING_TYPE, arg.Type.NamedType().Name.Value); + } + + /// + /// Validates that a required parameter with a config-supplied default value still emits a + /// NON_NULL input argument and preserves the default value on the GraphQL schema. + /// + [TestMethod] + public void StoredProcedure_RequiredWithDefault_KeepsDefaultValue() + { + const string parameterName = "title"; + + DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spReqDefault") + { + SourceType = EntitySourceType.StoredProcedure, + StoredProcedureDefinition = new() + { + Parameters = new() { { parameterName, new() { SystemType = typeof(string) } } } + } + }; + spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) }); + + List configParameters = new() + { + new ParameterMetadata + { + Name = parameterName, + Required = true, + Default = "Demo Title" + } + }; + + FieldDefinitionNode field = BuildSchemaAndGetExecuteField( + spDbObj: spDbObj, + configParameters: configParameters, + graphQLTypeName: "SpReqDefaultType", + entityName: "SpReqDefault"); + + InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName); + + Assert.IsInstanceOfType(arg.Type, typeof(NonNullTypeNode), "Required parameter should be NonNullTypeNode."); + Assert.IsNotNull(arg.DefaultValue, "Default value should be preserved on NON_NULL input argument."); + Assert.IsInstanceOfType(arg.DefaultValue, typeof(StringValueNode)); + Assert.AreEqual("Demo Title", ((StringValueNode)arg.DefaultValue!).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 + /// type nodes and default values. + /// + private static FieldDefinitionNode BuildSchemaAndGetExecuteField( + DatabaseObject spDbObj, + List configParameters, + string graphQLTypeName, + string entityName) + { + Entity spEntity = GraphQLTestHelpers.GenerateStoredProcedureEntity( + graphQLTypeName: graphQLTypeName, + graphQLOperation: GraphQLOperation.Query, + parameters: configParameters); + + ObjectTypeDefinitionNode objectType = CreateGraphQLTypeForEntity(spEntity, entityName, spDbObj); + + DocumentNode root = CreateGraphQLDocument(new Dictionary + { + { entityName, objectType } + }); + + Dictionary permissions = GraphQLTestHelpers.CreateStubEntityPermissionsMap( + entityNames: new[] { entityName }, + operations: new[] { EntityActionOperation.Execute }, + roles: SchemaConverterTests.GetRolesAllowedForEntity()); + + Dictionary entities = new() { { entityName, spEntity } }; + Dictionary entityToDatabaseName = new() { { entityName, DatabaseType.MSSQL } }; + Dictionary dbObjects = new() { { entityName, spDbObj } }; + + DocumentNode queryRoot = QueryBuilder.Build( + root, + entityToDatabaseName, + entities: new(entities), + inputTypes: null, + entityPermissionsMap: permissions, + dbObjects: dbObjects); + + ObjectTypeDefinitionNode queryNode = QueryBuilderTests.GetQueryNode(queryRoot); + return queryNode.Fields.First(f => f.Name.Value.StartsWith($"execute{graphQLTypeName}")); + } } } From f475cb084030987d3a4bf628a41b0be0608f5d80 Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Tue, 2 Jun 2026 16:30:55 -0700 Subject: [PATCH 2/2] Fixed failing tests. --- .../SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs | 6 ++---- .../SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs | 5 ++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs index 8cf55c247d..ad21509336 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs @@ -866,9 +866,7 @@ public async Task TestStoredProcedureQueryWithResultsContainingNull() /// /// Checks failure on providing arguments with no default in runtimeconfig. /// In this test, there is no default value for the argument 'id' in runtimeconfig, nor is it specified in the query. - /// Stored procedure expects id argument to be provided. - /// This test validates the "Development Mode" error message which denotes the - /// specific missing parameter and stored procedure name. + /// GraphQL validation should reject the request because required argument 'id' is missing. /// [TestMethod] public async Task TestStoredProcedureQueryWithNoDefaultInConfig() @@ -883,7 +881,7 @@ public async Task TestStoredProcedureQueryWithNoDefaultInConfig() JsonElement result = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); SqlTestHelper.TestForErrorInGraphQLResponse( response: result.ToString(), - message: "Procedure or function 'get_publisher_by_id' expects parameter '@id', which was not supplied."); + message: "The argument `id` is required."); } /// diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index 876424f0dd..8afcfd5ade 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -732,8 +732,7 @@ public async Task QueryAgainstSPWithOnlyTypenameInSelectionSet() /// /// Checks failure on providing arguments with no default in runtimeconfig. /// In this test, there is no default value for the argument 'id' in runtimeconfig, nor is it specified in the query. - /// Stored procedure expects id argument to be provided. - /// The expected error message contents align with the expected "Development" mode response. + /// GraphQL validation should reject the request because required argument 'id' is missing. /// [TestMethod] public async Task TestStoredProcedureQueryWithNoDefaultInConfig() @@ -746,7 +745,7 @@ public async Task TestStoredProcedureQueryWithNoDefaultInConfig() }"; JsonElement result = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); - SqlTestHelper.TestForErrorInGraphQLResponse(result.ToString(), message: "Procedure or function 'get_publisher_by_id' expects parameter '@id', which was not supplied."); + SqlTestHelper.TestForErrorInGraphQLResponse(result.ToString(), message: "The argument `id` is required."); } ///