apollo-federation-ruby icon indicating copy to clipboard operation
apollo-federation-ruby copied to clipboard

passing `@federation__shareable` to type PageInfo

Open matoni109 opened this issue 1 year ago • 17 comments

Hello Everyone 😀,

We are using type PageInfo in two subgraphs using federation 2.0

  • Currently we are getting the below error when trying to use connection_type
  • I've tried passing shareable: true also.
  • Any ideas ?
# query_type.rb
#... 
  field :users, UserType.connection_type, 'List all users.'  #, shareable: true

    def users
      User.all
    end
INVALID_FIELD_SHARING

Non-shareable field "PageInfo.endCursor" is resolved from multiple subgraphs: it is resolved from subgraphs "channels" and "users" and defined as non-shareable in all of them

INVALID_FIELD_SHARING

Non-shareable field "PageInfo.hasNextPage" is resolved from multiple subgraphs: it is resolved from subgraphs "channels" and "users" and defined as non-shareable in all of them
# base_connection.rb
module Types
  class BaseConnection < Types::BaseObject
    # add `nodes` and `pageInfo` fields, as well as `edge_type(...)` and `node_nullable(...)` overrides
    include GraphQL::Types::Relay::ConnectionBehaviors
  end
end

matoni109 avatar Aug 24 '22 06:08 matoni109

We faced this issue too and it was tricky to solve. We ended up monkeypatching GraphQL::Types::Relay::PageInfo like this:

# extending the Graphql Relay PageInfo class with federation abilities
class GraphQL::Types::Relay::PageInfo
  include ApolloFederation::Object
  include ApolloFederation::Field

  shareable if federation_2_condition?
end

and then requiring that file manually from our schema.rb file.

We had additional logic to decide if shareable should be called based on what federation version this subgraph was running. We're supporting something like 25 subgraphs, so not all of them could go to Federation 2 at the same time.

abachman avatar Aug 24 '22 15:08 abachman

Hi @matoni109, Thank you for submitting this. I'll take a closer look at it within the next day or so.

geshwho avatar Aug 24 '22 21:08 geshwho

Thanks @abachman & @geshwho,

If we get something up and running here I'll paste it into this thread

matoni109 avatar Aug 25 '22 00:08 matoni109

Sorry for the delay.. To me, this feels like something that the library should handle for you since connection_type is a graphql-ruby feature. I'm going to chat with some folks and see how we want to handle it.

geshwho avatar Aug 30 '22 12:08 geshwho

@geshwho our team came up with below in the mean time if that helps anyone else :

# app/graphql/types/base_connection.rb
module Types
  class BaseConnection < Types::BaseObject

    # add `nodes` and `pageInfo` fields, as well as `edge_type(...)` and `node_nullable(...)` overrides
    include GraphQL::Types::Relay::ConnectionBehaviors

    own_fields.delete("pageInfo")

    field :page_info, Types::PageInfo, 'Information to aid in pagination.', null: false
  end
end

# app/graphql/types/page_info.rb

module Types
  class PageInfo < Types::BaseObject
    shareable
    include GraphQL::Types::Relay::DefaultRelay

    field :has_next_page, Boolean, null: false,
      description: "When paginating forwards, are there more items?"

    field :has_previous_page, Boolean, null: false,
      description: "When paginating backwards, are there more items?"

    field :start_cursor, String, null: true,
      description: "When paginating backwards, the cursor to continue."

    field :end_cursor, String, null: true,
      description: "When paginating forwards, the cursor to continue."
  end
end

matoni109 avatar Aug 31 '22 23:08 matoni109

Hello, Shane from Apollo here. I am not familiar with the internals of this library but the original error is an issue where you might be running a combination of Fed 1 + Fed 2 or you are not using Fed 2 with the correct directives.

The short explanation is that in Fed 1 everything was considered "shareable" by default, but in Fed 2 it is now an opt-in model with the new @shareable directive.

https://www.apollographql.com/docs/federation/federation-2/new-in-federation-2#value-types

The quick fix is to add the @shareable directive on all subgraphs which will resolve the error, but this would require that all the subgraphs can also use Fed 2.

Because of this change and to remain backward compatible, if you are running Apollo Gateway v2, any Fed 1 subgraphs are auto-updated to add @shareable to their types, which will allow you to incremental update to Fed 2. There was a small bug around this so you might want to make sure you are running the latest versions of the gateway if you want to run in this mixed mode.

smyrick avatar Sep 13 '22 06:09 smyrick

Hello 👋 ,

We are also having this same problem. We have tested the approaches shared above and still can't find a workaround that works.

