Argument is nested input object with null as inner input value that aren't working as expected to trigger subscriptions

Question

Argument is nested input object with null as inner input value that aren't working as expected to trigger subscriptions

LanceRabbit opened this issue 3 months ago · 2 comments

Describe the bug

When a GraphQL subscription's argument is a nested input object, and an inner input field is defined as nullable but is provided with a null value, the subscription will fail with an error.

Versions

graphql version: 2.0.9
rails (or other framework): 6.1.7
ruby: 3.0.6

other applicable versions (graphql-batch, etc)

GraphQL schema

Include relevant types and fields (in Ruby is best, in GraphQL IDL is ok). Any custom extensions, etc?

  class InMemoryBackend < GraphQL::Subscriptions
    attr_reader :write_subscription_events, :execute_all_events

    def initialize(...)
      super
      @write_subscription_events = []
      @execute_all_events = []
    end

    def write_subscription(_query, events)
      @write_subscription_events.concat(events)
    end

    def execute_all(event, _object)
      @execute_all_events.push(event)
    end
  end

  class InnerInput < Types::BaseInputObject
    argument :first_name, String, required: false
    argument :last_name, String, required: false
  end

  class OuterInput < Types::BaseInputObject
    argument :inner_input, [InnerInput, { null: true }], required: false
  end

  class MySubscription < GraphQL::Schema::Subscription
    argument :input, OuterInput, required: false
    field :full_name, String
  end

  class SubscriptionType < GraphQL::Schema::Object
    field :my_subscription, resolver: MySubscription
  end

  class Schema < GraphQL::Schema
    subscription SubscriptionType
    use InMemoryBackend
  end
j

GraphQL query

Example GraphQL query and response (if query execution is involved)

subscription {
  mySubscription (input: { innerInput: null }) {
    fullName
  }
}

Steps to reproduce

Steps to reproduce the behavior

Schema.subscriptions.trigger(:mySubscription, { 'input' => { 'innerInput' => nil } }, nil)

Expected behavior

The subscription would update as expected.

Actual behavior

What specifically went wrong?

Place full backtrace here (if a Ruby exception is involved):

Click to view exception backtrace

     NoMethodError:
       undefined method `map' for nil:NilClass
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:301:in `normalize_arguments'
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:269:in `block in normalize_arguments'
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:255:in `each'
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:255:in `normalize_arguments'
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:269:in `block in normalize_arguments'
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:255:in `each'
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:255:in `normalize_arguments'
     # /Users/lance/.rvm/gems/ruby-3.0.6/gems/graphql-2.0.9/lib/graphql/subscriptions.rb:89:in `trigger'

Additional context

That's how we fixed it for our current version:

# frozen_string_literal: true

