Auto loaded objects inside an input object are called before 'ready?' in a mutation

[jfedgar]

Describe the bug

When I use an input object at the 'top level' in a mutation, any auto loaded objects within that mutation are loaded before ready? is called in the mutation.

However, if I autoload at the 'top level' of the mutation, ready? is called prior to attempting to load related data (as expected)

I would expect that in both cases ready? would be called prior to loading related data.

Versions

graphql version: 1.10.7 rails (or other framework): none other applicable versions (graphql-batch, etc)

GraphQL schema

schema:

class MyApp::Schema < ::GraphQL::Schema
  use GraphQL::Pagination::Connections
  
  query MyApp::QueryRoot
  mutation MyApp::MutationRoot
end

mutation root:

class MyApp::MutationRoot < ::GraphQL::Schema::Object
  field :create_account, mutation: MyApp::AccountCreateMutation
end

account_create_mutation:

  class MyApp::AccountCreateMutation < MyApp::BaseMutation
    null true

    argument :account, AccountCreateInputObject, required: true

    field :id, Types::UUID, null: true
    field :errors,  [Types::UserError], null: false

account_create_input_object:

 class MyApp::AccountCreateInputObject < Types::BaseInputObject
    argument :name, String, required: true
    argument :account_contact_ids, [ID], required: true, loads: 'AccountContactType'
end

The above code executes 'object_from_id' prior to executing 'ready?'.

However, if I forgo the AccountCreateInputObject and define the arguments at the 'top level' in the mutation, it calls ready? first, as expected based on https://graphql-ruby.org/mutations/mutation_authorization.html#checking-the-user-permissions

The below code properly authorizes the mutation via ready? prior to attempting to load the related data (AccountContact):

account_create_mutation:

  class MyApp::AccountCreateMutation < MyApp::BaseMutation
    null true

    argument :name, String, required: true
    argument :account_contact_ids, [ID], required: true, loads: 'AccountContactType'

    field :id, Types::UUID, null: true
    field :errors,  [Types::UserError], null: false

Steps to reproduce

Create an input object that auto loads related data using loads: in an argument. Use this input object in a mutation that utilizes the ready? method.

Expected behavior

ready? would be called "Before loading any data from the database".

Actual behavior

ready? is called after auto loading.

[rmosolgo]

Thanks for the detailed report! Yes, I agree it should work the way you suggest here.

I think the bug is because:

• For input objects, loads: is carried out during prepare:
  • https://github.com/rmosolgo/graphql-ruby/blob/3621e9d89819963e8b6e9704dde5fa033560d070/lib/graphql/schema/member/has_arguments.rb#L96-L103
  • called via https://github.com/rmosolgo/graphql-ruby/blob/3621e9d89819963e8b6e9704dde5fa033560d070/lib/graphql/execution/interpreter/runtime.rb#L171
• But, for mutations (and resolvers), loads: are carried out during resolve:
  • https://github.com/rmosolgo/graphql-ruby/blob/3621e9d89819963e8b6e9704dde5fa033560d070/lib/graphql/schema/resolver.rb#L73-L75
  • (which comes after the ready? check)

This is basically a historical problem: first I added loads: to mutation arguments only, but over time, it's been added to different kinds of arguments, and the implementation is a bit hacked together.

I'm not exactly sure the best solution. In general I can think of:

• Remove the ready? check from resolve and call it in a special case in the runtime. Icky because it adds another special case to the runtime.
• Change how mutations prepare their arguments. Icky for the same reason.
• Rework how ready? and loads: are implemented so that they represent general phases in execution. (We have the same problem with prepare:.) That would be awesome, but I don't have the time for it now, and it would be almost impossible to do while supporting the old runtime too. So it might have to come after GraphQL-Ruby 2.0.

Anyways, I don't have a quick fix on this one, but that's what's on my mind! 😅

[marten]

We're hitting this issue as well. One snag with it is that we have set up object_by_id to run through a Pundit policy scope (Pundit.policy_scope!(query_context[:api_session], type_name.safe_constantize).find_by(id: item_id)). That scope is dependent on your session, if you're not allowed to see something, you won't be able to find it.

This way I can either return nil, or raise ActiveRecord::NotFound. Neither option lets the mutation progress to the point where we can return an Unauthorized error message from ready? (via pundit_role from the Pro gem in our case).