Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enum with custom values aren't working as expected to trigger subscriptions #4713

Closed
sobrinho opened this issue Nov 22, 2023 · 5 comments
Closed

Comments

@sobrinho
Copy link

Describe the bug

When a subscription is created, event.topic represents the enum value as the custom value.

When the subscription is triggered, event.topic represents the enum value as the given value, which doesn't map to the original topic therefore not encountering the subscription and not triggering it.

Looks like values are normalized to write the subscription but not for triggering it!

Versions

graphql version: 1.13.19 (legacy app but tried the latest version and seems to behave as the same)
rails: 6.1.7.5 (doesn't seem related to rails)

GraphQL schema

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

class MyEnum < GraphQL::Schema::Enum
  value "ONE", value: "one"
  value "TWO", value: "two"
end

class MySubscription < GraphQL::Schema::Subscription
  argument :my_enum, MyEnum, required: true

  field :my_enum, MyEnum, null: false

  def update(my_enum:)
    { my_enum: my_enum }
  end
end

class ApplicationSchema < GraphQL::Schema
  subscription MySubscription
end

GraphQL query

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

subscription {
  mySubscriptionSubscription(myEnum: ONE) {
    myEnum
  }
}

Steps to reproduce

# Doesn't work!
RootSchema.subscriptions.trigger(:my_subscription_subscription, my_enum: "one")
#=> Variable $blah of type blah! was provided invalid value for myEnum (Expected "one" to be one of: ONE, TWO)"

# Doesn't raise an error but doesn't match the event.topic given for write_subscription
RootSchema.subscriptions.trigger(:my_subscription_subscription, my_enum: "ONE")

Expected behavior

The subscription would update as expected (by having the same event.topic between write_subscription and execute_all).

Actual behavior

I added some binding.pry in our app into write_subscription and execute_all.

Calling event.topic returns like this for write_subscription:

event.topic
#=> :mySubscription:input:myEnum:one

And like this for execute_all:

event.topic
#=> :mySubscription:input:myEnum:ONE

Therefore they don't match and we can't find the subscription.

Additional context

Tried adding prepare to the argument but prepare is only called when writing subscription and not for triggering it, i.e.:

argument :my_enum, MyEnum, required: true, prepare: ->(e, ctx) { e ? e.downcase : e }

Not sure if that was intended, it's not our use case but we tried to use it to normalize the values across write and trigger as workaround.

PS: We are using a custom middleware to write/read/trigger/etc subscriptions but nothing in special there that could case event.topic to behave differently from the gem.

@sobrinho
Copy link
Author

Here's a reproducible spec:

# 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 MyEnumType < GraphQL::Schema::Enum
    value "ONE", value: "one"
    value "TWO", value: "two"
  end

  class MySubscription < GraphQL::Schema::Subscription
    argument :my_enum, MyEnumType, required: true
    field :my_enum, MyEnumType
  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
      focus
      it "sends updated data" do
        query_str = <<-GRAPHQL
          subscription ($myEnum: MyEnum!) {
            mySubscription (myEnum: $myEnum) {
              myEnum
            }
          }
        GRAPHQL

        schema.execute(query_str, variables: { "myEnum" => "ONE" })

        schema.subscriptions.trigger(:mySubscription, { "myEnum" => "ONE" }, nil)

        assert_equal(":mySubscription:myEnum:one", write_subscription_events[0].topic)
        assert_equal(":mySubscription:myEnum:one", execute_all_events[0].topic)
      end
    end
  end
end

@sobrinho
Copy link
Author

It fails with:

$ bundle exec rake test

# Running tests with run options --seed 49142:

F

Finished tests in 0.062282s, 16.0560 tests/s, 32.1120 assertions/s.


Failure:
GraphQL::Subscriptions::enums::pushing updates#test_0001_sends updated data [/Users/sobrinho/Developer/opensource/graphql-ruby/spec/graphql/subscriptions_enum_spec.rb:66]
Minitest::Assertion: --- expected
+++ actual
@@ -1 +1 @@
-":mySubscription:myEnum:one"
+":mySubscription:myEnum:ONE"


1 tests, 2 assertions, 1 failures, 0 errors, 0 skips
rake aborted!
Command failed with status (1)
/Users/sobrinho/.gem/ruby/2.7.6/gems/rake-13.1.0/exe/rake:27:in `<top (required)>'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/cli/exec.rb:58:in `load'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/cli/exec.rb:58:in `kernel_load'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/cli/exec.rb:23:in `run'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/cli.rb:492:in `exec'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/cli.rb:34:in `dispatch'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/cli.rb:28:in `start'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/exe/bundle:45:in `block in <top (required)>'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors'
/Users/sobrinho/.gem/ruby/2.7.6/gems/bundler-2.4.13/exe/bundle:33:in `<top (required)>'
/Users/sobrinho/.gem/ruby/2.7.6/bin/bundle:25:in `load'
/Users/sobrinho/.gem/ruby/2.7.6/bin/bundle:25:in `<main>'
Tasks: TOP => test
(See full trace by running task with --trace)

@sobrinho
Copy link
Author

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

 # Normalize enum values to its custom value when triggering subscriptions.
 #
 # In a nutshell, Event#topic normalizes "ENUM_VALUE" to "enum_value" when
 # writing a subscription while when triggering it doesn't.
 #
 # This monkey-patch will allow us to call either way:
 #
 #   RootSchema.subscriptions.trigger(:my_subscription, my_enum: "enum_value")
 #   RootSchema.subscriptions.trigger(:my_subscription, my_enum: "ENUM_VALUE")
 #
 # See https://github.com/rmosolgo/graphql-ruby/issues/4713
 module NormalizeTriggerSubscriptionEnumValues
   def stringify_args(arg_owner, args, context)
     result = super

     if arg_owner.is_a?(Class) && arg_owner < GraphQL::Schema::Enum
       arg_owner.coerce_isolated_input(result) || result
     else
       result
     end
   end
 end

 GraphQL::Subscriptions::Event.singleton_class.prepend(NormalizeTriggerSubscriptionEnumValues)

@rmosolgo
Copy link
Owner

Hey, sorry for the trouble and thanks for the great report and work-around! I bet a patch very similar to what you shared will fix this in the gem. I'll take a look soon, or feel free to submit a PR if you're interested 👍

rmosolgo added a commit that referenced this issue Nov 30, 2023
Fix triggering with custom enum value, fixes #4713
@sobrinho
Copy link
Author

Thanks @rmosolgo ! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants