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

How to migrate middleware to class-based API? #2479

Closed
rtymchyk opened this issue Sep 15, 2019 · 7 comments
Closed

How to migrate middleware to class-based API? #2479

rtymchyk opened this issue Sep 15, 2019 · 7 comments

Comments

@rtymchyk
Copy link
Contributor

rtymchyk commented Sep 15, 2019

I have a middleware that wraps our Rails models/objects in a presenter, to expose presentational attributes of those models/objects to the GraphQL layer (in addition to any object/model attributes).

class PresenterMiddleware
  def self.call(parent_type, parent_object, field_definition, field_args, ctx)
    if parent_object.respond_to?(:as_presenter) && parent_object.as_presenter
      parent_object = parent_object.as_presenter
    end

    yield [parent_type, parent_object, field_definition, field_args, ctx]
  end
end

For types defined in the new class-based API, parent_object no longer corresponds to the actual object. It's the actual GraphQL layer object, such as UserType.

For types defined in the old .define API, parent_object always corresponds to the actual object, such as the User model.

What is the proper way to migrate this if I continue using a middleware? I am also open to other ideas that don't involve a middleware. I'm not sure if middlewares were public APIs before and aren't anymore, or they were never public in the first place.

EDIT: I think #73 (comment) was the inspiration for the middleware solution (I did not write the code above), but that was in 2015, and next_middleware is deprecated as well.

@swalkinshaw
Copy link
Collaborator

Field extensions would be the appropriate replacement for this. You can use the extension's resolve method to call as_presenter and yield that object.

@rtymchyk
Copy link
Contributor Author

rtymchyk commented Sep 15, 2019

@swalkinshaw I'm a worried about new presenter creation for every single field. I suppose I could memoize the it between multiple fields, but that increases the complexity.

The approach with/like the middleware provides seems more appropriate. Rather than do it at field level, just pass the presenter down the tree for all fields to use.

EDIT:
Actually I might be wrong. Had to dig into the source code to understand a bit more... It looks like a middleware executes per field, not at 'object' level as I originally thought (which is now obvious looking at the call signature of the middleware), so the suggestion to use field extension would be an almost identical implementation.

@rtymchyk
Copy link
Contributor Author

rtymchyk commented Sep 15, 2019

So I gave it a shot with field extensions:

# frozen_string_literal: true
class PresenterExtension < GraphQL::Schema::FieldExtension
  def resolve(**args)
    object = args.delete :object
    arguments = args.delete :arguments

    # Never works because object is always a graph object, not the true underlying rails model 
    object = object.as_presenter if object.respond_to?(:as_presenter) && object&.as_presenter

    yield(object, arguments, args)
  end
end
# frozen_string_literal: true
class BaseField < GraphQL::Schema::Field
  include ApolloFederation::Field

  def initialize(*args, **kwargs, &block)
    kwargs[:extensions] = [PresenterExtension]
    super(*args, **kwargs, &block)
  end
end

Running into the same problem. object here is UserType. I can see that I can access User by doing object.object based on API docs but it's a read-only properly.

What is the proper way to modify the underlying object? Or is there a way to shallow copy the wrapper and define the object?

@rmosolgo
Copy link
Owner

rmosolgo commented Sep 16, 2019

It looks like you're trying to modify the object that owns the field before a field is resolved on that object. But what if you went about it the other way, and tried to modify an object that was returned from a field? That is, after an field resolver returns an instance of ::User, you could call .to_presenter on before it's handed over to GraphQL-Ruby and wrapped in UserType. One advantage of that is that you only call .to_presenter once per object.

I think an extension like this would work:

class PresenterExtension < GraphQL::Schema::FieldExtension
  def after_resolve(value:, **rest)
    if value.respond_to?(:to_presenter)
      value.to_presenter
    else 
      value 
    end 
  end 
end

That way, any time a field returned a Ruby object with a .to_presenter method, that method would be called. What do you think?

@rmosolgo rmosolgo reopened this Sep 16, 2019
@rmosolgo
Copy link
Owner

(Sorry, bad at buttons)

@rtymchyk
Copy link
Contributor Author

@rmosolgo That makes sense. It should be decorating everything, just on the output not the input. I'll give this a shot, thanks for your comment :)

@rtymchyk
Copy link
Contributor Author

rtymchyk commented Sep 16, 2019

Unfortunately this breaks down in the case where I have an old type defined with .define with a field referencing a new type defined with class API. The extension won't execute there, and the middleware will run into the issue described in my first post, so I think I'll need to migrate over completely to the class API to guarantee that this works globally.

Anyway, the solution above works well assuming extensions are available 😄 Can probably close this.

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

3 participants