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

federation: queries for interface types require knowledge of concrete implementations #336

Closed
lennyburdette opened this issue Jun 14, 2019 · 13 comments

Comments

@lennyburdette
Copy link
Contributor

Version: @apollo/gateway@0.6.6
Reproduction: https:/apollographql/federation-demo/compare/master...lennyburdette:interfaces?expand=1
Related issues: apollographql/apollo-server#2848

I changed the federation-demo to use an interface for the Product type. I could get it to work, but not without some hacks that won't work in the real world.

The Reviews service has to know about Product implementations (Book and Furniture), even though it only knows about Product.id.

For this query:

{
  me {
    reviews {
      product {
        id # doesn't work without https:/apollographql/apollo-server/pull/2848
	name
        price
        inStock
        ... on Furniture {
          shippingEstimate
        }
      }
    }
  }
}

I see this in the query plan:

Fetch(service: "reviews") {
  {
    ... on User {
      __typename
      id
    }
  } =>
  {
    ... on User {
      reviews {
        product {
          __typename
          id
          ... on Book {
            __typename
            id
          }
          ... on Furniture {
            __typename
            id
          }
        }
      }
    }
  }
},

Because the Gateway fetches __typename, I have to implement Product.__resolveType (in a gross way.)

This is often very hard and sometimes impossible, depending on how much information the service stores. Reviews doesn't know anything about Products other than the id, so there's no way for it to return the correct __typename.

Ideally, the query to Reviews would be:

{
  ... on User {
    reviews {
      body
      product {
        # ideally we wouldn't subselect __typename here so that the Reviews service
        # doesn't have to care about interface implementations
        id
      }
    }
  }
}

This is less of a problem for the Inventory service because:

  • We use the ids from Reviews to call the Products service, which can return __typenames.
  • We fetch Inventory fields using references ({ __typename: "Furniture", id: "1" }), so Inventory can just return the given __typename.