module NormalizeTriggerSubscriptionEnumValues
  # Recursively normalize `args` as belonging to `arg_owner`:
  # - convert symbols to strings,
  # - if needed, camelize the string (using {#normalize_name})
  # @param arg_owner [GraphQL::Field, GraphQL::BaseType]
  # @param args [Hash, Array, Any] some GraphQL input value to coerce as `arg_owner`
  # @return [Any] normalized arguments value
  def normalize_arguments(event_name, arg_owner, args, context)
    case arg_owner
    when GraphQL::Schema::Field, Class
      if arg_owner.is_a?(Class) && !arg_owner.kind.input_object?
        # it's a type, but not an input object
        return args
      end

      normalized_args = {}
      missing_arg_names = []
      args.each do |k, v|
        arg_name = k.to_s
        arg_defn = arg_owner.get_argument(arg_name, context)
        if arg_defn
          normalized_arg_name = arg_name
        else
          normalized_arg_name = normalize_name(arg_name)
          arg_defn = arg_owner.get_argument(normalized_arg_name, context)
        end

        if arg_defn
          normalized_arg_name = arg_defn.keyword.to_s if arg_defn.loads
          normalized = normalize_arguments(event_name, arg_defn.type, v, context)
          normalized_args[normalized_arg_name] = normalized
        else
          # Couldn't find a matching argument definition
          missing_arg_names << arg_name
        end
      end

      # Backfill default values so that trigger arguments
      # match query arguments.
      arg_owner.arguments(context).each do |_name, arg_defn|
        next unless arg_defn.default_value? && !normalized_args.key?(arg_defn.name)

        default_value = arg_defn.default_value
        # We don't have an underlying "object" here, so it can't call methods.
        # This is broken.
        normalized_args[arg_defn.name] = arg_defn.prepare_value(nil, default_value, context: context)
      end

      if missing_arg_names.any?
        arg_owner_name = if arg_owner.is_a?(GraphQL::Schema::Field)
                           arg_owner.path
                         elsif arg_owner.is_a?(Class)
                           arg_owner.graphql_name
                         else
                           arg_owner.to_s
                         end
        raise InvalidTriggerError,
              "Can't trigger Subscription.#{event_name}, received undefined arguments: #{missing_arg_names.join(', ')}. (Should match arguments of #{arg_owner_name}.)"
      end

      normalized_args
    when GraphQL::Schema::List
     # NOTE:
     # use try method to avoid NoMethodError occurred. 
      args&.map { |a| normalize_arguments(event_name, arg_owner.of_type, a, context) }
      # args.map { |a| normalize_arguments(event_name, arg_owner.of_type, a, context) }
    when GraphQL::Schema::NonNull
      normalize_arguments(event_name, arg_owner.of_type, args, context)
    else
      args
    end
  end
end

GraphQL::Subscriptions.prepend(NormalizeTriggerSubscriptionEnumValues)

and below is unit test

# frozen_string_literal: true

require 'spec_helper'

module SubscriptionEnum
  class InMemoryBackend < GraphQL::Subscriptions
    attr_reader :write_subscription_events, :execute_all_events

    def initialize(...)
      super
      @write_subscription_events = []
      @execute_all_events = []
    end

    def write_subscription(_query, events)
      @write_subscription_events.concat(events)
    end

    def execute_all(event, _object)
      @execute_all_events.push(event)
    end
  end

  class InnerInput < Types::BaseInputObject
    argument :first_name, String, required: false
    argument :last_name, String, required: false
  end

  class OuterInput < Types::BaseInputObject
    argument :inner_input, [InnerInput, { null: true }], required: false
  end

  class MySubscription < GraphQL::Schema::Subscription
    argument :input, OuterInput, required: false
    field :full_name, String
  end

  class SubscriptionType < GraphQL::Schema::Object
    field :my_subscription, resolver: MySubscription
  end

  class Schema < GraphQL::Schema
    subscription SubscriptionType
    use InMemoryBackend
  end
end

describe GraphQL::Subscriptions do
  describe 'enums' do
    let(:schema) { SubscriptionEnum::Schema }
    let(:implementation) { schema.subscriptions }
    let(:write_subscription_events) { implementation.write_subscription_events }
    let(:execute_all_events) { implementation.execute_all_events }

    describe 'pushing updates' do
      it 'sends updated data' do
        query_str = <<-GRAPHQL
          subscription ($input: OuterInput) {
            mySubscription (input: $input) {
              fullName
            }
          }
        GRAPHQL

        schema.execute(query_str, variables: { 'input' => { 'innerInput' => nil } })

        schema.subscriptions.trigger(:mySubscription, { 'input' => { 'innerInput' => nil } }, nil)

        assert_equal(':mySubscription:input:innerInput:', write_subscription_events[0].topic)
        assert_equal(':mySubscription:input:innerInput:', execute_all_events[0].topic)
      end
    end
  end
end

Answer 1 · 2024-10-02T13:39:22.000Z

Hey, thanks so much for the very detailed report and for sharing the workaround that worked for you!

I agree that this should work better and I agree that &. is an appropriate solution here. If you'd like to contribute it as a PR, I'd welcome it! Otherwise I'll try to add it soon.

Answer 2 · 2024-10-04T02:46:01.000Z

Hi @rmosolgo

I have committed a PR. Could you please review it? Thank you.