Skip to content

Commit eccbdb3

Browse files
Merge remote-tracking branch 'refs/remotes/origin/dev/rubencerna/fix-cli-loglevel-flag' into dev/rubencerna/fix-cli-loglevel-flag
2 parents ab36b24 + c45b91d commit eccbdb3

4 files changed

Lines changed: 178 additions & 16 deletions

File tree

src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ public static class GraphQLStoredProcedureBuilder
2525
/// It uses the parameters to build the arguments and returns a list
2626
/// of the StoredProcedure GraphQL object.
2727
/// </summary>
28+
/// <remarks>
29+
/// Each input argument's GraphQL type is wrapped in <see cref="NonNullTypeNode"/> when the
30+
/// parameter is required, so introspection (<c>String!</c> vs <c>String</c>) reflects whether
31+
/// the caller must supply a value. A parameter is treated as required unless the runtime
32+
/// config explicitly sets <c>required: false</c> for it.
33+
/// </remarks>
2834
/// <param name="name">Name used for InputValueDefinition name.</param>
2935
/// <param name="entity">Entity's runtime config metadata.</param>
3036
/// <param name="dbObject">Stored procedure database schema metadata.</param>
@@ -55,16 +61,26 @@ public static FieldDefinitionNode GenerateStoredProcedureSchema(
5561
// Without database metadata, there is no way to know to cast 1 to a decimal versus an integer.
5662

5763
IValueNode? defaultValueNode = null;
58-
if (entity.Source.Parameters is not null)
64+
ParameterMetadata? paramMetadata = entity.Source.Parameters?
65+
.FirstOrDefault(p => p.Name == param);
66+
67+
if (paramMetadata is not null && paramMetadata.Default is not null)
5968
{
60-
ParameterMetadata? paramMetadata = entity.Source.Parameters
61-
.FirstOrDefault(p => p.Name == param);
69+
Tuple<string, IValueNode> defaultGraphQLValue = ConvertValueToGraphQLType(paramMetadata.Default.ToString()!, parameterDefinition: spdef.Parameters[param]);
70+
defaultValueNode = defaultGraphQLValue.Item2;
71+
}
6272

63-
if (paramMetadata is not null && paramMetadata.Default is not null)
64-
{
65-
Tuple<string, IValueNode> defaultGraphQLValue = ConvertValueToGraphQLType(paramMetadata.Default.ToString()!, parameterDefinition: spdef.Parameters[param]);
66-
defaultValueNode = defaultGraphQLValue.Item2;
67-
}
73+
// Default to required so the schema doesn't silently mark a mandatory parameter as
74+
// optional. T-SQL nullability does not indicate whether a caller must supply a value,
75+
// so we only relax this when the config explicitly opts out.
76+
bool isRequired = paramMetadata?.Required ?? true;
77+
78+
ITypeNode parameterTypeNode = new NamedTypeNode(
79+
SchemaConverter.GetGraphQLTypeFromSystemType(type: definition.SystemType));
80+
81+
if (isRequired)
82+
{
83+
parameterTypeNode = new NonNullTypeNode((INullableTypeNode)parameterTypeNode);
6884
}
6985

7086
inputValues.Add(
@@ -74,7 +90,7 @@ public static FieldDefinitionNode GenerateStoredProcedureSchema(
7490
description: definition.Description != null
7591
? new StringValueNode(definition.Description)
7692
: new StringValueNode($"parameters for {name.Value} stored-procedure"),
77-
type: new NamedTypeNode(SchemaConverter.GetGraphQLTypeFromSystemType(type: definition.SystemType)),
93+
type: parameterTypeNode,
7894
defaultValue: defaultValueNode,
7995
directives: new List<DirectiveNode>())
8096
);

src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,5 +249,154 @@ public static void ValidateStoredProcedureSchema(
249249
string mismatchedTypeErrorMsg = $"Failure: Parameter '{parameterName}' is type '{actualGraphQLType}' but should be type '{expectedGraphQLType}'";
250250
Assert.AreEqual(expected: expectedGraphQLType, actual: actualGraphQLType, message: mismatchedTypeErrorMsg);
251251
}
252+
253+
/// <summary>
254+
/// Validates that stored-procedure input arguments are emitted with the correct GraphQL
255+
/// nullability based on the parameter's required flag:
256+
/// <list type="bullet">
257+
/// <item><description>A parameter listed in config with <c>required: true</c> is wrapped in <see cref="NonNullTypeNode"/>.</description></item>
258+
/// <item><description>A parameter listed in config with <c>required: false</c> stays a nullable <see cref="NamedTypeNode"/>.</description></item>
259+
/// <item><description>A parameter discovered from database metadata but not declared in config defaults to required and is wrapped in <see cref="NonNullTypeNode"/>.</description></item>
260+
/// </list>
261+
/// </summary>
262+
[DataTestMethod]
263+
[DataRow("requiredParam", true, false, true, DisplayName = "Config required=true -> NonNull")]
264+
[DataRow("optionalParam", true, true, false, DisplayName = "Config required=false -> nullable")]
265+
[DataRow("dbOnlyParam", false, false, true, DisplayName = "Param not in config -> defaults to NonNull (required)")]
266+
public void StoredProcedure_RequiredFlag_ProducesNonNullType(
267+
string parameterName,
268+
bool listInConfig,
269+
bool configRequiredFalse,
270+
bool expectsNonNull)
271+
{
272+
DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spReqTest")
273+
{
274+
SourceType = EntitySourceType.StoredProcedure,
275+
StoredProcedureDefinition = new()
276+
{
277+
Parameters = new() { { parameterName, new() { SystemType = typeof(string) } } }
278+
}
279+
};
280+
spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) });
281+
282+
List<ParameterMetadata> configParameters = new();
283+
if (listInConfig)
284+
{
285+
configParameters.Add(new ParameterMetadata
286+
{
287+
Name = parameterName,
288+
Required = !configRequiredFalse
289+
});
290+
}
291+
292+
FieldDefinitionNode field = BuildSchemaAndGetExecuteField(
293+
spDbObj: spDbObj,
294+
configParameters: configParameters,
295+
graphQLTypeName: "SpReqTestType",
296+
entityName: "SpReqTest");
297+
298+
InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName);
299+
300+
if (expectsNonNull)
301+
{
302+
Assert.IsInstanceOfType(arg.Type, typeof(NonNullTypeNode),
303+
$"Expected '{parameterName}' to be NonNullTypeNode but was '{arg.Type.GetType().Name}'.");
304+
}
305+
else
306+
{
307+
Assert.IsInstanceOfType(arg.Type, typeof(NamedTypeNode),
308+
$"Expected '{parameterName}' to be nullable NamedTypeNode but was '{arg.Type.GetType().Name}'.");
309+
}
310+
311+
// Underlying named type should remain the same regardless of nullability wrapping.
312+
Assert.AreEqual(STRING_TYPE, arg.Type.NamedType().Name.Value);
313+
}
314+
315+
/// <summary>
316+
/// Validates that a required parameter with a config-supplied default value still emits a
317+
/// NON_NULL input argument and preserves the default value on the GraphQL schema.
318+
/// </summary>
319+
[TestMethod]
320+
public void StoredProcedure_RequiredWithDefault_KeepsDefaultValue()
321+
{
322+
const string parameterName = "title";
323+
324+
DatabaseObject spDbObj = new DatabaseStoredProcedure(schemaName: "dbo", tableName: "spReqDefault")
325+
{
326+
SourceType = EntitySourceType.StoredProcedure,
327+
StoredProcedureDefinition = new()
328+
{
329+
Parameters = new() { { parameterName, new() { SystemType = typeof(string) } } }
330+
}
331+
};
332+
spDbObj.SourceDefinition.Columns.TryAdd("col1", new() { SystemType = typeof(string) });
333+
334+
List<ParameterMetadata> configParameters = new()
335+
{
336+
new ParameterMetadata
337+
{
338+
Name = parameterName,
339+
Required = true,
340+
Default = "Demo Title"
341+
}
342+
};
343+
344+
FieldDefinitionNode field = BuildSchemaAndGetExecuteField(
345+
spDbObj: spDbObj,
346+
configParameters: configParameters,
347+
graphQLTypeName: "SpReqDefaultType",
348+
entityName: "SpReqDefault");
349+
350+
InputValueDefinitionNode arg = field.Arguments.First(a => a.Name.Value == parameterName);
351+
352+
Assert.IsInstanceOfType(arg.Type, typeof(NonNullTypeNode), "Required parameter should be NonNullTypeNode.");
353+
Assert.IsNotNull(arg.DefaultValue, "Default value should be preserved on NON_NULL input argument.");
354+
Assert.IsInstanceOfType(arg.DefaultValue, typeof(StringValueNode));
355+
Assert.AreEqual("Demo Title", ((StringValueNode)arg.DefaultValue!).Value);
356+
}
357+
358+
/// <summary>
359+
/// Helper that builds a query schema for a stored-procedure entity and returns
360+
/// the generated execute* field so individual tests can assert on its argument
361+
/// type nodes and default values.
362+
/// </summary>
363+
private static FieldDefinitionNode BuildSchemaAndGetExecuteField(
364+
DatabaseObject spDbObj,
365+
List<ParameterMetadata> configParameters,
366+
string graphQLTypeName,
367+
string entityName)
368+
{
369+
Entity spEntity = GraphQLTestHelpers.GenerateStoredProcedureEntity(
370+
graphQLTypeName: graphQLTypeName,
371+
graphQLOperation: GraphQLOperation.Query,
372+
parameters: configParameters);
373+
374+
ObjectTypeDefinitionNode objectType = CreateGraphQLTypeForEntity(spEntity, entityName, spDbObj);
375+
376+
DocumentNode root = CreateGraphQLDocument(new Dictionary<string, ObjectTypeDefinitionNode>
377+
{
378+
{ entityName, objectType }
379+
});
380+
381+
Dictionary<string, EntityMetadata> permissions = GraphQLTestHelpers.CreateStubEntityPermissionsMap(
382+
entityNames: new[] { entityName },
383+
operations: new[] { EntityActionOperation.Execute },
384+
roles: SchemaConverterTests.GetRolesAllowedForEntity());
385+
386+
Dictionary<string, Entity> entities = new() { { entityName, spEntity } };
387+
Dictionary<string, DatabaseType> entityToDatabaseName = new() { { entityName, DatabaseType.MSSQL } };
388+
Dictionary<string, DatabaseObject> dbObjects = new() { { entityName, spDbObj } };
389+
390+
DocumentNode queryRoot = QueryBuilder.Build(
391+
root,
392+
entityToDatabaseName,
393+
entities: new(entities),
394+
inputTypes: null,
395+
entityPermissionsMap: permissions,
396+
dbObjects: dbObjects);
397+
398+
ObjectTypeDefinitionNode queryNode = QueryBuilderTests.GetQueryNode(queryRoot);
399+
return queryNode.Fields.First(f => f.Name.Value.StartsWith($"execute{graphQLTypeName}"));
400+
}
252401
}
253402
}