(There's also some unfortunate redundancy in Reviews and Inventory—we have to repeat __resolveReference functions for each interface implementation, but I can solve that on my side.)

@jbaxleyiii
Copy link
Contributor

jbaxleyiii commented Jun 14, 2019

@lennyburdette can you change your reviews service to not know about the interface:

  extend type Book @key(fields: "id") {
    id: ID! @external
    reviews: [Review!]
  }
  extend type Furniture @key(fields: "id") {
    id: ID! @external
    reviews: [Review!]
  }

@lennyburdette
Copy link
Contributor Author

@jbaxleyiii Can I? I left the Review type alone—it still references the Product interface:

  type Review @key(fields: "id") {
    id: ID!
    body: String
    author: User @provides(fields: "username")
    product: Product
  }

@rtymchyk
Copy link

rtymchyk commented Sep 18, 2019

This seems like quite a blocker for using interfaces. I'm running into the same issue where defining the type resolver for an extended interface is just impossible from the that service (the review service in the example PR above). The only work-around seems to be to reverse where the relationship is defined, but that comes with other disadvantages.

@jhampton
Copy link
Contributor

Has this been tested in the latest version of the Gateway?

@mmuth
Copy link

mmuth commented Jul 3, 2020

Hi there, I am just struggling with the very same scenario.... I am just trying to reference an object that implements an interface and I'd prefer to just use that interface and only use the ID for referencing. In my case I could reluctantly even add knowledge of all implementations by listing the concrete types with @extends. Tried this but couldn't get it work...

The Gateway requests the original Product service via the _entities query using the interface like query($representations:[_Any!]!){_entities(representations:$representations){...on Product{niceName}}}
(it does not use the real implementing type like ... on Book or ... on Furniture here even though the type was previously resolved in the Reviews service). This could possibly even work, but the Gateway also seems to not pass any representations and so the query result is always empty.

@jhampton I tested it with the latest Gateway (0.17.0) and I'd be very happy about any ideas or workarounds :)

@mmuth
Copy link

mmuth commented Sep 3, 2020

Any updates on this?

@abernix abernix transferred this issue from apollographql/apollo-server Jan 15, 2021
@quant-daddy
Copy link

Any updates on this?

@wabrit
Copy link

wabrit commented Jan 12, 2022

I've hit this same problem; this (decoupling subgraphs via interfaces) is quite a common pattern in monolithic graphs,

The only workaround I can see is

  • to redeclare the interface in the dependent graph and extend all of its known implementations
  • have the dependent graph figure out the implementing type of an interface instance in order to construct the "reference stub" to return to the gateway

which is less than ideal because it couples the dependent graph too tightly to knowledge of what types implement the interface (leading to brittleness) and it forces an extra round trip on the part of the dependent graph to do the type resolution itself (assuming its even practical for it to do this).

I feel what is needed is for a subgraph to be able to return some kind of pseudo-reference to an interface instance { __interfacename: 'X', id: <id> }, and for the "owning" subgraph of that interface to be able to declare a reference resolver on the interface X that, given an id, can return the appropriate concrete instance of some type implementing X.

@cli00004
Copy link

cli00004 commented May 6, 2022

Any updates on this? I'm now stuck on this very same issue

@pcmanus
Copy link
Contributor

pcmanus commented May 9, 2022

Sorry for the very long silence on this.

Fixing this is officially on the roadmap (it's the "Entity interfaces can be spread ..." bullet point) and fwiw I'm personally very keen on prioritizing this I do think it's an important part of improving separation of concerns. That said, and to be transparent, we're still in the process of prioritizing what we (at Apollo) want to do next and it's not a completely trivial issue, so a proper fix may still be a couple months away.

What I can share is that the way we're currently thinking of fixing this is essentially by allowing a subgraph to "implement" an interface as an object type locally. More concretely, and to take the Product/Review example from the description of this ticket, the plan is to allow to write (only including the relevant parts):

# Subgraph 'products'
interface Product @key(fields: "id") {
  id: ID!
  name: String
  price: Int
}

type Book implements Product @key(fields: "id") {
  id: ID!
  name: String
  price: Int
  pages: Int
}

type Furniture implements Product @key(fields: "id") {
  id: ID!
  name: String
  price: Int
  weight: Int
}
# Subgraph 'reviews'
type Review @key(fields: "id") {
  id: ID!
  body: String
  author: User
  product: Product
}

type Product @interfaceObject @key(fields: "id") {
  id: ID!
  reviews: [Review!]
}

The idea being that the "reviews" subgraph don't want to have anything to do with Product concrete implementations, it wants Product to be completely abstracted, and to do so it declares Product as an object type (instead of an interface) and mark it as an @interfaceObject. The later tells federation (and other readers) that Product is really an interface in the federated graph, but that this particular subgraph don't want to deal with the implementations.

The motivation for doing it this way is that this allows the "reviews" subgraph to both abstract Product but also be a completely normal graphQL schema. In particular, "reviews" needs nothing special and can be tested in isolation with no magic needed. It is the gateway/router that will be in charge, during queries, to do the proper translations (around __typename in particular) as needed.

I'll also note that another part of this is the support of @key on interface (the @key on Product in the "products" subgraph). This is what will allow, when you have a Review, to get its product fields. This part will require a smal change for subgraph servers as they'll need to start supporting __resolveReference on interfaces, but that change should hopefully be relatively simple to implement.

There is more to be said on this, and I believe this proposal extends fairly naturally to more specific/complex use cases, but that can be expanded on later.

In any case, the reason I'm sharing this is also to solicit early feedback.

@pfyod
Copy link

pfyod commented Oct 25, 2022

@pcmanus Has there been any progress here?

@pcmanus
Copy link
Contributor

pcmanus commented Nov 24, 2022

@pcmanus Has there been any progress here?

Sorry again for the spotty updates.

But we have starting working more concretely on the proposal I've laid out in my previous comment. And I've just created a dedicated issue for it: #2277. It's essentially still what I mention above, but I provide a bunch more details in that issue on the motivation and scope, and discuss some followups. Fwiw, I have a fair amount of it implemented, so I have to be able to share an initial patch for first look within a week or two (but obviously it'll take a bit more time to gather feedback on it and get proper testing, so still early to say when this can be expected into a release).

Of course, any feedback there is welcome.

@lennyburdette
Copy link
Contributor Author

Closed by #2279 — see https://www.apollographql.com/docs/federation/federated-types/interfaces/

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

10 participants