rmosolgo / graphql-ruby

Ruby implementation of GraphQL
http://graphql-ruby.org
MIT License
5.38k stars 1.39k forks source link

Non-nullable input field default value ignored when using operation variables #5127

Closed myronmarston closed 4 weeks ago

myronmarston commented 1 month ago

Describe the bug

Non-nullable input field default values work correctly when using inline argument values, but appear to be ignored when using operation variables.

Versions

graphql version: 2.3.18. rails (or other framework): N/A

GraphQL schema

input AddOperands {
  x: Int
  y: Int
  base: Int! = 10
}

type Query {
  add(operands: AddOperands): String!
}

GraphQL query

This query works as expected:

query {
  add(operands: {x: 3, y: 4})
}

But if I use a variable for operands, it unexpectedly fails:

query Add($operands: AddOperands) {
  add(operands: $operands)
}

Steps to reproduce

Put this into a script and run it:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "graphql", "2.3.18"
  gem "debug"
end

require "graphql"
require "time"
require "debug"

SCHEMA_STRING = <<~EOS
  input AddOperands {
    x: Int
    y: Int
    base: Int! = 10
  }

  type Query {
    add(operands: AddOperands): String!
  }
EOS

class Application
  def initialize
    @schema = ::GraphQL::Schema.from_definition(SCHEMA_STRING, default_resolve: self)
  end

  def call(parent_type, field, object, args, context)
    if field.graphql_name.start_with?("add")
      operands = args.fetch(:operands).to_h
      (operands.fetch(:x) + operands.fetch(:y)).to_s(operands.fetch(:base))
    else
      raise "Unknown field: #{field.inspect}"
    end
  end

  def execute_query(description, query_string, variables: {})
    query = ::GraphQL::Query.new(@schema, query_string, variables: variables)
    response = query.result

    puts <<~EOS
    #{"-" * 80}
    #{description}

    Query:
    #{query_string}

    Variables:
    #{::JSON.generate(variables)}

    Response:
    #{::JSON.pretty_generate(response.to_h)}
    EOS
  end
end

app = Application.new

app.execute_query("Using inline arguments", <<~EOS)
  query {
    add(operands: {x: 3, y: 4})
  }
EOS

app.execute_query("Using operation variables", <<~EOS, variables: {operands: {x: 3, y: 4}})
  query Add($operands: AddOperands) {
    add(operands: $operands)
  }
EOS

Expected behavior

I expect output like:

--------------------------------------------------------------------------------
Using inline arguments

Query:
query {
  add(operands: {x: 3, y: 4})
}

Variables:
{}

Response:
{
  "data": {
    "add": "7"
  }
}
--------------------------------------------------------------------------------
Using operation variables

Query:
query Add($operands: AddOperands) {
  add(operands: $operands)
}

Variables:
{"operands":{"x":3,"y":4}}

Response:
{
  "data": {
    "add": "7"
  }
}

Actual behavior

I instead get output like:

--------------------------------------------------------------------------------
Using inline arguments

Query:
query {
  add(operands: {x: 3, y: 4})
}

Variables:
{}

Response:
{
  "data": {
    "add": "7"
  }
}
--------------------------------------------------------------------------------
Using operation variables

Query:
query Add($operands: AddOperands) {
  add(operands: $operands)
}

Variables:
{"operands":{"x":3,"y":4}}

Response:
{
  "errors": [
    {
      "message": "Variable $operands of type AddOperands was provided invalid value for base (Expected value to not be null)",
      "locations": [
        {
          "line": 1,
          "column": 11
        }
      ],
      "extensions": {
        "value": {
          "x": 3,
          "y": 4
        },
        "problems": [
          {
            "path": [
              "base"
            ],
            "explanation": "Expected value to not be null"
          }
        ]
      }
    }
  ]
}

Additional context

The GraphQL spec covers this situation:

Input object fields may be required. Much like a field may have required arguments, an input object may have required fields. An input field is required if it has a non-null type and does not have a default value. Otherwise, the input object field is optional.

In this case, the field has a default value, so it should be treated as optional.

Note that if I make the base input field nullable, the default value is respected and the problem goes away. However, that allows a client to explicitly pass base: null which breaks the resolver implementation (it relies on base always having an integer value, using the default of 10 as needed). So I would like to keep the input field non-nullable.

On a side note: I expect the same behavior whether field arguments are provided inline in the query or provided via operation variables, and was quite surprised to learn of the difference here. My test suite tends to just use inline arguments for simplicity, which allowed this issue to get through to production.

Are there any cases where I should expect operation variables and inline arguments to behave differently?

rmosolgo commented 4 weeks ago

Hey, thanks for the detailed write-up and sorry it took me a while to write back. I've worked up a fix over in #5133.

Are there any cases where I should expect operation variables and inline arguments to behave differently?

I think the short answer is no -- they're supposed to work the same!