graphql-ruby icon indicating copy to clipboard operation
graphql-ruby copied to clipboard

[ENTERPRISE] Changesets don't see new input objects for inputs of mutations

Open IvanChibisov opened this issue 2 years ago • 5 comments

Describe the bug

We want to update the type of the input argument for the mutation. Until then, the new type created for this mutation is not used anywhere. After adding it to the changeset, the old argument is removed and the new one is not applied, resulting in an error.

Can be solved by adding a new dummy mutation that uses this new InputObject on its own argument.

Versions

graphql version: graphql-enterprise (1.3.1) graphql (>= 1.13.0) graphql-pro (>= 1.24.0) graphql-pro (1.24.7) graphql (>= 1.7.5)

rails (or other framework): (7.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?

Initial goal -> update the argument type in the mutation's input.


module Mutations
  module Inputs
    # An old input object currently used by mutation input.
    class OldInputAttributes < Types::BaseInputObject
       argument :value, Integer, required: false 
    end

    # A new input object that hasn't been used anywhere before.
    class NewInputAttributes < Types::BaseInputObject
       argument :new_value1, String, required: false 
       argument :new_value2, Boolean, required: false
    end

    # An input object for mutation.
    class MutationExampleInputAttributes < Types::BaseInputObject
      argument :argument_type_should_change, Mutations::Inputs::OldInputAttributes, required: false
    end
  end

  class MutationExample < BaseMutation
    pundit_policy_class nil
    pundit_role nil

    argument :attributes, MutationExampleInputAttributes, required: true

    field :result, String, null: false

    def resolve(attributes:)
    end
  end
end

# The argument in the changeset
modifies Mutations::Inputs::MutationExampleInputAttributes do
    argument :argument_type_should_change, Mutations::Inputs::NewInputAttributes, required: true
  end
end

Steps to reproduce

  1. Create a mutation with an InputObject argument (MutationInputObject)
  2. Create another InputObject and add its type argument to MutationInputObject (OldInputObject)
  3. Create a new InputObject to be used in the changeset. (NewInputObject)
  4. Try changing the type of OldInputObject to NewInputObject in MutationInputObject in the changeset.

Expected behavior

The argument type is updated from OldInputObject to NewInputObject.

Actual behavior

The argument is removed from the schema.

{
  "message": "Variable $input of type MutationExampleInput! was provided invalid value for attributes.value (Field is not defined on MutationInputObject)",
}

Additional context

Adding NewInputObject to Schema#orphan_types does not help.

There is a way to avoid this problem. We can create a dummy mutation that does nothing but has an argument of type NewInputObject.

Maybe I am missing something and there is another way to make NewInputObject visible. Let me know if there is :)

IvanChibisov avatar Jul 19 '23 13:07 IvanChibisov

Hi, thanks for the detailed write-up! I agree it should work like you expect. I wrote up a replication script and it did work:

require "bundler/inline"

gemfile do
  gem "graphql", "2.0.24"
  gem "graphql-enterprise"
end


class MySchema < GraphQL::Schema
  class BaseArgument < GraphQL::Schema::Argument
    include GraphQL::Enterprise::Changeset::ArgumentIntegration
  end

  class BaseInputObject < GraphQL::Schema::InputObject
    argument_class BaseArgument
  end

  class BaseMutation < GraphQL::Schema::Mutation
    argument_class BaseArgument
  end

  class OldOperands < BaseInputObject
    argument :a, Integer
    argument :b, Integer
  end

  class NewOperands < BaseInputObject
    argument :new_a, Integer
    argument :new_b, Integer
  end

  class AddInputsAttributes < BaseInputObject
    argument :operands, OldOperands
  end

  class AddInputsNested < BaseMutation
    argument :attributes, AddInputsAttributes
    field :sum, Integer
    def resolve(attributes:)
      lhs = attributes[:operands][:a] || attributes[:operands][:new_a]
      rhs = attributes[:operands][:b] || attributes[:operands][:new_b]
      {
        sum: lhs + rhs
      }
    end
  end

  class Mutation < GraphQL::Schema::Object
    field :add_inputs_nested, mutation: AddInputsNested
  end

  class UpdateOperands < GraphQL::Enterprise::Changeset
    release "2023-06-01"

    modifies AddInputsAttributes do
      argument :operands, NewOperands
    end
  end

  mutation(Mutation)
  use GraphQL::Enterprise::Changeset::Release, changesets: [UpdateOperands]