I'm commenting here to receive updates from this thread.

eliocapelati avatar Sep 14 '22 23:09 eliocapelati

Hello 👋 ,

We are also having this same problem. We have tested the approaches shared above and still can't find a workaround that works.

I'm commenting here to receive updates from this thread.

@eliocapelati we have above working in staging happy to work through it if you have a sample repo to play with

matoni109 avatar Sep 15 '22 00:09 matoni109

@geshwho i'm also in the mix of working on a PR of a fed2 version of the libs example which is using fed1

matoni109 avatar Sep 15 '22 00:09 matoni109

Many have already pointed out the workaround already. Here is a quick two liner for those curious. (Does the same thing as other solutions posted).

GraphQL::Types::Relay::PageInfo.include ApolloFederation::Object
GraphQL::Types::Relay::PageInfo.shareable

Note: This must be done in all subgraphs for it to fully work

skukx avatar Oct 13 '22 21:10 skukx

thanks @skukx :) below for those wondering where to put the fix ( that might have just been me .. )

# app/graphql/types/base_connection.rb

module Types
  class BaseConnection < Types::BaseObject

    # add `nodes` and `pageInfo` fields, as well as `edge_type(...)` and `node_nullable(...)` overrides
    include GraphQL::Types::Relay::ConnectionBehaviors
    GraphQL::Types::Relay::PageInfo.include ApolloFederation::Object
    GraphQL::Types::Relay::PageInfo.shareable

  end
end

matoni109 avatar Oct 13 '22 22:10 matoni109

You could technically place those lines anywhere. I chose to place it at the top of my graphql_schema.rb. You could also place it in an initializer as well.

skukx avatar Oct 13 '22 22:10 skukx

You could technically place those lines anywhere. I chose to place it at the top of my graphql_schema.rb. You could also place it in an initializer as well.

Thanks @skukx I'll put them there :)

matoni109 avatar Oct 13 '22 22:10 matoni109

@geshwho Any thoughts on a library-level fix and how to go about doing it? I'd be happy to contribute a PR but you mentioned giving it some thought awhile back so figured I should ask before I bumble on in 😄. Admittedly I haven't gone that deep at this point so maybe the answer will be self-evident if I do.

rosscooperman avatar Mar 01 '23 12:03 rosscooperman

Hey @rosscooperman sorry for that lack of response. A PR would be welcome if you have the space to put one together so we can take a look.

grxy avatar Mar 29 '23 18:03 grxy

Does anyone have the same bahavior that I have here?

GraphQL::Types::Relay::PageInfo.include ApolloFederation::Object
GraphQL::Types::Relay::PageInfo.shareable

when I define the pagination shareable directive in my schema file, this works great but if I change the file without restarting rails server, I got this error on my Apollo Gateway below

[1] Error: A valid schema couldn't be composed. The following composition errors were found:
[1] 	[SUBGRAPH NAME] The directive "@federation__shareable" can only be used once at this location.
[1]     at IntrospectAndCompose.createSupergraphFromSubgraphList (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/supergraphManagers/IntrospectAndCompose/index.js:73:19)
[1]     at IntrospectAndCompose.updateSupergraphSdl (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/supergraphManagers/IntrospectAndCompose/index.js:65:36)
[1]     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
[1]     at async IntrospectAndCompose.initialize (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/supergraphManagers/IntrospectAndCompose/index.js:28:36)
[1]     at async ApolloGateway.initializeSupergraphManager (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/index.js:299:28)
[1]     at async ApolloGateway.load (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/index.js:236:13)
[1]     at async SchemaManager.start (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/server/dist/cjs/utils/schemaManager.js:37:28)
[1]     at async ApolloServer._start (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/server/dist/cjs/ApolloServer.js:163:30)
[1]     at async ApolloServer.start (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/server/dist/cjs/ApolloServer.js:141:16)

If I look at what my Apollo Gateway got the schema to compose,

"""
Information about pagination in a connection.
"""
type PageInfo @federation__shareable @federation__shareable {

Duplicated @federation__shareable causing this behavior. Although this is not gonna happen after I restart rails server. I was wondering whether this behavior causes some other issue and also someone knows a work around :thinking:

Thanks!

ikuto0608 avatar Apr 12 '23 02:04 ikuto0608

@ikuto0608

There was a small bug around this so you might want to make sure you are running the latest versions of the gateway if you want to run in this mixed mode.

https://github.com/Gusto/apollo-federation-ruby/issues/207#issuecomment-1244949720

smyrick avatar Apr 13 '23 19:04 smyrick