Unexpected Query Plan

Help
, ,
1

Hi guys,

At the summit in one of the Federation From Day One workshops, I asked a question about how one would go about returning a list of all reviews for a location that were over 3 stars. I was told it was out of scope for the demo, and I unfortunately did not have time to follow up in person so I am doing so here.

I took a stab at adding a couple of fields to the Location service.

type Location @key(fields: "id")
  id: ID!
<snip>

  reviewsForLocation: [Review]! @external
  reviews: [Review!]! @requires(fields: "reviewsForLocation { ... on Review { rating } }")
}

type Review @key(fields: "id") {
  id: ID!
  rating: Int @external
}

This composes just fine, but when I try to execute the following query, I get some strange behavior

query Locations {
  locations {
    id
    name
    description
    reviews {
      rating
      comment
    }
  }
}

There are subgraph errors redacted, and then it complains about returning null for non-nullable field location…

But most strikingly, the query plan has this step:

Flatten(path: "locations.@") {
      Fetch(service: "locations") {
        {
          ... on Location {
            __typename
            reviewsForLocation {
              id
              rating
            }
            id
          }
        } =>
        {
          ... on Location {
            reviews {
              __typename
              id
            }
          }
        }
      },
    },

This feels wrong. Why are we fetching for reviewsForLocation from the locations service? The rating field is marked as @external and the locations service can’t resolve it, the entire reviewsForLocation field is marked as @external. Furthermore, @requires against a local field doesn’t work (though I would love that feature).

The field resolvers are reporting some strange things as well. The reviews service is receiving requests for reviewsForLocation, and is resolving them correctly, but the field resolver for reviews on the location service does not have the field at all on the parent.

Is this working as intended? How am I supposed to do this if so?
I have attempted several minor tweaks of this (resolvable: false on the Review type, not using an inline fragment in requires) and get the same result.

2

Hello @Patrick_Hansen! I wasn’t at the workshop but I’m familiar with the demo app (FlyBy) it uses.

I think right off the bat, I would have added this functionality to the reviews subgraph, not the locations subgraph, since it’s related to review information. This could be a new field on the Location entity (reviewsOverThreeStars for example, if it’s something that the client needs to use consistently) or a more general field that takes in an argument for the rating value (reviewsForLocation(rating: Int!)). All the resolvers would live in the reviews subgraph and I don’t think we would need any extra directives like @external or @requires.

So to clarify - these are fields on the Location entity, but they live in the reviews subgraph.

With your approach:

I took a stab at adding a couple of fields to the Location service.

The fields are in the locations subgraph, so I believe that’s why the query plan is fetching the data from the locations service.

PS - If you’d like to see those redacted errors, you can set that configuration following these instructions in the docs.

Let me know if this makes sense! Feel free to share your code as well and I can have a look.

3

Thanks for the reply.

This is admittedly a bit of a contrived example. We have multiple examples of places where we need to go fetch a list of objects from some other subgraph and then do some filtering off of the results before resolving out the final response. If we had a situation here where there was some data in the locations service that we needed to filter the review off of, such as some input filter from the request. The reviews subgraph wouldn’t know about the input filters, so it needs to come back to the locations service for further processing. This is the scenario I’m trying to build.

The fields are in the locations subgraph, so I believe that’s why the query plan is fetching the data from the locations service.

The fields are explicitly marked as not in the locations subgraph. Isn’t that what @external means? The docs state Indicates that this subgraph usually can't resolve a particular object field, but it still needs to define that field for other purposes., which is what I’m doing here.

4

This isn’t a pattern supported by federation. If you need to paginate, aggregate, filter, or search for data that live in multiple sources, you’ll always be better off doing that in another data source, not in your API routing layer.

A common pattern is to replicate a subset of data from multiple sources into a single store with the appropriate indices. Elasticsearch is a good candidate. Then you can query that replicated data store to search/filter/whatever data using facets from multiple domains. If you put a subgraph in front of it, that subgraph can simply return entity references, and the query planner will fetch the rest of the fields from the canonical services.

This does mean you’ll have new eventual consistency challenges, and you’ll have another data store and service to maintain, but it’s almost always worth the investment.

Trying to bend query planning to support this kind of functionality will result in overly complex query plans, lots of wasted bytes traveling between systems, and features that are difficult to evolve and improve as requirements change.

5

Yea that’s the pattern we’ve been contemplating. It sure does break separation of concerns and encapsulation and pulls us away from the microservices pattern. It is dangerously close to just replicating another monolith. I’ve been trying to lean into federation but if it can’t do what we need it to do maybe we’ll have to find another solution.

Thank you!

6

This has been a point of contention for me w.r.t @requires feature in general. What is the right use-case for it then? Contributing a field to the entity may often require fetching more data - which is what I thought @requires was supposed to facilitate. Is there any guidebook on when to not use/abuse this feature?

7

We have a more detailed writeup available here (thank you Lenny!). I hope the example scenario and solution helps with your particular use case. Let us know if you have any follow-up questions though!

8

OK yea that writeup was very helpful and was very close to the long term solution we seem to be heading in. I do have a couple follow up questions…

  1. How would we handle auth issues that come up with this approach? Say you enable a search for users, but then we use some auth solution to gate user visibility. It seems clear to me that the domain subgraph should own that authZ definition. With the search subgraph solution, wouldn’t we end up in a situation where the output list of users is going to get partially filled with nulls when the auth check failed on individual results? This is a problem when we don’t want to expose the existence of additional users beyond what the client was authed to see. We could duplicate the auth check on the search subgraph but now we’ve got a duplication of highly sensitive logic that is subject to change and has a high cost of failure. *edit: It’s worse than this, isn’t it? The domain subgraph can’t edit the results of the search subgraph and so it can’t prevent leaking the id of the denied record. This exists anywhere we resolve out a type on another subgraph and we have no way to decline to resolve out the existence of a record. Am I right about that? We have no choice but to duplicate auth checks among all subgraphs that can resolve a particular type. This is a problem.

  2. Business logic is another issue where we can run into logic duplication issues between the search subgraph and domain subgraphs. I can’t come up with an example off the top of my head but my gut tells me there’s going to be a business rule here or there that need further abstraction beyond an elastic query. Again there’s the duplication issue between search and domain subgraphs. I’m wondering if on this issue it would be best to resolve the highest level entity id and provide the lowest amount of detail to allow for domain subgraphs to maintain autonomy.