end

pp MySchema.execute("mutation { addInputsNested(attributes: { operands: { a: 3, b: 4 } }) { sum }}").to_h
# {"data"=>{"addInputsNested"=>{"sum"=>7}}}

pp MySchema.execute("mutation { addInputsNested(attributes: { operands: { newA: 5, newB: 4 } }) { sum }}", context: { changeset_version: "2023-07-01" }).to_h
# {"data"=>{"addInputsNested"=>{"sum"=>9}}}

Could you please share the example query and variables that gave the result you posted above? Maybe that will help me refine my script to replicate this error or provide another clue!

rmosolgo avatar Jul 19 '23 21:07 rmosolgo

Thanks for the quick response! It's very useful to know that everything should work :) The difference in our setup is that we store the mutation, the input object, and the changeset in separate files. When I set everything up in one place everything works fine, but when I moved at least the changeset to another file, it doesn't work properly. This is probably due to the type lookup strategy in graphql or something else.

Not working example
# mutations/add_inputs_nested.rb
module Mutations
  module Inputs
    class OldOperands
      class Create < Types::BaseInputObject
        argument :a, Integer
        argument :b, Integer
      end
    end

    class NewOperands
      class Create < Types::BaseInputObject
        argument :new_a, Integer
        argument :new_b, Integer
      end
    end

    class AddInputsAttributes < Types::BaseInputObject
      argument :operands, OldOperands::Create
    end
  end

  class AddInputsNested < Mutations::BaseMutation
    argument :attributes, Inputs::AddInputsAttributes

    field :sum, Integer

    def resolve(attributes:)
      lhs = attributes[:operands][:a] || attributes[:operands][:new_a]
      rhs = attributes[:operands][:b] || attributes[:operands][:new_b]
      {
        sum: lhs + rhs,
      }
    end
  end
end

#Separate file changesets/update_operands.rb
module Changesets
  class UpdateOperands < BaseChangeset
    release "2023-07-20"

    modifies Mutations::Inputs::AddInputsAttributes do
      argument :operands, Mutations::Inputs::NewOperands::Create
    end
  end
end

pp MySchema.execute("mutation { addInputsNested(input: { attributes: { operands: { a: 3, b: 4 } }}) { sum }}").to_h
# {"data"=>{"addInputsNested"=>{"sum"=>7}}}

pp MySchema.execute("mutation { addInputsNested(input: { attributes: { operands: { newA: 5, newB: 4 } } }) { sum } }", context: { changeset_version: "2023-07-20" }).to_h
# {"errors"=>                                             
#  [{"message"=>                                         
#     "InputObject 'AddInputsAttributesInput' doesn't accept argument 'operands'",                                          
#    "locations"=>[{"line"=>1, "column"=>51}],
Working example
# mutations/add_inputs_nested.rb
module Mutations
  module Inputs
    class OldOperands
      class Create < Types::BaseInputObject
        argument :a, Integer
        argument :b, Integer
      end
    end

    class NewOperands
      class Create < Types::BaseInputObject
        argument :new_a, Integer
        argument :new_b, Integer
      end
    end

    class AddInputsAttributes < Types::BaseInputObject
      argument :operands, OldOperands::Create
    end
  end

  class AddInputsNested < Mutations::BaseMutation
    argument :attributes, Inputs::AddInputsAttributes

    field :sum, Integer

    def resolve(attributes:)
      lhs = attributes[:operands][:a] || attributes[:operands][:new_a]
      rhs = attributes[:operands][:b] || attributes[:operands][:new_b]
      {
        sum: lhs + rhs,
      }
    end
  end
