Skip to content

Mark required stored-procedure parameters as NON_NULL in GraphQL#3646

Open
anushakolan wants to merge 2 commits into
mainfrom
dev/anushakolan/required_params_graphql_3501
Open

Mark required stored-procedure parameters as NON_NULL in GraphQL#3646
anushakolan wants to merge 2 commits into
mainfrom
dev/anushakolan/required_params_graphql_3501

Conversation

@anushakolan
Copy link
Copy Markdown
Contributor

Why make this change?

Closes #3501.

The GraphQL schema generated for stored-procedure entities emitted every input argument as nullable (String), so introspection couldn't distinguish required parameters from optional ones. Clients calling those mutations had no way to know which arguments were mandatory until SQL Server failed the call at execution time.

What is this change?

In GenerateStoredProcedureSchema, each parameter's GraphQL type is now wrapped in NonNullTypeNode when the parameter is required, otherwise it stays a nullable NamedTypeNode. A parameter is treated as required unless the runtime config explicitly sets required: false for it. Names, value types, and the existing defaultValue handling are unchanged.

A required parameter with a config-supplied default still emits NON_NULL and keeps the default — that's valid GraphQL and matches client expectations.

The default-to-required behavior is intentional: T-SQL nullability (is_nullable) describes whether the parameter's type accepts NULL, not whether the caller must supply it. Defaulting to required produces a schema that's correct at execute time; users can opt out per-parameter via config when a procedure declares true defaults.

How was this tested?

  • Integration Tests
  • Unit Tests
  • Manual end-to-end test against a live SQL Server

Unit tests

Added to StoredProcedureBuilderTests:

  • StoredProcedure_RequiredFlag_ProducesNonNullType (3 data rows): required: trueNonNullTypeNode, required: false → nullable NamedTypeNode, parameter omitted from config → defaults to NonNullTypeNode.
  • StoredProcedure_RequiredWithDefault_KeepsDefaultValue: required parameter with a config default emits NonNullTypeNode and preserves the default value.

Manual end-to-end test

Three GraphQL mutations were configured against the existing dbo.insert_book(@title varchar(max), @publisher_id int) stored procedure. Each entity exercised a different config shape, and __schema introspection confirmed the input argument kinds:

GraphQL field Config shape title publisher_id
executeInsertBookPartialConfig only title listed (required: true) NON_NULL ✅ NON_NULL ✅ (DB-only → defaults to required)
executeInsertBookExplicitNotRequired title required: false, publisher_id required: true nullable ✅ NON_NULL ✅
executeInsertBookNoConfigParams no parameters block NON_NULL ✅ NON_NULL ✅ (silent → defaults to required)

All scenarios produced the expected GraphQL types.

Sample request

GraphQL introspection used to validate the schema:

{
  __schema {
    mutationType {
      fields {
        name
        args {
          name
          type { kind name ofType { kind name } }
        }
      }
    }
  }
}

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates stored-procedure GraphQL schema generation so input argument nullability reflects whether each parameter is required, enabling clients to identify mandatory vs optional inputs via introspection (fixing #3501).

Changes:

  • Wrap required stored-procedure parameter argument types in NonNullTypeNode, defaulting to required unless config explicitly sets required: false.
  • Preserve existing default-value behavior, including allowing defaults on NON_NULL arguments.
  • Add unit tests validating nullability behavior and default-value preservation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs Computes isRequired per parameter (defaulting to required) and conditionally wraps argument types in NonNullTypeNode.
src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs Adds unit tests asserting correct argument nullability and that defaults are preserved for required parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.x bug Something isn't working

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

[Bug]: Required params not marked in GraphQL

4 participants