src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -866,9 +866,7 @@ public async Task TestStoredProcedureQueryWithResultsContainingNull()
866866
/// <summary>
867867
/// Checks failure on providing arguments with no default in runtimeconfig.
868868
/// In this test, there is no default value for the argument 'id' in runtimeconfig, nor is it specified in the query.
869-
/// Stored procedure expects id argument to be provided.
870-
/// This test validates the "Development Mode" error message which denotes the
871-
/// specific missing parameter and stored procedure name.
869+
/// GraphQL validation should reject the request because required argument 'id' is missing.
872870
/// </summary>
873871
[TestMethod]
874872
public async Task TestStoredProcedureQueryWithNoDefaultInConfig()
@@ -883,7 +881,7 @@ public async Task TestStoredProcedureQueryWithNoDefaultInConfig()
883881
JsonElement result = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
884882
SqlTestHelper.TestForErrorInGraphQLResponse(
885883
response: result.ToString(),
886-
message: "Procedure or function 'get_publisher_by_id' expects parameter '@id', which was not supplied.");
884+
message: "The argument `id` is required.");
887885
}
888886

889887
/// <summary>

src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -732,8 +732,7 @@ public async Task QueryAgainstSPWithOnlyTypenameInSelectionSet()
732732
/// <summary>
733733
/// Checks failure on providing arguments with no default in runtimeconfig.
734734
/// In this test, there is no default value for the argument 'id' in runtimeconfig, nor is it specified in the query.
735-
/// Stored procedure expects id argument to be provided.
736-
/// The expected error message contents align with the expected "Development" mode response.
735+
/// GraphQL validation should reject the request because required argument 'id' is missing.
737736
/// </summary>
738737
[TestMethod]
739738
public async Task TestStoredProcedureQueryWithNoDefaultInConfig()
@@ -746,7 +745,7 @@ public async Task TestStoredProcedureQueryWithNoDefaultInConfig()
746745
}";
747746

748747
JsonElement result = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
749-
SqlTestHelper.TestForErrorInGraphQLResponse(result.ToString(), message: "Procedure or function 'get_publisher_by_id' expects parameter '@id', which was not supplied.");
748+
SqlTestHelper.TestForErrorInGraphQLResponse(result.ToString(), message: "The argument `id` is required.");
750749
}
751750

752751
/// <summary>

0 commit comments

Comments
 (0)