end

#The same file
module Changesets
  class UpdateOperands < BaseChangeset
    release "2023-07-20"

    modifies Mutations::Inputs::AddInputsAttributes do
      argument :operands, Mutations::Inputs::NewOperands::Create
    end
  end
end

pp MySchema.execute("mutation { addInputsNested(input: { attributes: { operands: { a: 3, b: 4 } }}) { sum }}").to_h
# {"data"=>{"addInputsNested"=>{"sum"=>7}}}

pp MySchema.execute("mutation { addInputsNested(input: { attributes: { operands: { newA: 5, newB: 4 } } }) { sum } }", context: { changeset_version: "2023-07-20" }).to_h
# {"data"=>{"addInputsNested"=>{"sum"=>9}}}

IvanChibisov avatar Jul 20 '23 10:07 IvanChibisov

@rmosolgo Are there any updates here?

IvanChibisov avatar Feb 19 '24 12:02 IvanChibisov

Hey, sorry this slipped off my radar 😞 Thanks for sharing those further details. I just now created a stand-alone replication based on your code above:

input_object_test.rb

# mutations/add_inputs_nested.rb
require "bundler/inline"

gemfile do
  gem "graphql", "2.2.8"
  gem "graphql-enterprise"
end

module Changesets
  autoload :UpdateOperands, "./input_object_test2.rb"
end

module Types
  class BaseArgument < GraphQL::Schema::Argument
    include GraphQL::Enterprise::Changeset::ArgumentIntegration
  end
  class BaseInputObject < GraphQL::Schema::InputObject
    argument_class(BaseArgument)
  end
end

module Mutations
  class BaseMutation < GraphQL::Schema::RelayClassicMutation
    argument_class Types::BaseArgument
  end

  module Inputs
    class OldOperands
      class Create < Types::BaseInputObject
        argument :a, Integer
        argument :b, Integer
      end
    end

    class NewOperands
      class Create < Types::BaseInputObject
        argument :new_a, Integer
        argument :new_b, Integer
      end
    end

    class AddInputsAttributes < Types::BaseInputObject
      argument :operands, OldOperands::Create
    end
  end

  class AddInputsNested < Mutations::BaseMutation
    argument :attributes, Inputs::AddInputsAttributes

    field :sum, Integer

    def resolve(attributes:)
      lhs = attributes[:operands][:a] || attributes[:operands][:new_a]
      rhs = attributes[:operands][:b] || attributes[:operands][:new_b]
      {
        sum: lhs + rhs,
      }
    end
  end
end

class MySchema < GraphQL::Schema
  class Mutation < GraphQL::Schema::Object
    field :add_inputs_nested, mutation: Mutations::AddInputsNested
  end
  mutation(Mutation)
  use GraphQL::Enterprise::Changeset::Release, changesets: [
    Changesets::UpdateOperands,
  ]
end

pp MySchema.execute("mutation { addInputsNested(input: { attributes: { operands: { a: 3, b: 4 } }}) { sum }}").to_h
# {"data"=>{"addInputsNested"=>{"sum"=>7}}}

pp MySchema.execute("mutation { addInputsNested(input: { attributes: { operands: { newA: 5, newB: 4 } } }) { sum } }", context: { changeset_version: "2023-07-20" }).to_h
# {"errors"=>
#  [{"message"=>
#     "InputObject 'AddInputsAttributesInput' doesn't accept argument 'operands'",
#    "locations"=>[{"line"=>1, "column"=>51}],
input_object_test2.rb

module Changesets
  class BaseChangeset < GraphQL::Enterprise::Changeset
  end

  class UpdateOperands < BaseChangeset
    release "2023-07-20"

    modifies Mutations::Inputs::AddInputsAttributes do
      argument :operands, Mutations::Inputs::NewOperands::Create
    end
  end
end

I'll follow up here when I have a fix ready!

rmosolgo avatar Feb 19 '24 16:02 rmosolgo

Ok, I did some debugging and found the cause for this strange issue. The underlying issue whether modifies ... is called before or after mutation(...) is configured.

When mutation(...) is configured in the schema, the schema traverses the given type and makes a registry of all types, fields, and arguments.

When modifies ... is called, new types (and fields, and arguments) maybe be added to the schema -- but schema's registry doesn't contain them. (The types, eg AddInputsAttributes, do have references to the definitions, but the top-level registry in the schema is not updated.)

This is a problem because, at runtime, the schema's registry is checked to make sure that the currently-used types are available to the current query. If a type isn't in that registry, then it isn't available to the current query 😖

The easiest fix is to reorder Changeset::Release and mutation(...):

-   mutation(Mutation)
    use GraphQL::Enterprise::Changeset::Release, changesets: [
      Changesets::UpdateOperands,
    ]
+   mutation(Mutation)

That way, the schema can find all types used in the schema, including those added in changesets. Does a change like that fix it for you?

I've added a note about this dependency to the docs in b0d289ebd.

If a fix like that doesn't work in your case, please let me know why not, and what happened when you tried it. It may be possible to re-enter the schema type traversal during modifies ..., but I'd like to avoid that. It will be slow to re-traverse things for each call to modifies ...!

rmosolgo avatar Feb 19 '24 17:02 rmosolgo

Moving mutation() under Changeset::Release breaks the schema because the changeset doesn't see AddInputsAttributes:

uninitialized constant Mutations::Inputs::AddInputsAttributes (NameError)

    modifies Mutations::Inputs::AddInputsAttributes do
                              ^^^^^^^^^^^^^^^^^^^^^
Did you mean?  Mutations::AddInputsNested

Our setup differs by placing the code: in one-two file or in many. I have 4 files for this setup:

  • app/graphql/changesets/update_operands.rb
  • app/graphql/my_schema.rb
  • app/graphql/mutations/add_inputs_nested.rb
  • app/graphql/types/mutation_type.rb - define all mutations

Please try splitting your replication into multiple files :)

IvanChibisov avatar Feb 20 '24 07:02 IvanChibisov

Ok, this sounds like an autoloading issue. What file is Mutations::Inputs::AddInputsAttributes in? I think Zeitwerk would check app/graphql/mutations.rb, app/graphql/mutations/inputs.rb, and app/graphql/mutations/inputs/add_inputs_attributes.rb, but if it isn't defined in one of those files, then Zeitwerk can't autoload the constant.

If you can't use Zeitwerk's expected file structure (doc), I think you could give it a clue by referencing the constant whose file does define Mutations::Inputs::AddInputsAttributes. For example, if that constant was defined in app/graphql/mutations/add_inputs_nested.rb, you could reference it like this:

Mutations::AddInputsNested # Reference this type so that the `Inputs` module below will be found

modifies Mutations::Inputs::AddInputsAttributes do
  # ... 
end 

That would cause Zeitwerk to load app/graphql/mutations/add_inputs_nested.rb before continuing to the modifies ... call.

Alternatively, you could restructure your module namespacing to match Zeitwerk's expectations, for example, moving AddInputsAttributes to Mutations::AddInputsNested::Inputs::AddInputsAttributes. I think that would tell Zeitwerk to try app/graphql/mutations/add_inputs_nested.rb to find that constant.

How about giving one of those a try?

rmosolgo avatar Feb 20 '24 14:02 rmosolgo

Thanks for your answer, I missed that this might be related to Zeitwerk. I confirm that the original issue is resolved by moving mutation(Mutation) above use GraphQL::Enterprise::Changeset::Release. We can probably close this issue.

IvanChibisov avatar Feb 20 '24 16:02 IvanChibisov

Ok, sounds good. Let know if you run into any more trouble on the GraphQL side of things! Sorry this order dependency wasn't documented before and thanks for helping me get to the bottom of it.

rmosolgo avatar Feb 20 '24 20:02 rmosolgo